Fix NPE and remove global focus listener when tearing down the view (… (#27671)
* Fix NPE and remove global focus listener when tearing down the view (#27655)
* Update ci.yaml to enable the tests
Co-authored-by: Emmanuel Garcia <egarciad@google.com>
diff --git a/.ci.yaml b/.ci.yaml
index 869db0a..6a1cece 100644
--- a/.ci.yaml
+++ b/.ci.yaml
@@ -6,6 +6,7 @@
# More information at:
# * https://github.com/flutter/cocoon/blob/master/CI_YAML.md
enabled_branches:
+ - flutter-2.4-candidate.8
- master
- dev
- beta
diff --git a/shell/platform/android/io/flutter/embedding/engine/mutatorsstack/FlutterMutatorView.java b/shell/platform/android/io/flutter/embedding/engine/mutatorsstack/FlutterMutatorView.java
index 0097154..d46f534 100644
--- a/shell/platform/android/io/flutter/embedding/engine/mutatorsstack/FlutterMutatorView.java
+++ b/shell/platform/android/io/flutter/embedding/engine/mutatorsstack/FlutterMutatorView.java
@@ -74,23 +74,41 @@
return false;
}
+ @Nullable @VisibleForTesting ViewTreeObserver.OnGlobalFocusChangeListener activeFocusListener;
+
/**
- * Adds a focus change listener that notifies when the current view or any of its descendant views
+ * Sets a focus change listener that notifies when the current view or any of its descendant views
* have received focus.
*
- * @param focusListener The focus listener.
+ * <p>If there's an active focus listener, it will first remove the current listener, and then add
+ * the new one.
+ *
+ * @param userFocusListener A user provided focus listener.
*/
- public void addOnFocusChangeListener(@NonNull OnFocusChangeListener focusListener) {
+ public void setOnDescendantFocusChangeListener(@NonNull OnFocusChangeListener userFocusListener) {
+ unsetOnDescendantFocusChangeListener();
+
final View mutatorView = this;
- final ViewTreeObserver observer = this.getViewTreeObserver();
- if (observer.isAlive()) {
- observer.addOnGlobalFocusChangeListener(
+ final ViewTreeObserver observer = getViewTreeObserver();
+ if (observer.isAlive() && activeFocusListener == null) {
+ activeFocusListener =
new ViewTreeObserver.OnGlobalFocusChangeListener() {
@Override
public void onGlobalFocusChanged(View oldFocus, View newFocus) {
- focusListener.onFocusChange(mutatorView, childHasFocus(mutatorView));
+ userFocusListener.onFocusChange(mutatorView, childHasFocus(mutatorView));
}
- });
+ };
+ observer.addOnGlobalFocusChangeListener(activeFocusListener);
+ }
+ }
+
+ /** Unsets any active focus listener. */
+ public void unsetOnDescendantFocusChangeListener() {
+ final ViewTreeObserver observer = getViewTreeObserver();
+ if (observer.isAlive() && activeFocusListener != null) {
+ final ViewTreeObserver.OnGlobalFocusChangeListener currFocusListener = activeFocusListener;
+ activeFocusListener = null;
+ observer.removeOnGlobalFocusChangeListener(currFocusListener);
}
}
diff --git a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java
index 8d59a26..781328b 100644
--- a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java
+++ b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java
@@ -61,7 +61,7 @@
// The texture registry maintaining the textures into which the embedded views will be rendered.
private TextureRegistry textureRegistry;
- private TextInputPlugin textInputPlugin;
+ @Nullable private TextInputPlugin textInputPlugin;
// The system channel used to communicate with the framework about platform views.
private PlatformViewsChannel platformViewsChannel;
@@ -164,8 +164,8 @@
platformViews.remove(viewId);
platformView.dispose();
}
-
if (parentView != null) {
+ parentView.unsetOnDescendantFocusChangeListener();
((ViewGroup) parentView.getParent()).removeView(parentView);
platformViewParent.remove(viewId);
}
@@ -748,11 +748,11 @@
new FlutterMutatorView(
context, context.getResources().getDisplayMetrics().density, androidTouchProcessor);
- parentView.addOnFocusChangeListener(
+ parentView.setOnDescendantFocusChangeListener(
(view, hasFocus) -> {
if (hasFocus) {
platformViewsChannel.invokeViewFocused(viewId);
- } else {
+ } else if (textInputPlugin != null) {
textInputPlugin.clearPlatformViewClient(viewId);
}
});
diff --git a/shell/platform/android/test/io/flutter/embedding/engine/mutatorsstack/FlutterMutatorViewTest.java b/shell/platform/android/test/io/flutter/embedding/engine/mutatorsstack/FlutterMutatorViewTest.java
index 37b9023..9a5e46b 100644
--- a/shell/platform/android/test/io/flutter/embedding/engine/mutatorsstack/FlutterMutatorViewTest.java
+++ b/shell/platform/android/test/io/flutter/embedding/engine/mutatorsstack/FlutterMutatorViewTest.java
@@ -143,7 +143,7 @@
};
final OnFocusChangeListener focusListener = mock(OnFocusChangeListener.class);
- view.addOnFocusChangeListener(focusListener);
+ view.setOnDescendantFocusChangeListener(focusListener);
final ArgumentCaptor<ViewTreeObserver.OnGlobalFocusChangeListener> focusListenerCaptor =
ArgumentCaptor.forClass(ViewTreeObserver.OnGlobalFocusChangeListener.class);
@@ -172,7 +172,7 @@
};
final OnFocusChangeListener focusListener = mock(OnFocusChangeListener.class);
- view.addOnFocusChangeListener(focusListener);
+ view.setOnDescendantFocusChangeListener(focusListener);
final ArgumentCaptor<ViewTreeObserver.OnGlobalFocusChangeListener> focusListenerCaptor =
ArgumentCaptor.forClass(ViewTreeObserver.OnGlobalFocusChangeListener.class);
@@ -193,6 +193,61 @@
return viewTreeObserver;
}
};
- view.addOnFocusChangeListener(mock(OnFocusChangeListener.class));
+ view.setOnDescendantFocusChangeListener(mock(OnFocusChangeListener.class));
+ }
+
+ @Test
+ public void setOnDescendantFocusChangeListener_keepsSingleListener() {
+ final ViewTreeObserver viewTreeObserver = mock(ViewTreeObserver.class);
+ when(viewTreeObserver.isAlive()).thenReturn(true);
+
+ final FlutterMutatorView view =
+ new FlutterMutatorView(RuntimeEnvironment.systemContext) {
+ @Override
+ public ViewTreeObserver getViewTreeObserver() {
+ return viewTreeObserver;
+ }
+ };
+
+ assertNull(view.activeFocusListener);
+
+ view.setOnDescendantFocusChangeListener(mock(OnFocusChangeListener.class));
+ assertNotNull(view.activeFocusListener);
+
+ final ViewTreeObserver.OnGlobalFocusChangeListener activeFocusListener =
+ view.activeFocusListener;
+
+ view.setOnDescendantFocusChangeListener(mock(OnFocusChangeListener.class));
+ assertNotNull(view.activeFocusListener);
+
+ verify(viewTreeObserver, times(1)).removeOnGlobalFocusChangeListener(activeFocusListener);
+ }
+
+ @Test
+ public void unsetOnDescendantFocusChangeListener_removesActiveListener() {
+ final ViewTreeObserver viewTreeObserver = mock(ViewTreeObserver.class);
+ when(viewTreeObserver.isAlive()).thenReturn(true);
+
+ final FlutterMutatorView view =
+ new FlutterMutatorView(RuntimeEnvironment.systemContext) {
+ @Override
+ public ViewTreeObserver getViewTreeObserver() {
+ return viewTreeObserver;
+ }
+ };
+
+ assertNull(view.activeFocusListener);
+
+ view.setOnDescendantFocusChangeListener(mock(OnFocusChangeListener.class));
+ assertNotNull(view.activeFocusListener);
+
+ final ViewTreeObserver.OnGlobalFocusChangeListener activeFocusListener =
+ view.activeFocusListener;
+
+ view.unsetOnDescendantFocusChangeListener();
+ assertNull(view.activeFocusListener);
+
+ view.unsetOnDescendantFocusChangeListener();
+ verify(viewTreeObserver, times(1)).removeOnGlobalFocusChangeListener(activeFocusListener);
}
}
