From 6661575791ea87bbe990ca1ffc5c20701bffa2e3 Mon Sep 17 00:00:00 2001 From: Charles Chen Date: Mon, 13 Apr 2020 09:44:04 +0000 Subject: [PATCH 1/2] Fix DecorView error about non-visual context This error showed because DecorContext uses application context to get WindowManager. This CL changes to use PhoneWindow to obtain WindowManager instance. Also refactor ctr to obtain context from PhoneWindow. Bug: 152806048 Test: manual - enable strict mode and check the error log not shown. Test: atest DecorContextTest Test: atest MemoryTests#testActivityRecreation Change-Id: I1d416b9cdb015c9bc3553571041f3b14bb9da5da --- core/java/android/view/WindowManagerImpl.java | 4 +- .../android/internal/policy/DecorContext.java | 76 ++++++++++--------- .../android/internal/policy/PhoneWindow.java | 2 +- .../internal/policy/DecorContextTest.java | 42 ++++++++-- 4 files changed, 77 insertions(+), 47 deletions(-) diff --git a/core/java/android/view/WindowManagerImpl.java b/core/java/android/view/WindowManagerImpl.java index 2975d5ee8e1c3..28a18da37b3e1 100644 --- a/core/java/android/view/WindowManagerImpl.java +++ b/core/java/android/view/WindowManagerImpl.java @@ -36,6 +36,7 @@ import android.os.Bundle; import android.os.IBinder; import android.os.RemoteException; +import com.android.internal.annotations.VisibleForTesting; import com.android.internal.os.IResultReceiver; import java.util.List; @@ -69,7 +70,8 @@ import java.util.List; public final class WindowManagerImpl implements WindowManager { @UnsupportedAppUsage private final WindowManagerGlobal mGlobal = WindowManagerGlobal.getInstance(); - private final Context mContext; + @VisibleForTesting + public final Context mContext; private final Window mParentWindow; private IBinder mDefaultToken; diff --git a/core/java/com/android/internal/policy/DecorContext.java b/core/java/com/android/internal/policy/DecorContext.java index 99b4b5fb77077..01bedb7b0fc6e 100644 --- a/core/java/com/android/internal/policy/DecorContext.java +++ b/core/java/com/android/internal/policy/DecorContext.java @@ -22,8 +22,6 @@ import android.content.Context; import android.content.res.AssetManager; import android.content.res.Resources; import android.view.ContextThemeWrapper; -import android.view.WindowManager; -import android.view.WindowManagerImpl; import android.view.contentcapture.ContentCaptureManager; import com.android.internal.annotations.VisibleForTesting; @@ -31,8 +29,8 @@ import com.android.internal.annotations.VisibleForTesting; import java.lang.ref.WeakReference; /** - * Context for decor views which can be seeded with pure application context and not depend on the - * activity, but still provide some of the facilities that Activity has, + * Context for decor views which can be seeded with display context and not depend on the activity, + * but still provide some of the facilities that Activity has, * e.g. themes, activity-based resources, etc. * * @hide @@ -40,79 +38,83 @@ import java.lang.ref.WeakReference; @VisibleForTesting(visibility = VisibleForTesting.Visibility.PACKAGE) public class DecorContext extends ContextThemeWrapper { private PhoneWindow mPhoneWindow; - private WindowManager mWindowManager; - private Resources mActivityResources; + private Resources mResources; private ContentCaptureManager mContentCaptureManager; - private WeakReference mActivityContext; + private WeakReference mContext; - // TODO(b/149928768): Non-activity context can be passed. @VisibleForTesting - public DecorContext(Context context, Context activityContext) { - super(context.createDisplayContext(activityContext.getDisplayNoVerify()), null); - mActivityContext = new WeakReference<>(activityContext); - mActivityResources = activityContext.getResources(); + public DecorContext(Context baseContext, PhoneWindow phoneWindow) { + super(null /* base */, null); + setPhoneWindow(phoneWindow); + final Context displayContext = baseContext.createDisplayContext( + // TODO(b/149790106): Non-activity context can be passed. + phoneWindow.getContext().getDisplayNoVerify()); + attachBaseContext(displayContext); } void setPhoneWindow(PhoneWindow phoneWindow) { mPhoneWindow = phoneWindow; - mWindowManager = null; + final Context context = phoneWindow.getContext(); + mContext = new WeakReference<>(context); + mResources = context.getResources(); } @Override public Object getSystemService(String name) { if (Context.WINDOW_SERVICE.equals(name)) { - if (mWindowManager == null) { - WindowManagerImpl wm = - (WindowManagerImpl) super.getSystemService(Context.WINDOW_SERVICE); - mWindowManager = wm.createLocalWindowManager(mPhoneWindow); - } - return mWindowManager; + return mPhoneWindow.getWindowManager(); } + final Context context = mContext.get(); if (Context.CONTENT_CAPTURE_MANAGER_SERVICE.equals(name)) { - if (mContentCaptureManager == null) { - Context activityContext = mActivityContext.get(); - if (activityContext != null) { - mContentCaptureManager = (ContentCaptureManager) activityContext - .getSystemService(name); - } + if (context != null && mContentCaptureManager == null) { + mContentCaptureManager = (ContentCaptureManager) context.getSystemService(name); } return mContentCaptureManager; } - return super.getSystemService(name); + // TODO(b/154191411): Try to revisit this issue in S. + // We use application to get DisplayManager here because ViewRootImpl holds reference of + // DisplayManager and implicitly holds reference of mContext, which makes activity cannot + // be GC'd even after destroyed if mContext is an activity object. + if (Context.DISPLAY_SERVICE.equals(name)) { + return super.getSystemService(name); + } + // LayoutInflater and WallpaperManagerService should also be obtained from visual context + // instead of base context. + return (context != null) ? context.getSystemService(name) : super.getSystemService(name); } @Override public Resources getResources() { - Context activityContext = mActivityContext.get(); + Context context = mContext.get(); // Attempt to update the local cached Resources from the activity context. If the activity // is no longer around, return the old cached values. - if (activityContext != null) { - mActivityResources = activityContext.getResources(); + if (context != null) { + mResources = context.getResources(); } - return mActivityResources; + return mResources; } @Override public AssetManager getAssets() { - return mActivityResources.getAssets(); + return mResources.getAssets(); } @Override public AutofillOptions getAutofillOptions() { - Context activityContext = mActivityContext.get(); - if (activityContext != null) { - return activityContext.getAutofillOptions(); + Context context = mContext.get(); + if (context != null) { + return context.getAutofillOptions(); } return null; } @Override public ContentCaptureOptions getContentCaptureOptions() { - Context activityContext = mActivityContext.get(); - if (activityContext != null) { - return activityContext.getContentCaptureOptions(); + Context context = mContext.get(); + if (context != null) { + return context.getContentCaptureOptions(); } return null; } diff --git a/core/java/com/android/internal/policy/PhoneWindow.java b/core/java/com/android/internal/policy/PhoneWindow.java index 23ba6530b0727..aa75d40107484 100644 --- a/core/java/com/android/internal/policy/PhoneWindow.java +++ b/core/java/com/android/internal/policy/PhoneWindow.java @@ -2358,7 +2358,7 @@ public class PhoneWindow extends Window implements MenuBuilder.Callback { if (applicationContext == null) { context = getContext(); } else { - context = new DecorContext(applicationContext, getContext()); + context = new DecorContext(applicationContext, this); if (mTheme != -1) { context.setTheme(mTheme); } diff --git a/core/tests/coretests/src/com/android/internal/policy/DecorContextTest.java b/core/tests/coretests/src/com/android/internal/policy/DecorContextTest.java index 3e40466e4b649..2764a16de33cf 100644 --- a/core/tests/coretests/src/com/android/internal/policy/DecorContextTest.java +++ b/core/tests/coretests/src/com/android/internal/policy/DecorContextTest.java @@ -20,19 +20,24 @@ import static android.view.Display.DEFAULT_DISPLAY; import static org.junit.Assert.assertEquals; +import android.app.Activity; +import android.app.EmptyActivity; import android.content.Context; import android.hardware.display.DisplayManagerGlobal; import android.platform.test.annotations.Presubmit; import android.view.Display; import android.view.DisplayAdjustments; import android.view.DisplayInfo; +import android.view.WindowManager; +import android.view.WindowManagerImpl; -import androidx.test.InstrumentationRegistry; +import androidx.test.core.app.ApplicationProvider; import androidx.test.filters.SmallTest; +import androidx.test.rule.ActivityTestRule; import androidx.test.runner.AndroidJUnit4; - import org.junit.Before; +import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; @@ -46,17 +51,22 @@ public final class DecorContextTest { private Context mContext; private static final int EXTERNAL_DISPLAY = DEFAULT_DISPLAY + 1; + @Rule + public ActivityTestRule mActivityRule = + new ActivityTestRule<>(EmptyActivity.class); + @Before - public void setUp() throws Exception { - mContext = InstrumentationRegistry.getContext(); + public void setUp() { + mContext = ApplicationProvider.getApplicationContext(); } @Test public void testDecorContextWithDefaultDisplay() { Display defaultDisplay = new Display(DisplayManagerGlobal.getInstance(), DEFAULT_DISPLAY, new DisplayInfo(), DisplayAdjustments.DEFAULT_DISPLAY_ADJUSTMENTS); - DecorContext context = new DecorContext(mContext.getApplicationContext(), - mContext.createDisplayContext(defaultDisplay)); + final Context defaultDisplayContext = mContext.createDisplayContext(defaultDisplay); + final PhoneWindow window = new PhoneWindow(defaultDisplayContext); + DecorContext context = new DecorContext(mContext.getApplicationContext(), window); assertDecorContextDisplay(DEFAULT_DISPLAY, context); } @@ -65,8 +75,9 @@ public final class DecorContextTest { public void testDecorContextWithExternalDisplay() { Display display = new Display(DisplayManagerGlobal.getInstance(), EXTERNAL_DISPLAY, new DisplayInfo(), DisplayAdjustments.DEFAULT_DISPLAY_ADJUSTMENTS); - DecorContext context = new DecorContext(mContext.getApplicationContext(), - mContext.createDisplayContext(display)); + final Context defaultDisplayContext = mContext.createDisplayContext(display); + final PhoneWindow window = new PhoneWindow(defaultDisplayContext); + DecorContext context = new DecorContext(mContext.getApplicationContext(), window); assertDecorContextDisplay(EXTERNAL_DISPLAY, context); } @@ -76,4 +87,19 @@ public final class DecorContextTest { Display associatedDisplay = decorContext.getDisplay(); assertEquals(expectedDisplayId, associatedDisplay.getDisplayId()); } + + @Test + public void testGetWindowManagerFromVisualDecorContext() throws Throwable { + mActivityRule.runOnUiThread(() -> { + Activity activity = mActivityRule.getActivity(); + final DecorContext decorContext = new DecorContext(mContext.getApplicationContext(), + (PhoneWindow) activity.getWindow()); + WindowManagerImpl actualWm = (WindowManagerImpl) + decorContext.getSystemService(WindowManager.class); + WindowManagerImpl expectedWm = (WindowManagerImpl) + activity.getSystemService(WindowManager.class); + // Verify that window manager is from activity not application context. + assertEquals(expectedWm.mContext, actualWm.mContext); + }); + } } From 441efaec077bd7b65420e200e8e63f5114245a36 Mon Sep 17 00:00:00 2001 From: Charles Chen Date: Mon, 20 Apr 2020 15:05:12 +0800 Subject: [PATCH 2/2] Fix error in DecorView's ViewConfiguration In ViewConfiguration, we use isUiContext to verify if a context is an visual context. However, DecorContext uses Application context as a base context and return false as intended. This patch overrides isUiContext to report context#isUiContext instead. fixes: 153664027 Test: atest DecorContextTest#testIsUiContextFromVisualDecorContext Change-Id: Ida26b9617c74dc6997fc2c00d97d3420a0978fc7 --- .../com/android/internal/policy/DecorContext.java | 9 +++++++++ .../com/android/internal/policy/DecorContextTest.java | 11 +++++++++++ 2 files changed, 20 insertions(+) diff --git a/core/java/com/android/internal/policy/DecorContext.java b/core/java/com/android/internal/policy/DecorContext.java index 01bedb7b0fc6e..51b41198e272f 100644 --- a/core/java/com/android/internal/policy/DecorContext.java +++ b/core/java/com/android/internal/policy/DecorContext.java @@ -118,4 +118,13 @@ public class DecorContext extends ContextThemeWrapper { } return null; } + + @Override + public boolean isUiContext() { + Context context = mContext.get(); + if (context != null) { + return context.isUiContext(); + } + return false; + } } diff --git a/core/tests/coretests/src/com/android/internal/policy/DecorContextTest.java b/core/tests/coretests/src/com/android/internal/policy/DecorContextTest.java index 2764a16de33cf..02870a53773e9 100644 --- a/core/tests/coretests/src/com/android/internal/policy/DecorContextTest.java +++ b/core/tests/coretests/src/com/android/internal/policy/DecorContextTest.java @@ -19,6 +19,7 @@ package com.android.internal.policy; import static android.view.Display.DEFAULT_DISPLAY; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; import android.app.Activity; import android.app.EmptyActivity; @@ -102,4 +103,14 @@ public final class DecorContextTest { assertEquals(expectedWm.mContext, actualWm.mContext); }); } + + @Test + public void testIsUiContextFromVisualDecorContext() throws Throwable { + mActivityRule.runOnUiThread(() -> { + Activity activity = mActivityRule.getActivity(); + final DecorContext decorContext = new DecorContext(mContext.getApplicationContext(), + (PhoneWindow) activity.getWindow()); + assertTrue(decorContext.isUiContext()); + }); + } }