diff --git a/packages/webview_flutter/webview_flutter_android/CHANGELOG.md b/packages/webview_flutter/webview_flutter_android/CHANGELOG.md index d45e73556256..37abf3cf2b1b 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 fefd577ee9b5..55775a914c56 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 @@ public class InstanceManager { // 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 @@ private InstanceManager(FinalizationListener finalizationListener) { * * @param identifier the identifier paired to an instance. * @param 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 remove(long identifier) { - assertManagerIsNotClosed(); + if (isClosed()) { + Log.w(TAG, CLOSED_WARNING); + return null; + } return (T) strongInstances.remove(identifier); } @@ -95,11 +102,14 @@ public T remove(long identifier) { * * @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 @@ public Long getIdentifierForStrongReference(Object instance) { * The Dart InstanceManager is considered the source of truth and has the capability to overwrite * stored pairs in response to hot restarts. * + *

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 @@ public void addDartCreatedInstance(Object instance, long identifier) { * 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 @@ public long addHostCreatedInstance(Object instance) { * @param identifier the identifier paired to an instance. * @param 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 getInstance(long identifier) { - assertManagerIsNotClosed(); + if (isClosed()) { + Log.w(TAG, CLOSED_WARNING); + return null; + } final WeakReference instance = (WeakReference) weakInstances.get(identifier); if (instance != null) { return instance.get(); @@ -157,22 +178,38 @@ public T getInstance(long identifier) { * 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. * - *

Calling a method after calling this one will throw an {@link AssertionError}. This method - * excluded. + *

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. + * + *

See {@link #close()}. + */ + public boolean isClosed() { + return isClosed; } private void releaseAllFinalizedInstances() { @@ -199,10 +236,4 @@ private void addInstance(Object instance, long identifier) { 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 4731e2a4beb1..6a19c883548a 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 @@ public void remove() { 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 57346e147e7a..123a7c918870 100644 --- a/packages/webview_flutter/webview_flutter_android/pubspec.yaml +++ b/packages/webview_flutter/webview_flutter_android/pubspec.yaml @@ -2,7 +2,7 @@ name: webview_flutter_android 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"