From 1b779384bc311a02d26df88bf09a25fde781e1a8 Mon Sep 17 00:00:00 2001 From: Michael Klimushyn Date: Mon, 9 Sep 2019 14:47:09 -0700 Subject: [PATCH 1/6] Enable platform view keyboard input on Android Q Naively embedded platform views on Android were never able to receive keyboard input, because they were never focusable. So far we've worked around the limiation by hooking into InputMethodManager and proxying the InputConnection from a focused window over to the embeded view. Android Q changed InputMethodManager to be instanced per display instead of a singleton. Because of this our proxy hook was never being called, since it was being set up on a different instance of IMM than was being used in the virtual display. Update `SingleViewPresentation` to store the IMM from the focused window and return it whenever there are any calls to `INPUT_METHOD_SERVICE`. This hooks our proxy back into place for the embedded view in the virtual display. This restores the functionality of our workaround from previous versions. Unfortunately there's still a lot of noisy error logs from IMM here. It can tell that the IMM has a different displayId than what it's expecting from the window. This also updates the unit tests to support SDK=27. SDK 16 doesn't have DisplayManager, so there were NPEs attempting to instantiate the class under test. --- DEPS | 2 +- shell/platform/android/BUILD.gn | 1 + .../platform/SingleViewPresentation.java | 56 +++++++++++++++---- shell/platform/android/test/README.md | 4 +- .../test/io/flutter/FlutterTestSuite.java | 21 +++---- .../platform/SingleViewPresentationTest.java | 51 +++++++++++++++++ 6 files changed, 111 insertions(+), 24 deletions(-) create mode 100644 shell/platform/android/test/io/flutter/plugin/platform/SingleViewPresentationTest.java diff --git a/DEPS b/DEPS index fda68d9893df5..887046120fe2b 100644 --- a/DEPS +++ b/DEPS @@ -483,7 +483,7 @@ deps = { 'packages': [ { 'package': 'flutter/android/robolectric_bundle', - 'version': 'last_updated:2019-08-02T16:01:27-0700' + 'version': 'last_updated:2019-09-09T16:47:38-0700' } ], 'condition': 'download_android_deps', diff --git a/shell/platform/android/BUILD.gn b/shell/platform/android/BUILD.gn index 71df245f06012..522fafd631b74 100644 --- a/shell/platform/android/BUILD.gn +++ b/shell/platform/android/BUILD.gn @@ -416,6 +416,7 @@ action("robolectric_tests") { "test/io/flutter/embedding/engine/RenderingComponentTest.java", "test/io/flutter/embedding/engine/renderer/FlutterRendererTest.java", "test/io/flutter/embedding/engine/systemchannels/PlatformChannelTest.java", + "test/io/flutter/plugin/platform/SingleViewPresentationTest.java", "test/io/flutter/util/PreconditionsTest.java", ] diff --git a/shell/platform/android/io/flutter/plugin/platform/SingleViewPresentation.java b/shell/platform/android/io/flutter/plugin/platform/SingleViewPresentation.java index 1d3587b6594ea..2388995d29766 100644 --- a/shell/platform/android/io/flutter/plugin/platform/SingleViewPresentation.java +++ b/shell/platform/android/io/flutter/plugin/platform/SingleViewPresentation.java @@ -13,13 +13,24 @@ import android.os.Build; import android.os.Bundle; import android.support.annotation.Keep; +import android.support.annotation.NonNull; +import android.support.annotation.Nullable; import android.util.Log; -import android.view.*; +import android.view.Display; +import android.view.Gravity; +import android.view.View; +import android.view.ViewGroup; +import android.view.WindowManager; import android.view.accessibility.AccessibilityEvent; +import android.view.inputmethod.InputMethodManager; import android.widget.FrameLayout; -import java.lang.reflect.*; +import java.lang.reflect.InvocationHandler; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.lang.reflect.Proxy; +import static android.content.Context.INPUT_METHOD_SERVICE; import static android.content.Context.WINDOW_SERVICE; import static android.view.View.OnFocusChangeListener; @@ -99,7 +110,7 @@ public SingleViewPresentation( Object createParams, OnFocusChangeListener focusChangeListener ) { - super(outerContext, display); + super(wrapContext(outerContext, /*windowManagerHandler=*/null), display); this.viewFactory = viewFactory; this.accessibilityEventsDelegate = accessibilityEventsDelegate; this.viewId = viewId; @@ -128,7 +139,7 @@ public SingleViewPresentation( OnFocusChangeListener focusChangeListener, boolean startFocused ) { - super(outerContext, display); + super(wrapContext(outerContext, /*windowManagerHandler=*/null), display); this.accessibilityEventsDelegate = accessibilityEventsDelegate; viewFactory = null; this.state = state; @@ -140,6 +151,11 @@ public SingleViewPresentation( this.startFocused = startFocused; } + private static Context wrapContext(Context context, @Nullable WindowManagerHandler windowManagerHandler) { + InputMethodManager inputMethodManager = (InputMethodManager) context.getSystemService(INPUT_METHOD_SERVICE); + return new PresentationContext(context, inputMethodManager, windowManagerHandler); + } + @Override protected void onCreate(Bundle savedInstanceState) { super.onCreate(savedInstanceState); @@ -154,7 +170,7 @@ protected void onCreate(Bundle savedInstanceState) { } container = new FrameLayout(getContext()); - PresentationContext context = new PresentationContext(getContext(), state.windowManagerHandler); + Context context = wrapContext(getContext(), state.windowManagerHandler); if (state.platformView == null) { state.platformView = viewFactory.create(context, viewId, createParams); @@ -236,25 +252,43 @@ private static int atMost(int measureSpec) { } /** - * Proxies a Context replacing the WindowManager with our custom instance. + * Proxies a Context replacing the WindowManager and InputMethodManager with our custom instance. */ static class PresentationContext extends ContextWrapper { - private WindowManager windowManager; - private final WindowManagerHandler windowManagerHandler; - - PresentationContext(Context base, WindowManagerHandler windowManagerHandler) { + private @Nullable + final WindowManagerHandler windowManagerHandler; + private @NonNull + final InputMethodManager inputMethodManager; + private @Nullable + WindowManager windowManager; + + /** + * Return the given {@code inputMethodManager} and {@code windowManagerHandler} as system + * services from this context. Does not override {@code windowManagerHandler} if it's null. + */ + PresentationContext(Context base, @NonNull InputMethodManager inputMethodManager, @Nullable WindowManagerHandler windowManagerHandler) { super(base); this.windowManagerHandler = windowManagerHandler; + this.inputMethodManager = inputMethodManager; } @Override public Object getSystemService(String name) { - if (WINDOW_SERVICE.equals(name)) { + if (WINDOW_SERVICE.equals(name) && windowManagerHandler != null) { return getWindowManager(); } + if (INPUT_METHOD_SERVICE.equals(name)) { + return inputMethodManager; + } return super.getSystemService(name); } + @Override + public Context createDisplayContext(Display display) { + Context displayContext = super.createDisplayContext(display); + return new PresentationContext(displayContext, inputMethodManager, windowManagerHandler); + } + private WindowManager getWindowManager() { if (windowManager == null) { windowManager = windowManagerHandler.getWindowManager(); diff --git a/shell/platform/android/test/README.md b/shell/platform/android/test/README.md index 88f7fc5c59d33..f17c2fd5b42c4 100644 --- a/shell/platform/android/test/README.md +++ b/shell/platform/android/test/README.md @@ -64,13 +64,13 @@ Once you've uploaded the new version, also make sure to tag it with the updated timestamp and robolectric version (most likely still 3.8, unless you've migrated all the packages to 4+). - $ cipd set-tag flutter/android/robolectric --version= -tag=last_updated: + $ cipd set-tag flutter/android/robolectric_bundle --version= -tag=last_updated: Example of a last-updated timestamp: 2019-07-29T15:27:42-0700 You can generate the same date format with `date +%Y-%m-%dT%T%z`. - $ cipd set-tag flutter/android/robolectric --version= -tag=robolectric_version: + $ cipd set-tag flutter/android/robolectric_bundle --version= -tag=robolectric_version: You can run `cipd describe flutter/android/robolectric_bundle --version=` to verify. You should see: diff --git a/shell/platform/android/test/io/flutter/FlutterTestSuite.java b/shell/platform/android/test/io/flutter/FlutterTestSuite.java index 036cbd56173b9..ca2a9ab5d24ca 100644 --- a/shell/platform/android/test/io/flutter/FlutterTestSuite.java +++ b/shell/platform/android/test/io/flutter/FlutterTestSuite.java @@ -8,28 +8,29 @@ import org.junit.runners.Suite; import org.junit.runners.Suite.SuiteClasses; -import io.flutter.embedding.android.FlutterActivityAndFragmentDelegateTest; import io.flutter.embedding.android.FlutterActivityTest; import io.flutter.embedding.android.FlutterFragmentTest; import io.flutter.embedding.engine.FlutterEngineCacheTest; -import io.flutter.embedding.engine.systemchannels.PlatformChannelTest; +import io.flutter.embedding.engine.FlutterJNITest; import io.flutter.embedding.engine.RenderingComponentTest; import io.flutter.embedding.engine.renderer.FlutterRendererTest; +import io.flutter.embedding.engine.systemchannels.PlatformChannelTest; +import io.flutter.plugin.platform.SingleViewPresentationTest; import io.flutter.util.PreconditionsTest; -import io.flutter.embedding.engine.FlutterJNITest; @RunWith(Suite.class) @SuiteClasses({ - PreconditionsTest.class, - SmokeTest.class, - FlutterActivityTest.class, - FlutterFragmentTest.class, // FlutterActivityAndFragmentDelegateTest.class, TODO(mklim): Fix and re-enable this + FlutterActivityTest.class, FlutterEngineCacheTest.class, + FlutterFragmentTest.class, FlutterJNITest.class, - RenderingComponentTest.class, FlutterRendererTest.class, - PlatformChannelTest.class + PlatformChannelTest.class, + PreconditionsTest.class, + RenderingComponentTest.class, + SingleViewPresentationTest.class, + SmokeTest.class, }) /** Runs all of the unit tests listed in the {@code @SuiteClasses} annotation. */ -public class FlutterTestSuite {} +public class FlutterTestSuite { } diff --git a/shell/platform/android/test/io/flutter/plugin/platform/SingleViewPresentationTest.java b/shell/platform/android/test/io/flutter/plugin/platform/SingleViewPresentationTest.java new file mode 100644 index 0000000000000..ae40331814268 --- /dev/null +++ b/shell/platform/android/test/io/flutter/plugin/platform/SingleViewPresentationTest.java @@ -0,0 +1,51 @@ +package io.flutter.plugin.platform; + +import android.content.Context; +import android.hardware.display.DisplayManager; +import android.view.inputmethod.InputMethodManager; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.robolectric.RobolectricTestRunner; +import org.robolectric.RuntimeEnvironment; +import org.robolectric.annotation.Config; +import org.robolectric.shadows.ShadowDisplay; +import org.robolectric.shadows.ShadowDisplayManager; +import org.robolectric.shadows.ShadowInputMethodManager; + +import static junit.framework.Assert.assertEquals; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.when; + +@Config(manifest = Config.NONE, shadows = {ShadowInputMethodManager.class, ShadowDisplayManager.class, ShadowDisplay.class}, sdk = 27) +@RunWith(RobolectricTestRunner.class) +public class SingleViewPresentationTest { + @Test + public void returnsOuterContextInputMethodManager() { + // There's a bug in Android Q caused by the IMM being instanced per display. + // https://github.com/flutter/flutter/issues/38375. We need the context returned by + // SingleViewPresentation to be consistent from its instantiation instead of defaulting to + // what the system would have returned at call time. + + // It's not possible to set up the exact same conditions as the unit test in the bug here, + // but we can make sure that we're wrapping the Context passed in at instantiation time and + // returning the same InputMethodManager from it. This test passes in a Spy context instance + // that initially returns a mock. Without the bugfix this test falls back to Robolectric's + // system service instead of the spy's and fails. + + // Create an SVP under text with a Context that returns a local IMM mock. + Context context = spy(RuntimeEnvironment.application); + InputMethodManager expected = mock(InputMethodManager.class); + when(context.getSystemService(Context.INPUT_METHOD_SERVICE)).thenReturn(expected); + DisplayManager dm = (DisplayManager) context.getSystemService(Context.DISPLAY_SERVICE); + SingleViewPresentation svp = new SingleViewPresentation(context, dm.getDisplay(0), null, null, null, false); + + // Get the IMM from the SVP's context. + InputMethodManager actual = (InputMethodManager) svp.getContext().getSystemService(Context.INPUT_METHOD_SERVICE); + + // This should be the mocked instance from construction, not the IMM from the greater + // Android OS (or Robolectric's shadow, in this case). + assertEquals(expected, actual); + } +} From 85ec94417f8076b5605971f900d628d056714a3b Mon Sep 17 00:00:00 2001 From: Michael Klimushyn Date: Mon, 9 Sep 2019 17:19:16 -0700 Subject: [PATCH 2/6] Fix lint. --- .../io/flutter/plugin/platform/SingleViewPresentationTest.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/shell/platform/android/test/io/flutter/plugin/platform/SingleViewPresentationTest.java b/shell/platform/android/test/io/flutter/plugin/platform/SingleViewPresentationTest.java index ae40331814268..61867dbe35a07 100644 --- a/shell/platform/android/test/io/flutter/plugin/platform/SingleViewPresentationTest.java +++ b/shell/platform/android/test/io/flutter/plugin/platform/SingleViewPresentationTest.java @@ -1,5 +1,6 @@ package io.flutter.plugin.platform; +import android.annotation.TargetApi; import android.content.Context; import android.hardware.display.DisplayManager; import android.view.inputmethod.InputMethodManager; @@ -20,6 +21,7 @@ @Config(manifest = Config.NONE, shadows = {ShadowInputMethodManager.class, ShadowDisplayManager.class, ShadowDisplay.class}, sdk = 27) @RunWith(RobolectricTestRunner.class) +@TargetApi(27) public class SingleViewPresentationTest { @Test public void returnsOuterContextInputMethodManager() { From 2dbacdc042a2726f724aa85c6c7bb6561a056d5c Mon Sep 17 00:00:00 2001 From: Michael Klimushyn Date: Tue, 10 Sep 2019 12:46:45 -0700 Subject: [PATCH 3/6] Review feedback, extra test. --- .../platform/SingleViewPresentation.java | 29 +++++++++++-------- .../platform/SingleViewPresentationTest.java | 22 +++++++++++++- 2 files changed, 38 insertions(+), 13 deletions(-) diff --git a/shell/platform/android/io/flutter/plugin/platform/SingleViewPresentation.java b/shell/platform/android/io/flutter/plugin/platform/SingleViewPresentation.java index 2388995d29766..cb69e89180c38 100644 --- a/shell/platform/android/io/flutter/plugin/platform/SingleViewPresentation.java +++ b/shell/platform/android/io/flutter/plugin/platform/SingleViewPresentation.java @@ -110,7 +110,7 @@ public SingleViewPresentation( Object createParams, OnFocusChangeListener focusChangeListener ) { - super(wrapContext(outerContext, /*windowManagerHandler=*/null), display); + super(new PresentationContext(outerContext, /*windowManagerHandler=*/null), display); this.viewFactory = viewFactory; this.accessibilityEventsDelegate = accessibilityEventsDelegate; this.viewId = viewId; @@ -139,7 +139,7 @@ public SingleViewPresentation( OnFocusChangeListener focusChangeListener, boolean startFocused ) { - super(wrapContext(outerContext, /*windowManagerHandler=*/null), display); + super(new PresentationContext(outerContext, /*windowManagerHandler=*/null), display); this.accessibilityEventsDelegate = accessibilityEventsDelegate; viewFactory = null; this.state = state; @@ -151,11 +151,6 @@ public SingleViewPresentation( this.startFocused = startFocused; } - private static Context wrapContext(Context context, @Nullable WindowManagerHandler windowManagerHandler) { - InputMethodManager inputMethodManager = (InputMethodManager) context.getSystemService(INPUT_METHOD_SERVICE); - return new PresentationContext(context, inputMethodManager, windowManagerHandler); - } - @Override protected void onCreate(Bundle savedInstanceState) { super.onCreate(savedInstanceState); @@ -170,7 +165,10 @@ protected void onCreate(Bundle savedInstanceState) { } container = new FrameLayout(getContext()); - Context context = wrapContext(getContext(), state.windowManagerHandler); + + // Our base mContext has already been wrapped with an IMM cache at instantiation time, but + // we want to wrap it again here to also return state.windowManagerHandler. + Context context = new PresentationContext(getContext(), state.windowManagerHandler); if (state.platformView == null) { state.platformView = viewFactory.create(context, viewId, createParams); @@ -263,13 +261,20 @@ static class PresentationContext extends ContextWrapper { WindowManager windowManager; /** - * Return the given {@code inputMethodManager} and {@code windowManagerHandler} as system - * services from this context. Does not override {@code windowManagerHandler} if it's null. + * Return the given {@code windowManagerHandler} as a system service from this context. + * Caches the current {@link InputMethodManager} for {@code base} at instantiation time and + * consistently returns it, too. + *

+ * Does not override {@code windowManagerHandler} if it's null. */ - PresentationContext(Context base, @NonNull InputMethodManager inputMethodManager, @Nullable WindowManagerHandler windowManagerHandler) { + PresentationContext(Context base, @Nullable WindowManagerHandler windowManagerHandler) { + this(base, /*inputMethodManager=*/null, windowManagerHandler); + } + + private PresentationContext(Context base, @Nullable InputMethodManager inputMethodManager, @Nullable WindowManagerHandler windowManagerHandler) { super(base); this.windowManagerHandler = windowManagerHandler; - this.inputMethodManager = inputMethodManager; + this.inputMethodManager = (inputMethodManager != null) ? inputMethodManager : (InputMethodManager) base.getSystemService(INPUT_METHOD_SERVICE); } @Override diff --git a/shell/platform/android/test/io/flutter/plugin/platform/SingleViewPresentationTest.java b/shell/platform/android/test/io/flutter/plugin/platform/SingleViewPresentationTest.java index 61867dbe35a07..cb70e89e68a21 100644 --- a/shell/platform/android/test/io/flutter/plugin/platform/SingleViewPresentationTest.java +++ b/shell/platform/android/test/io/flutter/plugin/platform/SingleViewPresentationTest.java @@ -3,6 +3,7 @@ import android.annotation.TargetApi; import android.content.Context; import android.hardware.display.DisplayManager; +import android.view.Display; import android.view.inputmethod.InputMethodManager; import org.junit.Test; @@ -36,7 +37,7 @@ public void returnsOuterContextInputMethodManager() { // that initially returns a mock. Without the bugfix this test falls back to Robolectric's // system service instead of the spy's and fails. - // Create an SVP under text with a Context that returns a local IMM mock. + // Create an SVP under test with a Context that returns a local IMM mock. Context context = spy(RuntimeEnvironment.application); InputMethodManager expected = mock(InputMethodManager.class); when(context.getSystemService(Context.INPUT_METHOD_SERVICE)).thenReturn(expected); @@ -50,4 +51,23 @@ public void returnsOuterContextInputMethodManager() { // Android OS (or Robolectric's shadow, in this case). assertEquals(expected, actual); } + + @Test + public void returnsOuterContextInputMethodManager_createDisplayContext() { + // The IMM should also persist across display contexts created from the base context. + + // Create an SVP under test with a Context that returns a local IMM mock. + Context context = spy(RuntimeEnvironment.application); + InputMethodManager expected = mock(InputMethodManager.class); + when(context.getSystemService(Context.INPUT_METHOD_SERVICE)).thenReturn(expected); + Display display = ((DisplayManager) context.getSystemService(Context.DISPLAY_SERVICE)).getDisplay(0); + SingleViewPresentation svp = new SingleViewPresentation(context, display, null, null, null, false); + + // Get the IMM from the SVP's context. + InputMethodManager actual = (InputMethodManager) svp.getContext().createDisplayContext(display).getSystemService(Context.INPUT_METHOD_SERVICE); + + // This should be the mocked instance from construction, not the IMM from the greater + // Android OS (or Robolectric's shadow, in this case). + assertEquals(expected, actual); + } } From f77c424fe5ce97df69a86ba11e1d292cb80cb0b7 Mon Sep 17 00:00:00 2001 From: Michael Klimushyn Date: Tue, 10 Sep 2019 13:37:06 -0700 Subject: [PATCH 4/6] Split out ContextWrapper instances into classes --- .../platform/SingleViewPresentation.java | 58 ++++++++++--------- 1 file changed, 32 insertions(+), 26 deletions(-) diff --git a/shell/platform/android/io/flutter/plugin/platform/SingleViewPresentation.java b/shell/platform/android/io/flutter/plugin/platform/SingleViewPresentation.java index cb69e89180c38..dfec88fd3b271 100644 --- a/shell/platform/android/io/flutter/plugin/platform/SingleViewPresentation.java +++ b/shell/platform/android/io/flutter/plugin/platform/SingleViewPresentation.java @@ -110,7 +110,7 @@ public SingleViewPresentation( Object createParams, OnFocusChangeListener focusChangeListener ) { - super(new PresentationContext(outerContext, /*windowManagerHandler=*/null), display); + super(new ImmContext(outerContext), display); this.viewFactory = viewFactory; this.accessibilityEventsDelegate = accessibilityEventsDelegate; this.viewId = viewId; @@ -139,7 +139,7 @@ public SingleViewPresentation( OnFocusChangeListener focusChangeListener, boolean startFocused ) { - super(new PresentationContext(outerContext, /*windowManagerHandler=*/null), display); + super(new ImmContext(outerContext), display); this.accessibilityEventsDelegate = accessibilityEventsDelegate; viewFactory = null; this.state = state; @@ -249,39 +249,22 @@ private static int atMost(int measureSpec) { } } - /** - * Proxies a Context replacing the WindowManager and InputMethodManager with our custom instance. - */ - static class PresentationContext extends ContextWrapper { - private @Nullable - final WindowManagerHandler windowManagerHandler; + /** Answers calls for {@link InputMethodManager} with an instance cached at creation time. */ + private static class ImmContext extends ContextWrapper { private @NonNull final InputMethodManager inputMethodManager; - private @Nullable - WindowManager windowManager; - /** - * Return the given {@code windowManagerHandler} as a system service from this context. - * Caches the current {@link InputMethodManager} for {@code base} at instantiation time and - * consistently returns it, too. - *

- * Does not override {@code windowManagerHandler} if it's null. - */ - PresentationContext(Context base, @Nullable WindowManagerHandler windowManagerHandler) { - this(base, /*inputMethodManager=*/null, windowManagerHandler); + ImmContext(Context base) { + this(base, /*inputMethodManager=*/null); } - private PresentationContext(Context base, @Nullable InputMethodManager inputMethodManager, @Nullable WindowManagerHandler windowManagerHandler) { + private ImmContext(Context base, @Nullable InputMethodManager inputMethodManager) { super(base); - this.windowManagerHandler = windowManagerHandler; - this.inputMethodManager = (inputMethodManager != null) ? inputMethodManager : (InputMethodManager) base.getSystemService(INPUT_METHOD_SERVICE); + this.inputMethodManager = inputMethodManager != null ? inputMethodManager : (InputMethodManager) base.getSystemService(INPUT_METHOD_SERVICE); } @Override public Object getSystemService(String name) { - if (WINDOW_SERVICE.equals(name) && windowManagerHandler != null) { - return getWindowManager(); - } if (INPUT_METHOD_SERVICE.equals(name)) { return inputMethodManager; } @@ -291,7 +274,30 @@ public Object getSystemService(String name) { @Override public Context createDisplayContext(Display display) { Context displayContext = super.createDisplayContext(display); - return new PresentationContext(displayContext, inputMethodManager, windowManagerHandler); + return new ImmContext(displayContext, inputMethodManager); + } + } + + /** + * Proxies a Context replacing the WindowManager and InputMethodManager with our custom instance. + */ + private static class PresentationContext extends ImmContext { + private @NonNull + final WindowManagerHandler windowManagerHandler; + private @Nullable + WindowManager windowManager; + + PresentationContext(Context base, @NonNull WindowManagerHandler windowManagerHandler) { + super(base); + this.windowManagerHandler = windowManagerHandler; + } + + @Override + public Object getSystemService(String name) { + if (WINDOW_SERVICE.equals(name)) { + return getWindowManager(); + } + return super.getSystemService(name); } private WindowManager getWindowManager() { From 814783ae0d27b5a96b75ebf87ad9cbb3ce891565 Mon Sep 17 00:00:00 2001 From: Michael Klimushyn Date: Wed, 11 Sep 2019 15:41:35 -0700 Subject: [PATCH 5/6] Review feedback --- .../plugin/platform/SingleViewPresentation.java | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/shell/platform/android/io/flutter/plugin/platform/SingleViewPresentation.java b/shell/platform/android/io/flutter/plugin/platform/SingleViewPresentation.java index dfec88fd3b271..129fc1b1f35cd 100644 --- a/shell/platform/android/io/flutter/plugin/platform/SingleViewPresentation.java +++ b/shell/platform/android/io/flutter/plugin/platform/SingleViewPresentation.java @@ -250,6 +250,10 @@ private static int atMost(int measureSpec) { } /** Answers calls for {@link InputMethodManager} with an instance cached at creation time. */ + // TODO(mklim): This caches the IMM at construction time and won't pick up any changes. In rare + // cases where the FlutterView changes windows this will return an outdated instance. This + // should be fixed to instead defer returning the IMM to something that know's FlutterView's + // true Context. private static class ImmContext extends ContextWrapper { private @NonNull final InputMethodManager inputMethodManager; @@ -281,15 +285,22 @@ public Context createDisplayContext(Display display) { /** * Proxies a Context replacing the WindowManager and InputMethodManager with our custom instance. */ - private static class PresentationContext extends ImmContext { + // TODO(mklim): This caches the IMM at construction time and won't pick up any changes. In rare + // cases where the FlutterView changes windows this will return an outdated instance. This + // should be fixed to instead defer returning the IMM to something that know's FlutterView's + // true Context. + private static class PresentationContext extends ContextWrapper { private @NonNull final WindowManagerHandler windowManagerHandler; private @Nullable WindowManager windowManager; + private @NonNull + final InputMethodManager inputMethodManager; PresentationContext(Context base, @NonNull WindowManagerHandler windowManagerHandler) { super(base); this.windowManagerHandler = windowManagerHandler; + this.inputMethodManager = (InputMethodManager) base.getSystemService(INPUT_METHOD_SERVICE); } @Override @@ -297,6 +308,9 @@ public Object getSystemService(String name) { if (WINDOW_SERVICE.equals(name)) { return getWindowManager(); } + if (INPUT_METHOD_SERVICE.equals(name)) { + return inputMethodManager; + } return super.getSystemService(name); } From f7487ac1ace237f47f37d671a55870d2bda18f46 Mon Sep 17 00:00:00 2001 From: Michael Klimushyn Date: Wed, 11 Sep 2019 16:38:26 -0700 Subject: [PATCH 6/6] Whoops. --- .../plugin/platform/SingleViewPresentation.java | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/shell/platform/android/io/flutter/plugin/platform/SingleViewPresentation.java b/shell/platform/android/io/flutter/plugin/platform/SingleViewPresentation.java index 129fc1b1f35cd..cde4b0c49fffc 100644 --- a/shell/platform/android/io/flutter/plugin/platform/SingleViewPresentation.java +++ b/shell/platform/android/io/flutter/plugin/platform/SingleViewPresentation.java @@ -282,9 +282,7 @@ public Context createDisplayContext(Display display) { } } - /** - * Proxies a Context replacing the WindowManager and InputMethodManager with our custom instance. - */ + /** Proxies a Context replacing the WindowManager with our custom instance. */ // TODO(mklim): This caches the IMM at construction time and won't pick up any changes. In rare // cases where the FlutterView changes windows this will return an outdated instance. This // should be fixed to instead defer returning the IMM to something that know's FlutterView's @@ -294,13 +292,10 @@ private static class PresentationContext extends ContextWrapper { final WindowManagerHandler windowManagerHandler; private @Nullable WindowManager windowManager; - private @NonNull - final InputMethodManager inputMethodManager; PresentationContext(Context base, @NonNull WindowManagerHandler windowManagerHandler) { super(base); this.windowManagerHandler = windowManagerHandler; - this.inputMethodManager = (InputMethodManager) base.getSystemService(INPUT_METHOD_SERVICE); } @Override @@ -308,9 +303,6 @@ public Object getSystemService(String name) { if (WINDOW_SERVICE.equals(name)) { return getWindowManager(); } - if (INPUT_METHOD_SERVICE.equals(name)) { - return inputMethodManager; - } return super.getSystemService(name); }