[webview_flutter_android] Fixes crash when the Java `InstanceManager` was used after plugin was removed from engine (#6943)
* dont throw errors on all instance manager methods
* tests
* version bump
* change to logging a warning and ignore calls in other methods
* the
* documentation
diff --git a/packages/webview_flutter/webview_flutter_android/CHANGELOG.md b/packages/webview_flutter/webview_flutter_android/CHANGELOG.md
index d45e735..37abf3c 100644
--- a/packages/webview_flutter/webview_flutter_android/CHANGELOG.md
+++ b/packages/webview_flutter/webview_flutter_android/CHANGELOG.md
@@ -1,3 +1,7 @@
+## 3.1.3
+
+* Fixes crash when the Java `InstanceManager` was used after plugin was removed from the engine.
+
## 3.1.2
* Fixes bug where an `AndroidWebViewController` couldn't be reused with a new `WebViewWidget`.
diff --git a/packages/webview_flutter/webview_flutter_android/android/src/main/java/io/flutter/plugins/webviewflutter/InstanceManager.java b/packages/webview_flutter/webview_flutter_android/android/src/main/java/io/flutter/plugins/webviewflutter/InstanceManager.java
index fefd577..55775a9 100644
--- a/packages/webview_flutter/webview_flutter_android/android/src/main/java/io/flutter/plugins/webviewflutter/InstanceManager.java
+++ b/packages/webview_flutter/webview_flutter_android/android/src/main/java/io/flutter/plugins/webviewflutter/InstanceManager.java
@@ -6,6 +6,7 @@
import android.os.Handler;
import android.os.Looper;
+import android.util.Log;
import androidx.annotation.Nullable;
import java.lang.ref.ReferenceQueue;
import java.lang.ref.WeakReference;
@@ -35,6 +36,8 @@
// 0 <= n < 2^16.
private static final long MIN_HOST_CREATED_IDENTIFIER = 65536;
private static final long CLEAR_FINALIZED_WEAK_REFERENCES_INTERVAL = 30000;
+ private static final String TAG = "InstanceManager";
+ private static final String CLOSED_WARNING = "Method was called while the manager was closed.";
/** Interface for listening when a weak reference of an instance is removed from the manager. */
public interface FinalizationListener {
@@ -79,11 +82,15 @@
*
* @param identifier the identifier paired to an instance.
* @param <T> the expected return type.
- * @return the removed instance if the manager contains the given identifier, otherwise null.
+ * @return the removed instance if the manager contains the given identifier, otherwise null if
+ * the manager doesn't contain the value or the manager is closed.
*/
@Nullable
public <T> T remove(long identifier) {
- assertManagerIsNotClosed();
+ if (isClosed()) {
+ Log.w(TAG, CLOSED_WARNING);
+ return null;
+ }
return (T) strongInstances.remove(identifier);
}
@@ -95,11 +102,14 @@
*
* @param instance an instance that may be stored in the manager.
* @return the identifier associated with `instance` if the manager contains the value, otherwise
- * null.
+ * null if the manager doesn't contain the value or the manager is closed.
*/
@Nullable
public Long getIdentifierForStrongReference(Object instance) {
- assertManagerIsNotClosed();
+ if (isClosed()) {
+ Log.w(TAG, CLOSED_WARNING);
+ return null;
+ }
final Long identifier = identifiers.get(instance);
if (identifier != null) {
strongInstances.put(identifier, instance);
@@ -114,11 +124,16 @@
* The Dart InstanceManager is considered the source of truth and has the capability to overwrite
* stored pairs in response to hot restarts.
*
+ * <p>If the manager is closed, the addition is ignored.
+ *
* @param instance the instance to be stored.
* @param identifier the identifier to be paired with instance. This value must be >= 0.
*/
public void addDartCreatedInstance(Object instance, long identifier) {
- assertManagerIsNotClosed();
+ if (isClosed()) {
+ Log.w(TAG, CLOSED_WARNING);
+ return;
+ }
addInstance(instance, identifier);
}
@@ -126,10 +141,13 @@
* Adds a new instance that was instantiated from the host platform.
*
* @param instance the instance to be stored.
- * @return the unique identifier stored with instance.
+ * @return the unique identifier stored with instance. If the manager is closed, returns -1.
*/
public long addHostCreatedInstance(Object instance) {
- assertManagerIsNotClosed();
+ if (isClosed()) {
+ Log.w(TAG, CLOSED_WARNING);
+ return -1;
+ }
final long identifier = nextIdentifier++;
addInstance(instance, identifier);
return identifier;
@@ -141,11 +159,14 @@
* @param identifier the identifier paired to an instance.
* @param <T> the expected return type.
* @return the instance associated with `identifier` if the manager contains the value, otherwise
- * null.
+ * null if the manager doesn't contain the value or the manager is closed.
*/
@Nullable
public <T> T getInstance(long identifier) {
- assertManagerIsNotClosed();
+ if (isClosed()) {
+ Log.w(TAG, CLOSED_WARNING);
+ return null;
+ }
final WeakReference<T> instance = (WeakReference<T>) weakInstances.get(identifier);
if (instance != null) {
return instance.get();
@@ -157,22 +178,38 @@
* Returns whether this manager contains the given `instance`.
*
* @param instance the instance whose presence in this manager is to be tested.
- * @return whether this manager contains the given `instance`.
+ * @return whether this manager contains the given `instance`. If the manager is closed, returns
+ * `false`.
*/
public boolean containsInstance(Object instance) {
- assertManagerIsNotClosed();
+ if (isClosed()) {
+ Log.w(TAG, CLOSED_WARNING);
+ return false;
+ }
return identifiers.containsKey(instance);
}
/**
* Closes the manager and releases resources.
*
- * <p>Calling a method after calling this one will throw an {@link AssertionError}. This method
- * excluded.
+ * <p>Methods called after this one will be ignored and log a warning.
*/
public void close() {
handler.removeCallbacks(this::releaseAllFinalizedInstances);
isClosed = true;
+ identifiers.clear();
+ weakInstances.clear();
+ strongInstances.clear();
+ weakReferencesToIdentifiers.clear();
+ }
+
+ /**
+ * Whether the manager has released resources and is not longer usable.
+ *
+ * <p>See {@link #close()}.
+ */
+ public boolean isClosed() {
+ return isClosed;
}
private void releaseAllFinalizedInstances() {
@@ -199,10 +236,4 @@
weakReferencesToIdentifiers.put(weakReference, identifier);
strongInstances.put(identifier, instance);
}
-
- private void assertManagerIsNotClosed() {
- if (isClosed) {
- throw new AssertionError("Manager has already been closed.");
- }
- }
}
diff --git a/packages/webview_flutter/webview_flutter_android/android/src/test/java/io/flutter/plugins/webviewflutter/InstanceManagerTest.java b/packages/webview_flutter/webview_flutter_android/android/src/test/java/io/flutter/plugins/webviewflutter/InstanceManagerTest.java
index 4731e2a..6a19c88 100644
--- a/packages/webview_flutter/webview_flutter_android/android/src/test/java/io/flutter/plugins/webviewflutter/InstanceManagerTest.java
+++ b/packages/webview_flutter/webview_flutter_android/android/src/test/java/io/flutter/plugins/webviewflutter/InstanceManagerTest.java
@@ -5,6 +5,7 @@
package io.flutter.plugins.webviewflutter;
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;
@@ -59,4 +60,52 @@
instanceManager.close();
}
+
+ @Test
+ public void removeReturnsNullWhenClosed() {
+ final Object object = new Object();
+ final InstanceManager instanceManager = InstanceManager.open(identifier -> {});
+ instanceManager.addDartCreatedInstance(object, 0);
+ instanceManager.close();
+
+ assertNull(instanceManager.remove(0));
+ }
+
+ @Test
+ public void getIdentifierForStrongReferenceReturnsNullWhenClosed() {
+ final Object object = new Object();
+ final InstanceManager instanceManager = InstanceManager.open(identifier -> {});
+ instanceManager.addDartCreatedInstance(object, 0);
+ instanceManager.close();
+
+ assertNull(instanceManager.getIdentifierForStrongReference(object));
+ }
+
+ @Test
+ public void addHostCreatedInstanceReturnsNegativeOneWhenClosed() {
+ final InstanceManager instanceManager = InstanceManager.open(identifier -> {});
+ instanceManager.close();
+
+ assertEquals(instanceManager.addHostCreatedInstance(new Object()), -1L);
+ }
+
+ @Test
+ public void getInstanceReturnsNullWhenClosed() {
+ final Object object = new Object();
+ final InstanceManager instanceManager = InstanceManager.open(identifier -> {});
+ instanceManager.addDartCreatedInstance(object, 0);
+ instanceManager.close();
+
+ assertNull(instanceManager.getInstance(0));
+ }
+
+ @Test
+ public void containsInstanceReturnsFalseWhenClosed() {
+ final Object object = new Object();
+ final InstanceManager instanceManager = InstanceManager.open(identifier -> {});
+ instanceManager.addDartCreatedInstance(object, 0);
+ instanceManager.close();
+
+ assertFalse(instanceManager.containsInstance(object));
+ }
}
diff --git a/packages/webview_flutter/webview_flutter_android/pubspec.yaml b/packages/webview_flutter/webview_flutter_android/pubspec.yaml
index 57346e1..123a7c9 100644
--- a/packages/webview_flutter/webview_flutter_android/pubspec.yaml
+++ b/packages/webview_flutter/webview_flutter_android/pubspec.yaml
@@ -2,7 +2,7 @@
description: A Flutter plugin that provides a WebView widget on Android.
repository: https://github.com/flutter/plugins/tree/main/packages/webview_flutter/webview_flutter_android
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+webview%22
-version: 3.1.2
+version: 3.1.3
environment:
sdk: ">=2.17.0 <3.0.0"
