Skip to content

Commit

Permalink
Fix Dimensions not updating on Android (facebook#31973)
Browse files Browse the repository at this point in the history
Summary:
When retrieving the device dimensions through the JS `Dimensions` utility, the result of `Dimensions.get` can be incorrect on Android.

- facebook#29105
- facebook#29451
- facebook#29323

The issue is caused by the Android `DeviceInfoModule` that provides initial screen dimensions and then subsequently updates those by emitting `didUpdateDimensions` events. The assumption in that implementation is that the initial display metrics will not have changed prior to the first check for updated metrics. However that is not the case as the device may be rotated (as shown in the attached video).

The solution in this PR is to keep track of the initial dimensions for comparison at the first check for updated metrics.

[Android] [Fixed] - Fix Dimensions not updating

Pull Request resolved: facebook#31973

Test Plan:
1. Install the RNTester app on Android from the `main` branch.
2. Set the device auto-rotation to ON
3. Start the RNTester app
4. While the app is loading, rotate the device
5. Navigate to the `Dimensions` screen
6. Either
 a. Observe the screen width and height are reversed, or
 b. Quit the app and return to step 3.

Using the above steps, the issue should no longer be reproducible.

See unit tests in `ReactAndroid/src/test/java/com/facebook/react/modules/deviceinfo/DeviceInfoModuleTest.java`

https://user-images.githubusercontent.com/4940864/128485453-2ae04724-4ac5-4267-a59a-140cc3af626b.mp4

Reviewed By: JoshuaGross

Differential Revision: D30319919

Pulled By: lunaleaps

fbshipit-source-id: 52a2faeafc522b1c2a196ca40357027eafa1a84b
  • Loading branch information
jonnyandrew authored and Bodapudi Mahesh Babu committed Aug 31, 2021
1 parent c722c6a commit aaaf728
Show file tree
Hide file tree
Showing 5 changed files with 187 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
import com.facebook.react.bridge.ReactNoCrashSoftException;
import com.facebook.react.bridge.ReactSoftException;
import com.facebook.react.bridge.ReadableMap;
import com.facebook.react.bridge.WritableNativeMap;
import com.facebook.react.bridge.WritableMap;
import com.facebook.react.module.annotations.ReactModule;
import com.facebook.react.modules.core.DeviceEventManagerModule;
import com.facebook.react.uimanager.DisplayMetricsHolder;
Expand Down Expand Up @@ -54,8 +54,13 @@ public String getName() {

@Override
public @Nullable Map<String, Object> getTypedExportedConstants() {
WritableMap displayMetrics = DisplayMetricsHolder.getDisplayMetricsWritableMap(mFontScale);

// Cache the initial dimensions for later comparison in emitUpdateDimensionsEvent
mPreviousDisplayMetrics = displayMetrics.copy();

HashMap<String, Object> constants = new HashMap<>();
constants.put("Dimensions", DisplayMetricsHolder.getDisplayMetricsMap(mFontScale));
constants.put("Dimensions", displayMetrics.toHashMap());
return constants;
}

Expand Down Expand Up @@ -85,8 +90,7 @@ public void emitUpdateDimensionsEvent() {

if (mReactApplicationContext.hasActiveCatalystInstance()) {
// Don't emit an event to JS if the dimensions haven't changed
WritableNativeMap displayMetrics =
DisplayMetricsHolder.getDisplayMetricsNativeMap(mFontScale);
WritableMap displayMetrics = DisplayMetricsHolder.getDisplayMetricsWritableMap(mFontScale);
if (mPreviousDisplayMetrics == null) {
mPreviousDisplayMetrics = displayMetrics.copy();
} else if (!displayMetrics.equals(mPreviousDisplayMetrics)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,10 @@
import android.view.WindowManager;
import androidx.annotation.Nullable;
import com.facebook.infer.annotation.Assertions;
import com.facebook.react.bridge.WritableMap;
import com.facebook.react.bridge.WritableNativeMap;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.util.HashMap;
import java.util.Map;

/**
* Holds an instance of the current DisplayMetrics so we don't have to thread it through all the
Expand Down Expand Up @@ -103,40 +102,20 @@ public static DisplayMetrics getScreenDisplayMetrics() {
return sScreenDisplayMetrics;
}

public static Map<String, Map<String, Object>> getDisplayMetricsMap(double fontScale) {
Assertions.assertNotNull(
sWindowDisplayMetrics != null || sScreenDisplayMetrics != null,
"DisplayMetricsHolder must be initialized with initDisplayMetricsIfNotInitialized or initDisplayMetrics");
final Map<String, Map<String, Object>> result = new HashMap<>();
result.put("windowPhysicalPixels", getPhysicalPixelsMap(sWindowDisplayMetrics, fontScale));
result.put("screenPhysicalPixels", getPhysicalPixelsMap(sScreenDisplayMetrics, fontScale));
return result;
}

public static WritableNativeMap getDisplayMetricsNativeMap(double fontScale) {
Assertions.assertNotNull(
sWindowDisplayMetrics != null || sScreenDisplayMetrics != null,
"DisplayMetricsHolder must be initialized with initDisplayMetricsIfNotInitialized or initDisplayMetrics");
public static WritableMap getDisplayMetricsWritableMap(double fontScale) {
Assertions.assertCondition(
sWindowDisplayMetrics != null && sScreenDisplayMetrics != null,
"DisplayMetricsHolder must be initialized with initDisplayMetricsIfNotInitialized or"
+ " initDisplayMetrics");
final WritableNativeMap result = new WritableNativeMap();
result.putMap(
"windowPhysicalPixels", getPhysicalPixelsNativeMap(sWindowDisplayMetrics, fontScale));
"windowPhysicalPixels", getPhysicalPixelsWritableMap(sWindowDisplayMetrics, fontScale));
result.putMap(
"screenPhysicalPixels", getPhysicalPixelsNativeMap(sScreenDisplayMetrics, fontScale));
return result;
}

private static Map<String, Object> getPhysicalPixelsMap(
DisplayMetrics displayMetrics, double fontScale) {
final Map<String, Object> result = new HashMap<>();
result.put("width", displayMetrics.widthPixels);
result.put("height", displayMetrics.heightPixels);
result.put("scale", displayMetrics.density);
result.put("fontScale", fontScale);
result.put("densityDpi", displayMetrics.densityDpi);
"screenPhysicalPixels", getPhysicalPixelsWritableMap(sScreenDisplayMetrics, fontScale));
return result;
}

private static WritableNativeMap getPhysicalPixelsNativeMap(
private static WritableMap getPhysicalPixelsWritableMap(
DisplayMetrics displayMetrics, double fontScale) {
final WritableNativeMap result = new WritableNativeMap();
result.putInt("width", displayMetrics.widthPixels);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ public void handleException(Exception e) {
when(reactInstance.getReactQueueConfiguration()).thenReturn(ReactQueueConfiguration);
when(reactInstance.getNativeModule(UIManagerModule.class))
.thenReturn(mock(UIManagerModule.class));
when(reactInstance.isDestroyed()).thenReturn(false);

return reactInstance;
}
Expand Down
1 change: 1 addition & 0 deletions ReactAndroid/src/test/java/com/facebook/react/modules/BUCK
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ rn_robolectric_test(
react_native_target("java/com/facebook/react/modules/common:common"),
react_native_target("java/com/facebook/react/modules/core:core"),
react_native_target("java/com/facebook/react/modules/debug:debug"),
react_native_target("java/com/facebook/react/modules/deviceinfo:deviceinfo"),
react_native_target("java/com/facebook/react/modules/dialog:dialog"),
react_native_target("java/com/facebook/react/modules/network:network"),
react_native_target("java/com/facebook/react/modules/share:share"),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,168 @@
/*
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

package com.facebook.react.modules.deviceinfo;

import static org.fest.assertions.api.Assertions.assertThat;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoMoreInteractions;
import static org.mockito.Mockito.when;
import static org.powermock.api.mockito.PowerMockito.mockStatic;

import com.facebook.react.bridge.Arguments;
import com.facebook.react.bridge.CatalystInstance;
import com.facebook.react.bridge.JavaOnlyMap;
import com.facebook.react.bridge.ReactApplicationContext;
import com.facebook.react.bridge.ReactTestHelper;
import com.facebook.react.bridge.WritableMap;
import com.facebook.react.modules.core.DeviceEventManagerModule;
import com.facebook.react.uimanager.DisplayMetricsHolder;
import java.util.Arrays;
import java.util.List;
import junit.framework.TestCase;
import org.junit.After;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.ArgumentCaptor;
import org.mockito.invocation.InvocationOnMock;
import org.mockito.stubbing.Answer;
import org.powermock.core.classloader.annotations.PowerMockIgnore;
import org.powermock.core.classloader.annotations.PrepareForTest;
import org.powermock.modules.junit4.rule.PowerMockRule;
import org.robolectric.RobolectricTestRunner;
import org.robolectric.RuntimeEnvironment;

@RunWith(RobolectricTestRunner.class)
@PrepareForTest({Arguments.class, DisplayMetricsHolder.class})
@PowerMockIgnore({"org.mockito.*", "org.robolectric.*", "androidx.*", "android.*"})
public class DeviceInfoModuleTest extends TestCase {

@Rule public PowerMockRule rule = new PowerMockRule();

private DeviceInfoModule mDeviceInfoModule;
private DeviceEventManagerModule.RCTDeviceEventEmitter mRCTDeviceEventEmitterMock;

private WritableMap fakePortraitDisplayMetrics;
private WritableMap fakeLandscapeDisplayMetrics;

@Before
public void setUp() {
initTestData();

mockStatic(DisplayMetricsHolder.class);

mRCTDeviceEventEmitterMock = mock(DeviceEventManagerModule.RCTDeviceEventEmitter.class);

final ReactApplicationContext context =
spy(new ReactApplicationContext(RuntimeEnvironment.application));
CatalystInstance catalystInstanceMock = ReactTestHelper.createMockCatalystInstance();
context.initializeWithInstance(catalystInstanceMock);
when(context.getJSModule(DeviceEventManagerModule.RCTDeviceEventEmitter.class))
.thenReturn(mRCTDeviceEventEmitterMock);

mDeviceInfoModule = new DeviceInfoModule(context);
}

@After
public void teardown() {
DisplayMetricsHolder.setWindowDisplayMetrics(null);
DisplayMetricsHolder.setScreenDisplayMetrics(null);
}

@Test
public void test_itDoesNotEmitAnEvent_whenDisplayMetricsNotChanged() {
givenDisplayMetricsHolderContains(fakePortraitDisplayMetrics);

mDeviceInfoModule.getTypedExportedConstants();
mDeviceInfoModule.emitUpdateDimensionsEvent();

verifyNoMoreInteractions(mRCTDeviceEventEmitterMock);
}

@Test
public void test_itEmitsOneEvent_whenDisplayMetricsChangedOnce() {
givenDisplayMetricsHolderContains(fakePortraitDisplayMetrics);

mDeviceInfoModule.getTypedExportedConstants();
givenDisplayMetricsHolderContains(fakeLandscapeDisplayMetrics);
mDeviceInfoModule.emitUpdateDimensionsEvent();

verifyUpdateDimensionsEventsEmitted(mRCTDeviceEventEmitterMock, fakeLandscapeDisplayMetrics);
}

@Test
public void test_itEmitsJustOneEvent_whenUpdateRequestedMultipleTimes() {
givenDisplayMetricsHolderContains(fakePortraitDisplayMetrics);
mDeviceInfoModule.getTypedExportedConstants();
givenDisplayMetricsHolderContains(fakeLandscapeDisplayMetrics);
mDeviceInfoModule.emitUpdateDimensionsEvent();
mDeviceInfoModule.emitUpdateDimensionsEvent();

verifyUpdateDimensionsEventsEmitted(mRCTDeviceEventEmitterMock, fakeLandscapeDisplayMetrics);
}

@Test
public void test_itEmitsMultipleEvents_whenDisplayMetricsChangedBetweenUpdates() {
givenDisplayMetricsHolderContains(fakePortraitDisplayMetrics);

mDeviceInfoModule.getTypedExportedConstants();
mDeviceInfoModule.emitUpdateDimensionsEvent();
givenDisplayMetricsHolderContains(fakeLandscapeDisplayMetrics);
mDeviceInfoModule.emitUpdateDimensionsEvent();
givenDisplayMetricsHolderContains(fakePortraitDisplayMetrics);
mDeviceInfoModule.emitUpdateDimensionsEvent();
givenDisplayMetricsHolderContains(fakeLandscapeDisplayMetrics);
mDeviceInfoModule.emitUpdateDimensionsEvent();

verifyUpdateDimensionsEventsEmitted(
mRCTDeviceEventEmitterMock,
fakeLandscapeDisplayMetrics,
fakePortraitDisplayMetrics,
fakeLandscapeDisplayMetrics);
}

private static void givenDisplayMetricsHolderContains(final WritableMap fakeDisplayMetrics) {
when(DisplayMetricsHolder.getDisplayMetricsWritableMap(1.0)).thenReturn(fakeDisplayMetrics);
}

private static void verifyUpdateDimensionsEventsEmitted(
DeviceEventManagerModule.RCTDeviceEventEmitter emitter, WritableMap... expectedEvents) {
List<WritableMap> expectedEventList = Arrays.asList(expectedEvents);
ArgumentCaptor<WritableMap> captor = ArgumentCaptor.forClass(WritableMap.class);
verify(emitter, times(expectedEventList.size()))
.emit(eq("didUpdateDimensions"), captor.capture());

List<WritableMap> actualEvents = captor.getAllValues();
assertThat(actualEvents).isEqualTo(expectedEventList);
}

private void initTestData() {
mockStatic(Arguments.class);
when(Arguments.createMap())
.thenAnswer(
new Answer<Object>() {
@Override
public Object answer(InvocationOnMock invocation) throws Throwable {
return new JavaOnlyMap();
}
});

fakePortraitDisplayMetrics = Arguments.createMap();
fakePortraitDisplayMetrics.putInt("width", 100);
fakePortraitDisplayMetrics.putInt("height", 200);

fakeLandscapeDisplayMetrics = Arguments.createMap();
fakeLandscapeDisplayMetrics.putInt("width", 200);
fakeLandscapeDisplayMetrics.putInt("height", 100);
}
}

0 comments on commit aaaf728

Please sign in to comment.