Instantiate InputMethodManager for each display (2nd try)
InputMethodManager has been a per-process singleton object. In order to support behavior changes for multi-display support in Android Q, however, InputMethodManager now needs to be per-display objects. With this CL, context.getSystemService(InputMethodManager.class) will start returning per-display InputMethodManager (IMM) instance. Why? There are two major reasons. 1. To support per-display focused window. 2. To support more simplified API for multi-session IME. Currently per-process InputMethodManager instance directly receives callback from ViewRootImpl upon windowFocusChanged, then it keeps track of which Window is focused by storing its root view into InputMethodManager#mCurRootView. This design assumes that (within the same process) at most one Window can have window focus, which is no longer true once we start supporting per-display focused window (Bug 111361570). Why we need to do this to support per-display focused window: For traditional non multi-session IME cases (e.g. apps that use Virtual Display APIs on phones), internal state of IMM can be easily messed up once the system starts sending per-display windowFocusChanged events to the same process, because IMM still doesn't know that now each display has focused window. It is hard to precisely predict what kind of issues we would see simply because such a use case is most likely not expected in the original design. Why we need to do this for multi-session IME: For multi-session IME scenarios, in addition to the above concern in InputMethodManager, the current design allows at most one IME session per process. This means that if a process X is showing Activities to 3 different displays, only one Activity can interact with the multi-session IME at the same time. If we do not change the current design, the only way to work around is to ask app developers to explicitly use different processes for each Activity, which may require a lot of work (e.g. SharedPreference is not optimized for multi-process use cases). This would also make multi-session IME development complicated because the IME cannot know on which display the IME is interacting until startInputOrWindowGainedFocus() is actually called, and needs to do all the preparation and cleanup tasks whenever startInputOrWindowGainedFocus() is called for a different display than it's currently interacting with. Alternative solutions considered: Another possible approach is to update InputMethodManager singleton to be able to maintain multiple mCurRootView and mServedView for each display. This approach was abandoned because those fields and methods are already marked as @UnsupportedAppUsage. I concluded that touching @UnsupportedAppUsage things would have bigger compatibility risks than per-display instance model. Implementation note: * Public APIs in IMM that take View instance as the first parameter will verify whether the given View and IMM are associated with the same display ID or not. If there is a display ID mismatch, such an API call will be automatically forwarded to the correct IMM instance IMM with a clear warning in logcat which tells that app developers should use the correct IMM instance to avoid unnecessary performance overhead. * As a general rule, system server process cannot trust display ID reported from applications. In order to enable IMMS to verify the reported display ID, this CL also exposes display ID verification logic from WMS to other system components via WindowManagerInternal. * isInputMethodClientFocus() in WindowManagerService (WMS) is updated to use top-focused-display to determine whether a given IME client has IME focus or not. This is now necessary because with a recent change [1] each display can have focused window. The previous logic to check all the displays that belong to the given pid/uid [2] no longer makes sense. * Currently per-display InputMethodManager instances will not be garbage collected because InputMethodManager#sInstanceMap keeps holding strong references to them. Freeing those instances is technically possible, but we need to be careful because multiple processes (app, system, IME) are involved and at least system process has a strict verification logic that lets the calling process crash with SecurityException. We need to carefully implement such a cleanup logic to avoid random process crash due to race condition. Bug 116699479 will take care of this task. Also to make sure that the performance regression (Bug 117434607) we observed after my initial attempt [3] no longer exists, here are the benchmark results with and without this CL. testExpandNotificationsLatency on taimen-userdebug without this CL: results=[55, 46, 61, 67, 50, 48, 57, 50, 55, 63] min:46.0, max:67.0, avg:55.2, median:55.0, std_dev:6.539 with this CL: results=[45, 55, 58, 57, 47, 60, 59, 60, 56, 53] min:45.0, max:60.0, avg:55.0, median:56.5, std_dev:4.980 [1]: I776cabaeaf41ff4240f504fb1430d3e40892023d 1e5b10a2 [2]: I8da315936caebdc8b2c16cff4e24192c06743251 90120a8b [3]: I7242e765426353672823fcc8277f20ac361930d7 c53d78e9 Bug: 111364446 Fix: 115893206 Test: atest ActivityManagerMultiDisplayTests Test: atest CtsInputMethodTestCases CtsInputMethodServiceHostTestCases Test: atest FrameworksCoreTests:android.view.inputmethod.InputMethodManagerTest Test: No perf regression in LatencyTests#testExpandNotificationsLatency() Change-Id: I78ad7cccb9586474c83f7e2f90c0bcabb221c47b
Loading
Please register or sign in to comment