Skip to content

Commit

Permalink
Fix Dimensions not updating on Android (#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.

### Related issues

- #29105
- #29451
- #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.

## Changelog

[Android] [Fixed] - Fix Dimensions not updating

Pull Request resolved: #31973

Test Plan:
### Steps to reproduce
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.

### Verifying the fix

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

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

### Video

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 facebook-github-bot committed Aug 18, 2021
1 parent 842bcb9 commit c18a492
Show file tree
Hide file tree
Showing 5 changed files with 185 additions and 32 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.ReactSoftExceptionLogger;
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.hasActiveReactInstance()) {
// 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 @@ -13,9 +13,8 @@
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.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 @@ -81,40 +80,20 @@ public static DisplayMetrics getScreenDisplayMetrics() {
return sScreenDisplayMetrics;
}

public static Map<String, Map<String, Object>> getDisplayMetricsMap(double fontScale) {
public static WritableMap getDisplayMetricsWritableMap(double fontScale) {
Assertions.assertCondition(
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.assertCondition(
sWindowDisplayMetrics != null && sScreenDisplayMetrics != null,
"DisplayMetricsHolder must be initialized with initDisplayMetricsIfNotInitialized or initDisplayMetrics");
"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 @@ -32,6 +32,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 c18a492

Please sign in to comment.