From 5281b6b4c06f3b6b3d63ed9877101229645df9a9 Mon Sep 17 00:00:00 2001 From: Yohei Yukawa Date: Mon, 15 Oct 2018 07:38:25 +0800 Subject: [PATCH] Add Context.getDisplayId() to avoid possible IPC ContextImpl has an internal rule that when ContextImpl#mDisplay is null the Context is associated with the default display. The problem is that, as discussed in Bug 117709581, when ContextImpl#mDisplay is null ContextImpl#getDisplay() tries to get some non-null Display object by making an IPC to the system server, which is redundant when the display ID is the only thing that the caller wants to know. By having an @hide method Context.getDisplayId(), we can ensure that display ID can be obtained without any IPC. This enables us to re-submit my CL [1] that aimed to instantiate InputMethodManager (IMM) for each display but then got reverted due to a performance regression (Bug 117434607). There should be no developer-observable behavior change. [1]: I7242e765426353672823fcc8277f20ac361930d7 c53d78e992694e471ddaae73f9a30977db9cdb75 Fix: 117712745 Test: atest FrameworksCoreTests:android.content.ContextTest Test: prebuilts/checkstyle/checkstyle.py -f \ frameworks/base/core/tests/coretests/src/android/content/ContextTest.java Change-Id: I2534530a5ce90e2620c5039d793a6454a0a1e154 --- core/java/android/app/ActivityThread.java | 4 +- core/java/android/app/ContextImpl.java | 16 ++--- core/java/android/content/Context.java | 8 +++ core/java/android/content/ContextWrapper.java | 8 +++ .../hardware/display/DisplayManager.java | 2 +- core/java/android/widget/Toast.java | 2 +- .../android/internal/policy/PhoneWindow.java | 4 +- .../src/android/content/ContextTest.java | 60 +++++++++++++++++++ .../shared/system/RotationWatcher.java | 2 +- .../phone/NavigationBarFragment.java | 2 +- .../server/wm/ScreenDecorWindowTests.java | 2 +- .../src/android/test/mock/MockContext.java | 6 ++ 12 files changed, 100 insertions(+), 16 deletions(-) create mode 100644 core/tests/coretests/src/android/content/ContextTest.java diff --git a/core/java/android/app/ActivityThread.java b/core/java/android/app/ActivityThread.java index ff6aca6fa6c4a..9d3c5c6417afe 100644 --- a/core/java/android/app/ActivityThread.java +++ b/core/java/android/app/ActivityThread.java @@ -5054,7 +5054,7 @@ public final class ActivityThread extends ClientTransactionHandler { private void performConfigurationChangedForActivity(ActivityClientRecord r, Configuration newBaseConfig) { performConfigurationChangedForActivity(r, newBaseConfig, - r.activity.getDisplay().getDisplayId(), false /* movedToDifferentDisplay */); + r.activity.getDisplayId(), false /* movedToDifferentDisplay */); } /** @@ -5406,7 +5406,7 @@ public final class ActivityThread extends ClientTransactionHandler { return; } final boolean movedToDifferentDisplay = displayId != INVALID_DISPLAY - && displayId != r.activity.getDisplay().getDisplayId(); + && displayId != r.activity.getDisplayId(); // Perform updates. r.overrideConfig = overrideConfig; diff --git a/core/java/android/app/ContextImpl.java b/core/java/android/app/ContextImpl.java index 77f639535b15f..dc707e892d9a7 100644 --- a/core/java/android/app/ContextImpl.java +++ b/core/java/android/app/ContextImpl.java @@ -2088,8 +2088,7 @@ class ContextImpl extends Context { ContextImpl c = new ContextImpl(this, mMainThread, pi, null, mActivityToken, new UserHandle(UserHandle.getUserId(application.uid)), flags, null); - final int displayId = mDisplay != null - ? mDisplay.getDisplayId() : Display.DEFAULT_DISPLAY; + final int displayId = getDisplayId(); c.setResources(createResources(mActivityToken, pi, null, displayId, null, getDisplayAdjustments(displayId).getCompatibilityInfo())); @@ -2124,8 +2123,7 @@ class ContextImpl extends Context { ContextImpl c = new ContextImpl(this, mMainThread, pi, null, mActivityToken, user, flags, null); - final int displayId = mDisplay != null - ? mDisplay.getDisplayId() : Display.DEFAULT_DISPLAY; + final int displayId = getDisplayId(); c.setResources(createResources(mActivityToken, pi, null, displayId, null, getDisplayAdjustments(displayId).getCompatibilityInfo())); @@ -2152,8 +2150,7 @@ class ContextImpl extends Context { final ContextImpl context = new ContextImpl(this, mMainThread, mPackageInfo, splitName, mActivityToken, mUser, mFlags, classLoader); - final int displayId = mDisplay != null - ? mDisplay.getDisplayId() : Display.DEFAULT_DISPLAY; + final int displayId = getDisplayId(); context.setResources(ResourcesManager.getInstance().getResources( mActivityToken, @@ -2177,7 +2174,7 @@ class ContextImpl extends Context { ContextImpl context = new ContextImpl(this, mMainThread, mPackageInfo, mSplitName, mActivityToken, mUser, mFlags, mClassLoader); - final int displayId = mDisplay != null ? mDisplay.getDisplayId() : Display.DEFAULT_DISPLAY; + final int displayId = getDisplayId(); context.setResources(createResources(mActivityToken, mPackageInfo, mSplitName, displayId, overrideConfiguration, getDisplayAdjustments(displayId).getCompatibilityInfo())); return context; @@ -2249,6 +2246,11 @@ class ContextImpl extends Context { return mDisplay; } + @Override + public int getDisplayId() { + return mDisplay != null ? mDisplay.getDisplayId() : Display.DEFAULT_DISPLAY; + } + @Override public void updateDisplay(int displayId) { mDisplay = mResourcesManager.getAdjustedDisplay(displayId, mResources); diff --git a/core/java/android/content/Context.java b/core/java/android/content/Context.java index db4adcfaaf351..31973524ceaff 100644 --- a/core/java/android/content/Context.java +++ b/core/java/android/content/Context.java @@ -4981,6 +4981,14 @@ public abstract class Context { @UnsupportedAppUsage public abstract Display getDisplay(); + /** + * Gets the display ID. + * + * @return display ID associated with this {@link Context}. + * @hide + */ + public abstract int getDisplayId(); + /** * @hide */ diff --git a/core/java/android/content/ContextWrapper.java b/core/java/android/content/ContextWrapper.java index c5dce017430b2..bfad2b42bc94b 100644 --- a/core/java/android/content/ContextWrapper.java +++ b/core/java/android/content/ContextWrapper.java @@ -917,6 +917,14 @@ public class ContextWrapper extends Context { return mBase.getDisplay(); } + /** + * @hide + */ + @Override + public int getDisplayId() { + return mBase.getDisplayId(); + } + /** * @hide */ diff --git a/core/java/android/hardware/display/DisplayManager.java b/core/java/android/hardware/display/DisplayManager.java index 09113e5d175df..01ef58e987d71 100644 --- a/core/java/android/hardware/display/DisplayManager.java +++ b/core/java/android/hardware/display/DisplayManager.java @@ -397,7 +397,7 @@ public final class DisplayManager { if (display == null) { // TODO: We cannot currently provide any override configurations for metrics on displays // other than the display the context is associated with. - final Context context = mContext.getDisplay().getDisplayId() == displayId + final Context context = mContext.getDisplayId() == displayId ? mContext : mContext.getApplicationContext(); display = mGlobal.getCompatibleDisplay(displayId, context.getResources()); diff --git a/core/java/android/widget/Toast.java b/core/java/android/widget/Toast.java index 10cf702157473..c256d57a6af6a 100644 --- a/core/java/android/widget/Toast.java +++ b/core/java/android/widget/Toast.java @@ -137,7 +137,7 @@ public class Toast { String pkg = mContext.getOpPackageName(); TN tn = mTN; tn.mNextView = mNextView; - final int displayId = mContext.getDisplay().getDisplayId(); + final int displayId = mContext.getDisplayId(); try { service.enqueueToast(pkg, tn, mDuration, displayId); diff --git a/core/java/com/android/internal/policy/PhoneWindow.java b/core/java/com/android/internal/policy/PhoneWindow.java index 87515173e8def..3b7ce0a22b180 100644 --- a/core/java/com/android/internal/policy/PhoneWindow.java +++ b/core/java/com/android/internal/policy/PhoneWindow.java @@ -1387,7 +1387,7 @@ public class PhoneWindow extends Window implements MenuBuilder.Callback { private int getOptionsPanelGravity() { try { return WindowManagerHolder.sWindowManager.getPreferredOptionsPanelGravity( - getContext().getDisplay().getDisplayId()); + getContext().getDisplayId()); } catch (RemoteException ex) { Log.e(TAG, "Couldn't getOptionsPanelGravity; using default", ex); return Gravity.CENTER | Gravity.BOTTOM; @@ -3642,7 +3642,7 @@ public class PhoneWindow extends Window implements MenuBuilder.Callback { if (!mIsWatching) { try { WindowManagerHolder.sWindowManager.watchRotation(this, - phoneWindow.getContext().getDisplay().getDisplayId()); + phoneWindow.getContext().getDisplayId()); mHandler = new Handler(); mIsWatching = true; } catch (RemoteException ex) { diff --git a/core/tests/coretests/src/android/content/ContextTest.java b/core/tests/coretests/src/android/content/ContextTest.java new file mode 100644 index 0000000000000..c8a3098690bed --- /dev/null +++ b/core/tests/coretests/src/android/content/ContextTest.java @@ -0,0 +1,60 @@ +/* + * Copyright (C) 2018 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.content; + +import static org.junit.Assert.assertEquals; + +import android.app.ActivityThread; +import android.support.test.InstrumentationRegistry; +import android.support.test.filters.SmallTest; +import android.support.test.runner.AndroidJUnit4; +import android.view.WindowManager; + +import org.junit.Test; +import org.junit.runner.RunWith; + +@SmallTest +@RunWith(AndroidJUnit4.class) +public class ContextTest { + @Test + public void testDisplayIdForSystemContext() { + final Context systemContext = + ActivityThread.currentActivityThread().getSystemContext(); + + assertEquals(systemContext.getDisplay().getDisplayId(), systemContext.getDisplayId()); + } + + @Test + public void testDisplayIdForTestContext() { + final Context testContext = + InstrumentationRegistry.getInstrumentation().getTargetContext(); + + assertEquals(testContext.getDisplay().getDisplayId(), testContext.getDisplayId()); + } + + @Test + public void testDisplayIdForDefaultDisplayContext() { + final Context testContext = + InstrumentationRegistry.getInstrumentation().getTargetContext(); + final WindowManager wm = testContext.getSystemService(WindowManager.class); + final Context defaultDisplayContext = + testContext.createDisplayContext(wm.getDefaultDisplay()); + + assertEquals(defaultDisplayContext.getDisplay().getDisplayId(), + defaultDisplayContext.getDisplayId()); + } +} diff --git a/packages/SystemUI/shared/src/com/android/systemui/shared/system/RotationWatcher.java b/packages/SystemUI/shared/src/com/android/systemui/shared/system/RotationWatcher.java index 5a28a5e28d912..7c8c23eba73b4 100644 --- a/packages/SystemUI/shared/src/com/android/systemui/shared/system/RotationWatcher.java +++ b/packages/SystemUI/shared/src/com/android/systemui/shared/system/RotationWatcher.java @@ -48,7 +48,7 @@ public abstract class RotationWatcher { if (!mIsWatching) { try { WindowManagerGlobal.getWindowManagerService().watchRotation(mWatcher, - mContext.getDisplay().getDisplayId()); + mContext.getDisplayId()); mIsWatching = true; } catch (RemoteException e) { Log.w(TAG, "Failed to set rotation watcher", e); diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/NavigationBarFragment.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/NavigationBarFragment.java index 6bccf31b04a63..99a2cdc9a5459 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/NavigationBarFragment.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/NavigationBarFragment.java @@ -236,7 +236,7 @@ public class NavigationBarFragment extends Fragment implements Callbacks { try { WindowManagerGlobal.getWindowManagerService() - .watchRotation(mRotationWatcher, getContext().getDisplay().getDisplayId()); + .watchRotation(mRotationWatcher, getContext().getDisplayId()); } catch (RemoteException e) { throw e.rethrowFromSystemServer(); } diff --git a/services/tests/servicestests/src/com/android/server/wm/ScreenDecorWindowTests.java b/services/tests/servicestests/src/com/android/server/wm/ScreenDecorWindowTests.java index 60025f0f03fe4..ae40f7e7d2354 100644 --- a/services/tests/servicestests/src/com/android/server/wm/ScreenDecorWindowTests.java +++ b/services/tests/servicestests/src/com/android/server/wm/ScreenDecorWindowTests.java @@ -334,7 +334,7 @@ public class ScreenDecorWindowTests { final Activity activity = mInstrumentation.startActivitySync(intent, options.toBundle()); waitForIdle(); - assertEquals(displayId, activity.getDisplay().getDisplayId()); + assertEquals(displayId, activity.getDisplayId()); return activity; } diff --git a/test-mock/src/android/test/mock/MockContext.java b/test-mock/src/android/test/mock/MockContext.java index 9d260ebf72311..fa5b896ea1268 100644 --- a/test-mock/src/android/test/mock/MockContext.java +++ b/test-mock/src/android/test/mock/MockContext.java @@ -772,6 +772,12 @@ public class MockContext extends Context { throw new UnsupportedOperationException(); } + /** @hide */ + @Override + public int getDisplayId() { + throw new UnsupportedOperationException(); + } + /** @hide */ @Override public void updateDisplay(int displayId) {