From 29de30597b65c9111418278d04f03c35eb37d700 Mon Sep 17 00:00:00 2001 From: Ming-Shin Lu Date: Mon, 10 Jan 2022 12:14:51 +0800 Subject: [PATCH 1/3] Fix IME shrunk by WindowTokenClient mis-detach As CL[1] introduced WindowTokenClient for WindowProviderService (aka the parent class of InputMethodService starts from CL[2]) as a token that IME context can associate with the windowContainer of the InputMethod window in server side. Like the activity context, IME context can adopt configuration/resources update when the IME window changed by display/window changes. And, the IME context caller can also create another type of context with wrapping IME context (i.e. calling createDisplayContext to create a display context), that makes this context can be mixed the window token of WindowProviderService since it's the base context. However, the finalization of the context mixed WindowTokenClient will detach the token when the attached context type is non-window context, this action will mis-detach the token when it managed by WindowProviderService. So like SoftKeyboard previously using createDisplayContext in CL[3] to workaround context resources issues, will in-directly expose this mis-detach token issue as the above. Beside, the handling of WindowTokenClient#{onConfigurationChange, onWindowTokenRemoved} does not thread-safe since this is called from IPC. As the result, the fix is to ignore the check in ContextImpl#finalize to not detach the token when it managed by WindowProviderService, also make sure to post to the main handler when received onConfigurationChanged/onWindowTokenRemoved in WindowTokenClient. Note that this fix could help to resolve "The Window Context should have been attached to a DisplayArea." exception if the token has been detached as the above case that happens before the next WindowProviderService#attachToWindowToken invoked. [1]: I64a1614f32d097785915f6105b1813a929e0fe32 [2]: Ie565e30ed5dd3f2cfe27355a6dded76dc3adc14b [3]: Ic592a1d2fb2da149220c8b503b522b3e864bcc77 Bug: 213118079 Bug: 211062619 Test: manual as steps: 1) adb install -r EditTextVariations.apk 2) adb install -r SoftKeyboard.apk 3) adb shell ime enable com.example.android.softkeyboard/.SoftKeyboard 4) adb shell ime set com.example.android.softkeyboard/.SoftKeyboard5 5) Enable screen auto-rotation 6) Launch EditTextVariations from launcher's shortcut 7) Tap the first EditText field to show IME 8) Rotate the device to the landscape mode 9) Expect the IME should not be shrunk Change-Id: I7beb7a122af93e596239a36db62073233cea0726 --- core/java/android/app/ContextImpl.java | 11 +++++- .../android/window/WindowTokenClient.java | 34 ++++++++++++------- 2 files changed, 32 insertions(+), 13 deletions(-) diff --git a/core/java/android/app/ContextImpl.java b/core/java/android/app/ContextImpl.java index db3c7d9bcb024..30e088f15ff6f 100644 --- a/core/java/android/app/ContextImpl.java +++ b/core/java/android/app/ContextImpl.java @@ -311,6 +311,14 @@ class ContextImpl extends Context { @ContextType private int mContextType; + /** + * {@code true} to indicate that the {@link Context} owns the {@link #getWindowContextToken()} + * and is responsible for detaching the token when the Context is released. + * + * @see #finalize() + */ + private boolean mOwnsToken = false; + @GuardedBy("mSync") private File mDatabasesDir; @GuardedBy("mSync") @@ -2979,7 +2987,7 @@ class ContextImpl extends Context { // WindowContainer. We should detach from WindowContainer when the Context is finalized // if this Context is not a WindowContext. WindowContext finalization is handled in // WindowContext class. - if (mToken instanceof WindowTokenClient && mContextType != CONTEXT_TYPE_WINDOW_CONTEXT) { + if (mToken instanceof WindowTokenClient && mOwnsToken) { ((WindowTokenClient) mToken).detachFromWindowContainerIfNeeded(); } super.finalize(); @@ -3010,6 +3018,7 @@ class ContextImpl extends Context { token.attachContext(context); token.attachToDisplayContent(displayId); context.mContextType = CONTEXT_TYPE_SYSTEM_OR_SYSTEM_UI; + context.mOwnsToken = true; return context; } diff --git a/core/java/android/window/WindowTokenClient.java b/core/java/android/window/WindowTokenClient.java index b331a9e81e27d..1ba63f5fca3c6 100644 --- a/core/java/android/window/WindowTokenClient.java +++ b/core/java/android/window/WindowTokenClient.java @@ -20,9 +20,10 @@ import static android.window.ConfigurationHelper.freeTextLayoutCachesIfNeeded; import static android.window.ConfigurationHelper.isDifferentDisplay; import static android.window.ConfigurationHelper.shouldUpdateResources; +import android.annotation.BinderThread; +import android.annotation.MainThread; import android.annotation.NonNull; import android.annotation.Nullable; -import android.app.ActivityThread; import android.app.IWindowToken; import android.app.ResourcesManager; import android.content.Context; @@ -31,7 +32,9 @@ import android.inputmethodservice.AbstractInputMethodService; import android.os.Build; import android.os.Bundle; import android.os.Debug; +import android.os.Handler; import android.os.IBinder; +import android.os.Looper; import android.os.RemoteException; import android.util.Log; import android.view.IWindowManager; @@ -72,6 +75,8 @@ public class WindowTokenClient extends IWindowToken.Stub { private boolean mAttachToWindowContainer; + private final Handler mHandler = new Handler(Looper.getMainLooper()); + /** * Attaches {@code context} to this {@link WindowTokenClient}. Each {@link WindowTokenClient} * can only attach one {@link Context}. @@ -133,7 +138,8 @@ public class WindowTokenClient extends IWindowToken.Stub { if (configuration == null) { return false; } - onConfigurationChanged(configuration, displayId, false /* shouldReportConfigChange */); + mHandler.post(() -> onConfigurationChanged(configuration, displayId, + false /* shouldReportConfigChange */)); mAttachToWindowContainer = true; return true; } catch (RemoteException e) { @@ -180,9 +186,11 @@ public class WindowTokenClient extends IWindowToken.Stub { * @param newConfig the updated {@link Configuration} * @param newDisplayId the updated {@link android.view.Display} ID */ + @BinderThread @Override public void onConfigurationChanged(Configuration newConfig, int newDisplayId) { - onConfigurationChanged(newConfig, newDisplayId, true /* shouldReportConfigChange */); + mHandler.post(() -> onConfigurationChanged(newConfig, newDisplayId, + true /* shouldReportConfigChange */)); } // TODO(b/192048581): rewrite this method based on WindowContext and WindowProviderService @@ -193,6 +201,7 @@ public class WindowTokenClient extends IWindowToken.Stub { * Similar to {@link #onConfigurationChanged(Configuration, int)}, but adds a flag to control * whether to dispatch configuration update or not. */ + @MainThread @VisibleForTesting(visibility = VisibleForTesting.Visibility.PACKAGE) public void onConfigurationChanged(Configuration newConfig, int newDisplayId, boolean shouldReportConfigChange) { @@ -218,8 +227,7 @@ public class WindowTokenClient extends IWindowToken.Stub { if (shouldReportConfigChange && context instanceof WindowContext) { final WindowContext windowContext = (WindowContext) context; - ActivityThread.currentActivityThread().getHandler().post( - () -> windowContext.dispatchConfigurationChanged(newConfig)); + windowContext.dispatchConfigurationChanged(newConfig); } // Dispatch onConfigurationChanged only if there's a significant public change to @@ -233,8 +241,7 @@ public class WindowTokenClient extends IWindowToken.Stub { if (shouldReportConfigChange && diff != 0 && context instanceof WindowProviderService) { final WindowProviderService windowProviderService = (WindowProviderService) context; - ActivityThread.currentActivityThread().getHandler().post( - () -> windowProviderService.onConfigurationChanged(newConfig)); + windowProviderService.onConfigurationChanged(newConfig); } freeTextLayoutCachesIfNeeded(diff); if (mShouldDumpConfigForIme) { @@ -256,12 +263,15 @@ public class WindowTokenClient extends IWindowToken.Stub { } } + @BinderThread @Override public void onWindowTokenRemoved() { - final Context context = mContextRef.get(); - if (context != null) { - context.destroy(); - mContextRef.clear(); - } + mHandler.post(() -> { + final Context context = mContextRef.get(); + if (context != null) { + context.destroy(); + mContextRef.clear(); + } + }); } } From 17a6606ec1bb742cdb5fc05d19251fe633a43510 Mon Sep 17 00:00:00 2001 From: Kalesh Singh Date: Tue, 7 Dec 2021 23:48:05 +0000 Subject: [PATCH 2/3] Add system_server to readtracefs group This allows system_server to search/read tracefs entries. It is needed for attaching cpu timeinstate bpf programs to tracepoints. Bug: 208892266 Bug: 209513178 Bug: 214061655 Test: libtimeinstate_test Change-Id: I4139605eb7c5277887092b3d6a3fb26bf4f8f171 Merged-In: I4139605eb7c5277887092b3d6a3fb26bf4f8f171 --- core/java/com/android/internal/os/ZygoteInit.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/java/com/android/internal/os/ZygoteInit.java b/core/java/com/android/internal/os/ZygoteInit.java index 0f26f57e21559..89b33590ab35c 100644 --- a/core/java/com/android/internal/os/ZygoteInit.java +++ b/core/java/com/android/internal/os/ZygoteInit.java @@ -783,7 +783,7 @@ public class ZygoteInit { "--setuid=1000", "--setgid=1000", "--setgroups=1001,1002,1003,1004,1005,1006,1007,1008,1009,1010,1018,1021,1023," - + "1024,1032,1065,3001,3002,3003,3006,3007,3009,3010,3011", + + "1024,1032,1065,3001,3002,3003,3006,3007,3009,3010,3011,3012", "--capabilities=" + capabilities + "," + capabilities, "--nice-name=system_server", "--runtime-args", From 09e0a6e823a77a6fd0d1b5a1e0d16880f8b46e56 Mon Sep 17 00:00:00 2001 From: Shivam Agrawal Date: Wed, 12 Jan 2022 15:09:58 -0500 Subject: [PATCH 3/3] Send Task Fragment Info Update When Task Invisible... ...only when the Task has no more running activities. Send a TaskFragment info changed callback if the callback is for the last activities to finish in a Task so that the TaskFragmentOrganizer can delete this TaskFragment. Otherwise, the Task may be removed before it becomes visible again to send this callback because it no longer has activities. As a result, the organizer will never get this info changed event and will not delete the TaskFragment because the organizer thinks the TaskFragment still has running activities. Bug: b/214090984 Test: atest TaskFragmentOrganizerControllerTest Test: atest ActivityEmbeddingLaunchTests Change-Id: Idb58975f4bfb6c67ed6cda3dc6b7c33abd960bbd --- .../wm/TaskFragmentOrganizerController.java | 21 +++++++++- .../TaskFragmentOrganizerControllerTest.java | 38 +++++++++++++++++++ 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/services/core/java/com/android/server/wm/TaskFragmentOrganizerController.java b/services/core/java/com/android/server/wm/TaskFragmentOrganizerController.java index c7fdefc412ccd..123ca889c73e9 100644 --- a/services/core/java/com/android/server/wm/TaskFragmentOrganizerController.java +++ b/services/core/java/com/android/server/wm/TaskFragmentOrganizerController.java @@ -22,6 +22,7 @@ import static com.android.internal.protolog.ProtoLogGroup.WM_DEBUG_WINDOW_ORGANI import static com.android.server.wm.WindowOrganizerController.configurationsAreEqualForOrganizer; import android.annotation.IntDef; +import android.annotation.NonNull; import android.annotation.Nullable; import android.content.res.Configuration; import android.graphics.Rect; @@ -497,6 +498,23 @@ public class TaskFragmentOrganizerController extends ITaskFragmentOrganizerContr return null; } + private boolean shouldSendEventWhenTaskInvisible(@NonNull Task task, + @NonNull PendingTaskFragmentEvent event) { + final TaskFragmentOrganizerState state = + mTaskFragmentOrganizerState.get(event.mTaskFragmentOrg.asBinder()); + final TaskFragmentInfo lastInfo = state.mLastSentTaskFragmentInfos.get(event.mTaskFragment); + final TaskFragmentInfo info = event.mTaskFragment.getTaskFragmentInfo(); + // Send an info changed callback if this event is for the last activities to finish in a + // Task so that the {@link TaskFragmentOrganizer} can delete this TaskFragment. Otherwise, + // the Task may be removed before it becomes visible again to send this event because it no + // longer has activities. As a result, the organizer will never get this info changed event + // and will not delete the TaskFragment because the organizer thinks the TaskFragment still + // has running activities. + return event.mEventType == PendingTaskFragmentEvent.EVENT_INFO_CHANGED + && task.topRunningActivity() == null && lastInfo != null + && lastInfo.getRunningActivityCount() > 0 && info.getRunningActivityCount() == 0; + } + void dispatchPendingEvents() { if (mAtmService.mWindowManager.mWindowPlacerLocked.isLayoutDeferred() || mPendingTaskFragmentEvents.isEmpty()) { @@ -510,7 +528,8 @@ public class TaskFragmentOrganizerController extends ITaskFragmentOrganizerContr final PendingTaskFragmentEvent event = mPendingTaskFragmentEvents.get(i); final Task task = event.mTaskFragment != null ? event.mTaskFragment.getTask() : null; if (task != null && (task.lastActiveTime <= event.mDeferTime - || !isTaskVisible(task, visibleTasks, invisibleTasks))) { + || !(isTaskVisible(task, visibleTasks, invisibleTasks) + || shouldSendEventWhenTaskInvisible(task, event)))) { // Defer sending events to the TaskFragment until the host task is active again. event.mDeferTime = task.lastActiveTime; continue; diff --git a/services/tests/wmtests/src/com/android/server/wm/TaskFragmentOrganizerControllerTest.java b/services/tests/wmtests/src/com/android/server/wm/TaskFragmentOrganizerControllerTest.java index dcaf9d7ae4342..f8c7207cefa70 100644 --- a/services/tests/wmtests/src/com/android/server/wm/TaskFragmentOrganizerControllerTest.java +++ b/services/tests/wmtests/src/com/android/server/wm/TaskFragmentOrganizerControllerTest.java @@ -21,14 +21,17 @@ import static com.android.dx.mockito.inline.extended.ExtendedMockito.spyOn; import static com.android.server.wm.testing.Assert.assertThrows; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.clearInvocations; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; +import static org.mockito.Mockito.reset; import static org.mockito.Mockito.verify; import android.content.Intent; @@ -471,6 +474,7 @@ public class TaskFragmentOrganizerControllerTest extends WindowTestsBase { final TaskFragment taskFragment = new TaskFragmentBuilder(mAtm) .setParentTask(task) .setOrganizer(mOrganizer) + .setFragmentToken(mFragmentToken) .build(); // Mock the task to invisible @@ -485,4 +489,38 @@ public class TaskFragmentOrganizerControllerTest extends WindowTestsBase { // Verifies that event was not sent verify(mOrganizer, never()).onTaskFragmentInfoChanged(any()); } + + /** + * Tests that a task fragment info changed event is still sent if the task is invisible only + * when the info changed event is because of the last activity in a task finishing. + */ + @Test + public void testLastPendingTaskFragmentInfoChangedEventOfInvisibleTaskSent() { + // Create a TaskFragment with an activity, all within a parent task + final TaskFragment taskFragment = new TaskFragmentBuilder(mAtm) + .setOrganizer(mOrganizer) + .setFragmentToken(mFragmentToken) + .setCreateParentTask() + .createActivityCount(1) + .build(); + final Task parentTask = taskFragment.getTask(); + final ActivityRecord activity = taskFragment.getTopNonFinishingActivity(); + assertTrue(parentTask.shouldBeVisible(null)); + + // Dispatch pending info changed event from creating the activity + mController.registerOrganizer(mIOrganizer); + taskFragment.mTaskFragmentAppearedSent = true; + mController.onTaskFragmentInfoChanged(mIOrganizer, taskFragment); + mController.dispatchPendingEvents(); + + // Finish the activity and verify that the task is invisible + activity.finishing = true; + assertFalse(parentTask.shouldBeVisible(null)); + + // Verify the info changed callback still occurred despite the task being invisible + reset(mOrganizer); + mController.onTaskFragmentInfoChanged(mIOrganizer, taskFragment); + mController.dispatchPendingEvents(); + verify(mOrganizer).onTaskFragmentInfoChanged(any()); + } }