diff --git a/packages/webview_flutter/webview_flutter_android/CHANGELOG.md b/packages/webview_flutter/webview_flutter_android/CHANGELOG.md index 411684c2ad1..9bb8d8205af 100644 --- a/packages/webview_flutter/webview_flutter_android/CHANGELOG.md +++ b/packages/webview_flutter/webview_flutter_android/CHANGELOG.md @@ -1,3 +1,7 @@ +## 3.4.3 + +* Updates internal Java InstanceManager to be cleared on hot restart. + ## 3.4.2 * Clarifies explanation of endorsement in README. diff --git a/packages/webview_flutter/webview_flutter_android/android/src/main/java/io/flutter/plugins/webviewflutter/GeneratedAndroidWebView.java b/packages/webview_flutter/webview_flutter_android/android/src/main/java/io/flutter/plugins/webviewflutter/GeneratedAndroidWebView.java index 189b85c32bb..88edf5815f1 100644 --- a/packages/webview_flutter/webview_flutter_android/android/src/main/java/io/flutter/plugins/webviewflutter/GeneratedAndroidWebView.java +++ b/packages/webview_flutter/webview_flutter_android/android/src/main/java/io/flutter/plugins/webviewflutter/GeneratedAndroidWebView.java @@ -439,6 +439,51 @@ public interface Result { void error(Throwable error); } + /** + * Host API for managing the native `InstanceManager`. + * + *

Generated interface from Pigeon that represents a handler of messages from Flutter. + */ + public interface InstanceManagerHostApi { + /** + * Clear the native `InstanceManager`. + * + *

This is typically only used after a hot restart. + */ + void clear(); + + /** The codec used by InstanceManagerHostApi. */ + static MessageCodec getCodec() { + return new StandardMessageCodec(); + } + /** + * Sets up an instance of `InstanceManagerHostApi` to handle messages through the + * `binaryMessenger`. + */ + static void setup(BinaryMessenger binaryMessenger, InstanceManagerHostApi api) { + { + BasicMessageChannel channel = + new BasicMessageChannel<>( + binaryMessenger, "dev.flutter.pigeon.InstanceManagerHostApi.clear", getCodec()); + if (api != null) { + channel.setMessageHandler( + (message, reply) -> { + ArrayList wrapped = new ArrayList(); + try { + api.clear(); + wrapped.add(0, null); + } catch (Error | RuntimeException exception) { + ArrayList wrappedError = wrapError(exception); + wrapped = wrappedError; + } + reply.reply(wrapped); + }); + } else { + channel.setMessageHandler(null); + } + } + } + } /** * Handles methods calls to the native Java Object class. * 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 55775a914c5..de51ebd0482 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 @@ -30,6 +30,9 @@ */ @SuppressWarnings("unchecked") public class InstanceManager { + /// Constant returned from #addHostCreatedInstance() if the manager is closed. + public static final int INSTANCE_CLOSED = -1; + // Identifiers are locked to a specific range to avoid collisions with objects // created simultaneously from Dart. // Host uses identifiers >= 2^16 and Dart is expected to use values n where, @@ -87,8 +90,7 @@ private InstanceManager(FinalizationListener finalizationListener) { */ @Nullable public T remove(long identifier) { - if (isClosed()) { - Log.w(TAG, CLOSED_WARNING); + if (assertNotClosed()) { return null; } return (T) strongInstances.remove(identifier); @@ -97,8 +99,14 @@ public T remove(long identifier) { /** * Retrieves the identifier paired with an instance. * - *

If the manager contains `instance`, as a strong or weak reference, the strong reference to - * `instance` will be recreated and will need to be removed again with {@link #remove(long)}. + *

If the manager contains a strong reference to `instance`, it will return the identifier + * associated with `instance`. If the manager contains only a weak reference to `instance`, a new + * strong reference to `instance` will be added and will need to be removed again with {@link + * #remove(long)}. + * + *

If this method returns a nonnull identifier, this method also expects the Dart + * `InstanceManager` to have, or recreate, a weak reference to the Dart instance the identifier is + * associated with. * * @param instance an instance that may be stored in the manager. * @return the identifier associated with `instance` if the manager contains the value, otherwise @@ -106,8 +114,7 @@ public T remove(long identifier) { */ @Nullable public Long getIdentifierForStrongReference(Object instance) { - if (isClosed()) { - Log.w(TAG, CLOSED_WARNING); + if (assertNotClosed()) { return null; } final Long identifier = identifiers.get(instance); @@ -120,18 +127,18 @@ public Long getIdentifierForStrongReference(Object instance) { /** * Adds a new instance that was instantiated from Dart. * - *

If an instance or identifier has already been added, it will be replaced by the new values. - * The Dart InstanceManager is considered the source of truth and has the capability to overwrite - * stored pairs in response to hot restarts. + *

The same instance can be added multiple times, but each identifier must be unique. This + * allows two objects that are equivalent (e.g. the `equals` method returns true and their + * hashcodes are equal) to both be added. * - *

If the manager is closed, the addition is ignored. + *

If the manager is closed, the addition is ignored and a warning is logged. * * @param instance the instance to be stored. - * @param identifier the identifier to be paired with instance. This value must be >= 0. + * @param identifier the identifier to be paired with instance. This value must be >= 0 and + * unique. */ public void addDartCreatedInstance(Object instance, long identifier) { - if (isClosed()) { - Log.w(TAG, CLOSED_WARNING); + if (assertNotClosed()) { return; } addInstance(instance, identifier); @@ -140,13 +147,18 @@ 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. + * @param instance the instance to be stored. This must be unique to all other added instances. * @return the unique identifier stored with instance. If the manager is closed, returns -1. + * Otherwise, returns a value >= 0. */ public long addHostCreatedInstance(Object instance) { - if (isClosed()) { - Log.w(TAG, CLOSED_WARNING); - return -1; + if (assertNotClosed()) { + return INSTANCE_CLOSED; + } + + if (containsInstance(instance)) { + throw new IllegalArgumentException( + String.format("Instance of `%s` has already been added.", instance.getClass())); } final long identifier = nextIdentifier++; addInstance(instance, identifier); @@ -156,22 +168,22 @@ public long addHostCreatedInstance(Object instance) { /** * Retrieves the instance associated with identifier. * - * @param identifier the identifier paired to an instance. + * @param identifier the identifier associated with an instance. * @param the expected return type. * @return the instance associated with `identifier` if the manager contains the value, otherwise * null if the manager doesn't contain the value or the manager is closed. */ @Nullable public T getInstance(long identifier) { - if (isClosed()) { - Log.w(TAG, CLOSED_WARNING); + if (assertNotClosed()) { return null; } + final WeakReference instance = (WeakReference) weakInstances.get(identifier); if (instance != null) { return instance.get(); } - return (T) strongInstances.get(identifier); + return null; } /** @@ -182,8 +194,7 @@ public T getInstance(long identifier) { * `false`. */ public boolean containsInstance(Object instance) { - if (isClosed()) { - Log.w(TAG, CLOSED_WARNING); + if (assertNotClosed()) { return false; } return identifiers.containsKey(instance); @@ -197,6 +208,15 @@ public boolean containsInstance(Object instance) { public void close() { handler.removeCallbacks(this::releaseAllFinalizedInstances); isClosed = true; + clear(); + } + + /** + * Removes all of the instances from this manager. + * + *

The manager will be empty after this call returns. + */ + public void clear() { identifiers.clear(); weakInstances.clear(); strongInstances.clear(); @@ -204,7 +224,7 @@ public void close() { } /** - * Whether the manager has released resources and is not longer usable. + * Whether the manager has released resources and is no longer usable. * *

See {@link #close()}. */ @@ -228,7 +248,11 @@ private void releaseAllFinalizedInstances() { private void addInstance(Object instance, long identifier) { if (identifier < 0) { - throw new IllegalArgumentException("Identifier must be >= 0."); + throw new IllegalArgumentException(String.format("Identifier must be >= 0: %d", identifier)); + } + if (weakInstances.containsKey(identifier)) { + throw new IllegalArgumentException( + String.format("Identifier has already been added: %d", identifier)); } final WeakReference weakReference = new WeakReference<>(instance, referenceQueue); identifiers.put(instance, identifier); @@ -236,4 +260,12 @@ private void addInstance(Object instance, long identifier) { weakReferencesToIdentifiers.put(weakReference, identifier); strongInstances.put(identifier, instance); } + + private boolean assertNotClosed() { + if (isClosed()) { + Log.w(TAG, CLOSED_WARNING); + return true; + } + return false; + } } diff --git a/packages/webview_flutter/webview_flutter_android/android/src/main/java/io/flutter/plugins/webviewflutter/WebViewFlutterPlugin.java b/packages/webview_flutter/webview_flutter_android/android/src/main/java/io/flutter/plugins/webviewflutter/WebViewFlutterPlugin.java index 04a9735e028..c30e956c893 100644 --- a/packages/webview_flutter/webview_flutter_android/android/src/main/java/io/flutter/plugins/webviewflutter/WebViewFlutterPlugin.java +++ b/packages/webview_flutter/webview_flutter_android/android/src/main/java/io/flutter/plugins/webviewflutter/WebViewFlutterPlugin.java @@ -17,6 +17,7 @@ import io.flutter.plugins.webviewflutter.GeneratedAndroidWebView.CookieManagerHostApi; import io.flutter.plugins.webviewflutter.GeneratedAndroidWebView.DownloadListenerHostApi; import io.flutter.plugins.webviewflutter.GeneratedAndroidWebView.FlutterAssetManagerHostApi; +import io.flutter.plugins.webviewflutter.GeneratedAndroidWebView.InstanceManagerHostApi; import io.flutter.plugins.webviewflutter.GeneratedAndroidWebView.JavaObjectHostApi; import io.flutter.plugins.webviewflutter.GeneratedAndroidWebView.JavaScriptChannelHostApi; import io.flutter.plugins.webviewflutter.GeneratedAndroidWebView.WebChromeClientHostApi; @@ -84,6 +85,8 @@ private void setUp( new GeneratedAndroidWebView.JavaObjectFlutterApi(binaryMessenger) .dispose(identifier, reply -> {})); + InstanceManagerHostApi.setup(binaryMessenger, () -> instanceManager.clear()); + viewRegistry.registerViewFactory( "plugins.flutter.io/webview", new FlutterWebViewFactory(instanceManager)); 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 6a19c883548..2bda2d12c75 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 @@ -108,4 +108,67 @@ public void containsInstanceReturnsFalseWhenClosed() { assertFalse(instanceManager.containsInstance(object)); } + + @Test + public void clear() { + final InstanceManager instanceManager = InstanceManager.open(identifier -> {}); + + final Object instance = new Object(); + + instanceManager.addDartCreatedInstance(instance, 0); + assertTrue(instanceManager.containsInstance(instance)); + + instanceManager.clear(); + assertFalse(instanceManager.containsInstance(instance)); + + instanceManager.close(); + } + + @Test + public void canAddSameObjectWithAddDartCreatedInstance() { + final InstanceManager instanceManager = InstanceManager.open(identifier -> {}); + + final Object instance = new Object(); + + instanceManager.addDartCreatedInstance(instance, 0); + instanceManager.addDartCreatedInstance(instance, 1); + + assertTrue(instanceManager.containsInstance(instance)); + + assertEquals(instanceManager.getInstance(0), instance); + assertEquals(instanceManager.getInstance(1), instance); + + instanceManager.close(); + } + + @Test(expected = IllegalArgumentException.class) + public void cannotAddSameObjectsWithAddHostCreatedInstance() { + final InstanceManager instanceManager = InstanceManager.open(identifier -> {}); + + final Object instance = new Object(); + + instanceManager.addHostCreatedInstance(instance); + instanceManager.addHostCreatedInstance(instance); + + instanceManager.close(); + } + + @Test(expected = IllegalArgumentException.class) + public void cannotUseIdentifierLessThanZero() { + final InstanceManager instanceManager = InstanceManager.open(identifier -> {}); + + instanceManager.addDartCreatedInstance(new Object(), -1); + + instanceManager.close(); + } + + @Test(expected = IllegalArgumentException.class) + public void identifiersMustBeUnique() { + final InstanceManager instanceManager = InstanceManager.open(identifier -> {}); + + instanceManager.addDartCreatedInstance(new Object(), 0); + instanceManager.addDartCreatedInstance(new Object(), 0); + + instanceManager.close(); + } } diff --git a/packages/webview_flutter/webview_flutter_android/android/src/test/java/io/flutter/plugins/webviewflutter/WebViewClientTest.java b/packages/webview_flutter/webview_flutter_android/android/src/test/java/io/flutter/plugins/webviewflutter/WebViewClientTest.java index b1cf1105771..f9e836e4510 100644 --- a/packages/webview_flutter/webview_flutter_android/android/src/test/java/io/flutter/plugins/webviewflutter/WebViewClientTest.java +++ b/packages/webview_flutter/webview_flutter_android/android/src/test/java/io/flutter/plugins/webviewflutter/WebViewClientTest.java @@ -118,8 +118,8 @@ public WebViewClient createWebViewClient(WebViewClientFlutterApiImpl flutterApi) }, mockFlutterApi); - instanceManager.addDartCreatedInstance(mockWebViewClient, 0); - webViewClientHostApi.setSynchronousReturnValueForShouldOverrideUrlLoading(0L, false); + instanceManager.addDartCreatedInstance(mockWebViewClient, 2); + webViewClientHostApi.setSynchronousReturnValueForShouldOverrideUrlLoading(2L, false); verify(mockWebViewClient).setReturnValueForShouldOverrideUrlLoading(false); } diff --git a/packages/webview_flutter/webview_flutter_android/android/src/test/java/io/flutter/plugins/webviewflutter/WebViewTest.java b/packages/webview_flutter/webview_flutter_android/android/src/test/java/io/flutter/plugins/webviewflutter/WebViewTest.java index 4a36184f03c..28027c30a61 100644 --- a/packages/webview_flutter/webview_flutter_android/android/src/test/java/io/flutter/plugins/webviewflutter/WebViewTest.java +++ b/packages/webview_flutter/webview_flutter_android/android/src/test/java/io/flutter/plugins/webviewflutter/WebViewTest.java @@ -310,9 +310,9 @@ public void destroy() { } }; - testInstanceManager.addDartCreatedInstance(webView, 0); + testInstanceManager.addDartCreatedInstance(webView, 1); final JavaObjectHostApiImpl javaObjectHostApi = new JavaObjectHostApiImpl(testInstanceManager); - javaObjectHostApi.dispose(0L); + javaObjectHostApi.dispose(1L); assertTrue(destroyCalled[0]); } diff --git a/packages/webview_flutter/webview_flutter_android/example/integration_test/webview_flutter_test.dart b/packages/webview_flutter/webview_flutter_android/example/integration_test/webview_flutter_test.dart index d8db40801e3..4ec7ae829e6 100644 --- a/packages/webview_flutter/webview_flutter_android/example/integration_test/webview_flutter_test.dart +++ b/packages/webview_flutter/webview_flutter_android/example/integration_test/webview_flutter_test.dart @@ -18,6 +18,7 @@ import 'package:flutter/services.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:integration_test/integration_test.dart'; import 'package:webview_flutter_android/src/android_webview.dart' as android; +import 'package:webview_flutter_android/src/android_webview.g.dart'; import 'package:webview_flutter_android/src/android_webview_api_impls.dart'; import 'package:webview_flutter_android/src/instance_manager.dart'; import 'package:webview_flutter_android/src/weak_reference_utils.dart'; @@ -111,6 +112,11 @@ Future main() async { } }); + // Since the InstanceManager of the apis are being changed, the native + // InstanceManager needs to be cleared otherwise an exception will be + // thrown that an identifier is being reused. + await InstanceManagerHostApi().clear(); + android.WebView.api = WebViewHostApiImpl( instanceManager: instanceManager, ); diff --git a/packages/webview_flutter/webview_flutter_android/lib/src/android_webview.dart b/packages/webview_flutter/webview_flutter_android/lib/src/android_webview.dart index 1f80e51d5ab..ca9b7184570 100644 --- a/packages/webview_flutter/webview_flutter_android/lib/src/android_webview.dart +++ b/packages/webview_flutter/webview_flutter_android/lib/src/android_webview.dart @@ -2,11 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -// TODO(bparrishMines): Replace unused callback methods in constructors with -// variables once automatic garbage collection is fully implemented. See -// https://github.com/flutter/flutter/issues/107199. -// ignore_for_file: avoid_unused_constructor_parameters - // TODO(a14n): remove this import once Flutter 3.1 or later reaches stable (including flutter/flutter#104231) // ignore: unnecessary_import import 'dart:typed_data'; @@ -14,7 +9,8 @@ import 'dart:ui'; import 'package:flutter/foundation.dart'; import 'package:flutter/services.dart' show BinaryMessenger; -import 'package:flutter/widgets.dart' show AndroidViewSurface; +import 'package:flutter/widgets.dart' + show AndroidViewSurface, WidgetsFlutterBinding; import 'android_webview.g.dart'; import 'android_webview_api_impls.dart'; @@ -30,6 +26,7 @@ class JavaObject with Copyable { /// /// This should only be used by subclasses created by this library or to /// create copies. + @protected JavaObject.detached({ BinaryMessenger? binaryMessenger, InstanceManager? instanceManager, @@ -39,11 +36,18 @@ class JavaObject with Copyable { ); /// Global instance of [InstanceManager]. - static final InstanceManager globalInstanceManager = InstanceManager( - onWeakReferenceRemoved: (int identifier) { - JavaObjectHostApiImpl().dispose(identifier); - }, - ); + static final InstanceManager globalInstanceManager = _initInstanceManager(); + + static InstanceManager _initInstanceManager() { + WidgetsFlutterBinding.ensureInitialized(); + // Clears the native `InstanceManager` on initial use of the Dart one. + InstanceManagerHostApi().clear(); + return InstanceManager( + onWeakReferenceRemoved: (int identifier) { + JavaObjectHostApiImpl().dispose(identifier); + }, + ); + } /// Pigeon Host Api implementation for [JavaObject]. final JavaObjectHostApiImpl _api; @@ -84,7 +88,11 @@ class WebView extends JavaObject { /// Due to changes in Flutter 3.0 the [useHybridComposition] doesn't have /// any effect and should not be exposed publicly. More info here: /// https://github.com/flutter/flutter/issues/108106 - WebView({this.useHybridComposition = false}) : super.detached() { + WebView({ + this.useHybridComposition = false, + @visibleForTesting super.binaryMessenger, + @visibleForTesting super.instanceManager, + }) : super.detached() { api.createFromInstance(this); } @@ -92,7 +100,12 @@ class WebView extends JavaObject { /// /// This should only be used by subclasses created by this library or to /// create copies. - WebView.detached({this.useHybridComposition = false}) : super.detached(); + @protected + WebView.detached({ + this.useHybridComposition = false, + super.binaryMessenger, + super.instanceManager, + }) : super.detached(); /// Pigeon Host Api implementation for [WebView]. @visibleForTesting @@ -399,7 +412,11 @@ class WebView extends JavaObject { @override WebView copy() { - return WebView.detached(useHybridComposition: useHybridComposition); + return WebView.detached( + useHybridComposition: useHybridComposition, + binaryMessenger: _api.binaryMessenger, + instanceManager: _api.instanceManager, + ); } } @@ -459,7 +476,11 @@ class WebSettings extends JavaObject { /// This constructor is only used for testing. An instance should be obtained /// with [WebView.settings]. @visibleForTesting - WebSettings(WebView webView) : super.detached() { + WebSettings( + WebView webView, { + @visibleForTesting super.binaryMessenger, + @visibleForTesting super.instanceManager, + }) : super.detached() { api.createFromInstance(this, webView); } @@ -467,7 +488,11 @@ class WebSettings extends JavaObject { /// /// This should only be used by subclasses created by this library or to /// create copies. - WebSettings.detached() : super.detached(); + @protected + WebSettings.detached({ + super.binaryMessenger, + super.instanceManager, + }) : super.detached(); /// Pigeon Host Api implementation for [WebSettings]. @visibleForTesting @@ -600,7 +625,10 @@ class WebSettings extends JavaObject { @override WebSettings copy() { - return WebSettings.detached(); + return WebSettings.detached( + binaryMessenger: _api.binaryMessenger, + instanceManager: _api.instanceManager, + ); } } @@ -612,6 +640,8 @@ class JavaScriptChannel extends JavaObject { JavaScriptChannel( this.channelName, { required this.postMessage, + @visibleForTesting super.binaryMessenger, + @visibleForTesting super.instanceManager, }) : super.detached() { AndroidWebViewFlutterApis.instance.ensureSetUp(); api.createFromInstance(this); @@ -622,9 +652,12 @@ class JavaScriptChannel extends JavaObject { /// /// This should only be used by subclasses created by this library or to /// create copies. + @protected JavaScriptChannel.detached( this.channelName, { required this.postMessage, + super.binaryMessenger, + super.instanceManager, }) : super.detached(); /// Pigeon Host Api implementation for [JavaScriptChannel]. @@ -639,7 +672,12 @@ class JavaScriptChannel extends JavaObject { @override JavaScriptChannel copy() { - return JavaScriptChannel.detached(channelName, postMessage: postMessage); + return JavaScriptChannel.detached( + channelName, + postMessage: postMessage, + binaryMessenger: _api.binaryMessenger, + instanceManager: _api.instanceManager, + ); } } @@ -653,6 +691,8 @@ class WebViewClient extends JavaObject { @Deprecated('Only called on Android version < 23.') this.onReceivedError, this.requestLoading, this.urlLoading, + @visibleForTesting super.binaryMessenger, + @visibleForTesting super.instanceManager, }) : super.detached() { AndroidWebViewFlutterApis.instance.ensureSetUp(); api.createFromInstance(this); @@ -662,6 +702,7 @@ class WebViewClient extends JavaObject { /// /// This should only be used by subclasses created by this library or to /// create copies. + @protected WebViewClient.detached({ this.onPageStarted, this.onPageFinished, @@ -669,6 +710,8 @@ class WebViewClient extends JavaObject { @Deprecated('Only called on Android version < 23.') this.onReceivedError, this.requestLoading, this.urlLoading, + super.binaryMessenger, + super.instanceManager, }) : super.detached(); /// User authentication failed on server. @@ -838,6 +881,8 @@ class WebViewClient extends JavaObject { onReceivedError: onReceivedError, requestLoading: requestLoading, urlLoading: urlLoading, + binaryMessenger: _api.binaryMessenger, + instanceManager: _api.instanceManager, ); } } @@ -846,7 +891,11 @@ class WebViewClient extends JavaObject { /// engine for [WebView], and should be downloaded instead. class DownloadListener extends JavaObject { /// Constructs a [DownloadListener]. - DownloadListener({required this.onDownloadStart}) : super.detached() { + DownloadListener({ + required this.onDownloadStart, + @visibleForTesting super.binaryMessenger, + @visibleForTesting super.instanceManager, + }) : super.detached() { AndroidWebViewFlutterApis.instance.ensureSetUp(); api.createFromInstance(this); } @@ -856,7 +905,12 @@ class DownloadListener extends JavaObject { /// /// This should only be used by subclasses created by this library or to /// create copies. - DownloadListener.detached({required this.onDownloadStart}) : super.detached(); + @protected + DownloadListener.detached({ + required this.onDownloadStart, + super.binaryMessenger, + super.instanceManager, + }) : super.detached(); /// Pigeon Host Api implementation for [DownloadListener]. @visibleForTesting @@ -873,15 +927,23 @@ class DownloadListener extends JavaObject { @override DownloadListener copy() { - return DownloadListener.detached(onDownloadStart: onDownloadStart); + return DownloadListener.detached( + onDownloadStart: onDownloadStart, + binaryMessenger: _api.binaryMessenger, + instanceManager: _api.instanceManager, + ); } } /// Handles JavaScript dialogs, favicons, titles, and the progress for [WebView]. class WebChromeClient extends JavaObject { /// Constructs a [WebChromeClient]. - WebChromeClient({this.onProgressChanged, this.onShowFileChooser}) - : super.detached() { + WebChromeClient({ + this.onProgressChanged, + this.onShowFileChooser, + @visibleForTesting super.binaryMessenger, + @visibleForTesting super.instanceManager, + }) : super.detached() { AndroidWebViewFlutterApis.instance.ensureSetUp(); api.createFromInstance(this); } @@ -891,9 +953,12 @@ class WebChromeClient extends JavaObject { /// /// This should only be used by subclasses created by this library or to /// create copies. + @protected WebChromeClient.detached({ this.onProgressChanged, this.onShowFileChooser, + super.binaryMessenger, + super.instanceManager, }) : super.detached(); /// Pigeon Host Api implementation for [WebChromeClient]. @@ -950,6 +1015,8 @@ class WebChromeClient extends JavaObject { return WebChromeClient.detached( onProgressChanged: onProgressChanged, onShowFileChooser: onShowFileChooser, + binaryMessenger: _api.binaryMessenger, + instanceManager: _api.instanceManager, ); } } @@ -963,6 +1030,7 @@ class FileChooserParams extends JavaObject { /// /// This should only be used by subclasses created by this library or to /// create copies. + @protected FileChooserParams.detached({ required this.isCaptureEnabled, required this.acceptTypes, @@ -991,6 +1059,8 @@ class FileChooserParams extends JavaObject { acceptTypes: acceptTypes, filenameHint: filenameHint, mode: mode, + binaryMessenger: _api.binaryMessenger, + instanceManager: _api.instanceManager, ); } } @@ -1074,7 +1144,10 @@ class WebStorage extends JavaObject { /// This constructor is only used for testing. An instance should be obtained /// with [WebStorage.instance]. @visibleForTesting - WebStorage() : super.detached() { + WebStorage({ + @visibleForTesting super.binaryMessenger, + @visibleForTesting super.instanceManager, + }) : super.detached() { AndroidWebViewFlutterApis.instance.ensureSetUp(); api.createFromInstance(this); } @@ -1083,7 +1156,11 @@ class WebStorage extends JavaObject { /// /// This should only be used by subclasses created by this library or to /// create copies. - WebStorage.detached() : super.detached(); + @protected + WebStorage.detached({ + super.binaryMessenger, + super.instanceManager, + }) : super.detached(); /// Pigeon Host Api implementation for [WebStorage]. @visibleForTesting @@ -1099,6 +1176,9 @@ class WebStorage extends JavaObject { @override WebStorage copy() { - return WebStorage.detached(); + return WebStorage.detached( + binaryMessenger: _api.binaryMessenger, + instanceManager: _api.instanceManager, + ); } } diff --git a/packages/webview_flutter/webview_flutter_android/lib/src/android_webview.g.dart b/packages/webview_flutter/webview_flutter_android/lib/src/android_webview.g.dart index ebfb0cc017f..d7a965f22ec 100644 --- a/packages/webview_flutter/webview_flutter_android/lib/src/android_webview.g.dart +++ b/packages/webview_flutter/webview_flutter_android/lib/src/android_webview.g.dart @@ -152,6 +152,42 @@ class WebViewPoint { } } +/// Host API for managing the native `InstanceManager`. +class InstanceManagerHostApi { + /// Constructor for [InstanceManagerHostApi]. The [binaryMessenger] named argument is + /// available for dependency injection. If it is left null, the default + /// BinaryMessenger will be used which routes to the host platform. + InstanceManagerHostApi({BinaryMessenger? binaryMessenger}) + : _binaryMessenger = binaryMessenger; + final BinaryMessenger? _binaryMessenger; + + static const MessageCodec codec = StandardMessageCodec(); + + /// Clear the native `InstanceManager`. + /// + /// This is typically only used after a hot restart. + Future clear() async { + final BasicMessageChannel channel = BasicMessageChannel( + 'dev.flutter.pigeon.InstanceManagerHostApi.clear', codec, + binaryMessenger: _binaryMessenger); + final List? replyList = await channel.send(null) as List?; + if (replyList == null) { + throw PlatformException( + code: 'channel-error', + message: 'Unable to establish connection on channel.', + ); + } else if (replyList.length > 1) { + throw PlatformException( + code: replyList[0]! as String, + message: replyList[1] as String?, + details: replyList[2], + ); + } else { + return; + } + } +} + /// Handles methods calls to the native Java Object class. /// /// Also handles calls to remove the reference to an instance with `dispose`. diff --git a/packages/webview_flutter/webview_flutter_android/lib/src/android_webview_api_impls.dart b/packages/webview_flutter/webview_flutter_android/lib/src/android_webview_api_impls.dart index ce34673b431..0af40571ce0 100644 --- a/packages/webview_flutter/webview_flutter_android/lib/src/android_webview_api_impls.dart +++ b/packages/webview_flutter/webview_flutter_android/lib/src/android_webview_api_impls.dart @@ -915,8 +915,16 @@ class WebStorageHostApiImpl extends WebStorageHostApi { /// Flutter api implementation for [FileChooserParams]. class FileChooserParamsFlutterApiImpl extends FileChooserParamsFlutterApi { /// Constructs a [FileChooserParamsFlutterApiImpl]. - FileChooserParamsFlutterApiImpl({InstanceManager? instanceManager}) - : instanceManager = instanceManager ?? JavaObject.globalInstanceManager; + FileChooserParamsFlutterApiImpl({ + this.binaryMessenger, + InstanceManager? instanceManager, + }) : instanceManager = instanceManager ?? JavaObject.globalInstanceManager; + + /// Receives binary data across the Flutter platform barrier. + /// + /// If it is null, the default BinaryMessenger will be used which routes to + /// the host platform. + final BinaryMessenger? binaryMessenger; /// Maintains instances stored to communicate with java objects. final InstanceManager instanceManager; @@ -935,6 +943,8 @@ class FileChooserParamsFlutterApiImpl extends FileChooserParamsFlutterApi { acceptTypes: acceptTypes.cast(), mode: mode.value, filenameHint: filenameHint, + binaryMessenger: binaryMessenger, + instanceManager: instanceManager, ), instanceId, ); diff --git a/packages/webview_flutter/webview_flutter_android/lib/src/instance_manager.dart b/packages/webview_flutter/webview_flutter_android/lib/src/instance_manager.dart index 5892823aabd..732e2e631ad 100644 --- a/packages/webview_flutter/webview_flutter_android/lib/src/instance_manager.dart +++ b/packages/webview_flutter/webview_flutter_android/lib/src/instance_manager.dart @@ -7,10 +7,7 @@ import 'package:flutter/foundation.dart'; /// An immutable object that can provide functional copies of itself. /// /// All implementers are expected to be immutable as defined by the annotation. -// TODO(bparrishMines): Uncomment annotation once -// https://github.com/flutter/plugins/pull/5831 lands or when making a breaking -// change for https://github.com/flutter/flutter/issues/107199. -// @immutable +@immutable mixin Copyable { /// Instantiates and returns a functionally identical object to oneself. /// @@ -82,8 +79,6 @@ class InstanceManager { /// /// Returns the randomly generated id of the [instance] added. int addDartCreatedInstance(Copyable instance) { - assert(getIdentifier(instance) == null); - final int identifier = _nextUniqueIdentifier(); _addInstanceWithIdentifier(instance, identifier); return identifier; @@ -168,13 +163,14 @@ class InstanceManager { /// /// Returns unique identifier of the [instance] added. void addHostCreatedInstance(Copyable instance, int identifier) { - assert(!containsIdentifier(identifier)); - assert(getIdentifier(instance) == null); - assert(identifier >= 0); _addInstanceWithIdentifier(instance, identifier); } void _addInstanceWithIdentifier(Copyable instance, int identifier) { + assert(!containsIdentifier(identifier)); + assert(getIdentifier(instance) == null); + assert(identifier >= 0); + _identifiers[instance] = identifier; _weakInstances[identifier] = WeakReference(instance); _finalizer.attach(instance, identifier, detach: instance); diff --git a/packages/webview_flutter/webview_flutter_android/pigeons/android_webview.dart b/packages/webview_flutter/webview_flutter_android/pigeons/android_webview.dart index 20bc9155596..9205e83b6a6 100644 --- a/packages/webview_flutter/webview_flutter_android/pigeons/android_webview.dart +++ b/packages/webview_flutter/webview_flutter_android/pigeons/android_webview.dart @@ -27,6 +27,15 @@ import 'package:pigeon/pigeon.dart'; ), ) +/// Host API for managing the native `InstanceManager`. +@HostApi(dartHostTestHandler: 'TestInstanceManagerHostApi') +abstract class InstanceManagerHostApi { + /// Clear the native `InstanceManager`. + /// + /// This is typically only used after a hot restart. + void clear(); +} + /// Mode of how to select files for a file chooser. /// /// See https://developer.android.com/reference/android/webkit/WebChromeClient.FileChooserParams. diff --git a/packages/webview_flutter/webview_flutter_android/pubspec.yaml b/packages/webview_flutter/webview_flutter_android/pubspec.yaml index 8ad0017b040..6e270606a4c 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/packages/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.4.2 +version: 3.4.3 environment: sdk: ">=2.17.0 <3.0.0" diff --git a/packages/webview_flutter/webview_flutter_android/test/android_navigation_delegate_test.dart b/packages/webview_flutter/webview_flutter_android/test/android_navigation_delegate_test.dart index dac7c69a84f..2bf563b293d 100644 --- a/packages/webview_flutter/webview_flutter_android/test/android_navigation_delegate_test.dart +++ b/packages/webview_flutter/webview_flutter_android/test/android_navigation_delegate_test.dart @@ -5,13 +5,23 @@ import 'dart:async'; import 'package:flutter_test/flutter_test.dart'; +import 'package:mockito/annotations.dart'; import 'package:webview_flutter_android/src/android_proxy.dart'; import 'package:webview_flutter_android/src/android_webview.dart' as android_webview; import 'package:webview_flutter_android/webview_flutter_android.dart'; import 'package:webview_flutter_platform_interface/webview_flutter_platform_interface.dart'; +import 'android_navigation_delegate_test.mocks.dart'; +import 'test_android_webview.g.dart'; + +@GenerateMocks([TestInstanceManagerHostApi]) void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + + // Mocks the call to clear the native InstanceManager. + TestInstanceManagerHostApi.setup(MockTestInstanceManagerHostApi()); + group('AndroidNavigationDelegate', () { test('onPageFinished', () { final AndroidNavigationDelegate androidNavigationDelegate = @@ -452,6 +462,7 @@ AndroidNavigationDelegateCreationParams _buildCreationParams() { } // Records the last created instance of itself. +// ignore: must_be_immutable class CapturingWebViewClient extends android_webview.WebViewClient { CapturingWebViewClient({ super.onPageFinished, @@ -460,6 +471,8 @@ class CapturingWebViewClient extends android_webview.WebViewClient { super.onReceivedRequestError, super.requestLoading, super.urlLoading, + super.binaryMessenger, + super.instanceManager, }) : super.detached() { lastCreatedDelegate = this; } @@ -480,6 +493,8 @@ class CapturingWebChromeClient extends android_webview.WebChromeClient { CapturingWebChromeClient({ super.onProgressChanged, super.onShowFileChooser, + super.binaryMessenger, + super.instanceManager, }) : super.detached() { lastCreatedDelegate = this; } @@ -491,6 +506,8 @@ class CapturingWebChromeClient extends android_webview.WebChromeClient { class CapturingDownloadListener extends android_webview.DownloadListener { CapturingDownloadListener({ required super.onDownloadStart, + super.binaryMessenger, + super.instanceManager, }) : super.detached() { lastCreatedListener = this; } diff --git a/packages/webview_flutter/webview_flutter_android/test/android_navigation_delegate_test.mocks.dart b/packages/webview_flutter/webview_flutter_android/test/android_navigation_delegate_test.mocks.dart new file mode 100644 index 00000000000..25e2d424a44 --- /dev/null +++ b/packages/webview_flutter/webview_flutter_android/test/android_navigation_delegate_test.mocks.dart @@ -0,0 +1,38 @@ +// Mocks generated by Mockito 5.3.2 from annotations +// in webview_flutter_android/test/android_navigation_delegate_test.dart. +// Do not manually edit this file. + +// ignore_for_file: no_leading_underscores_for_library_prefixes +import 'package:mockito/mockito.dart' as _i1; + +import 'test_android_webview.g.dart' as _i2; + +// ignore_for_file: type=lint +// ignore_for_file: avoid_redundant_argument_values +// ignore_for_file: avoid_setters_without_getters +// ignore_for_file: comment_references +// ignore_for_file: implementation_imports +// ignore_for_file: invalid_use_of_visible_for_testing_member +// ignore_for_file: prefer_const_constructors +// ignore_for_file: unnecessary_parenthesis +// ignore_for_file: camel_case_types +// ignore_for_file: subtype_of_sealed_class + +/// A class which mocks [TestInstanceManagerHostApi]. +/// +/// See the documentation for Mockito's code generation for more information. +class MockTestInstanceManagerHostApi extends _i1.Mock + implements _i2.TestInstanceManagerHostApi { + MockTestInstanceManagerHostApi() { + _i1.throwOnMissingStub(this); + } + + @override + void clear() => super.noSuchMethod( + Invocation.method( + #clear, + [], + ), + returnValueForMissingStub: null, + ); +} diff --git a/packages/webview_flutter/webview_flutter_android/test/android_webview_controller_test.dart b/packages/webview_flutter/webview_flutter_android/test/android_webview_controller_test.dart index b5214889c95..777d53bc166 100644 --- a/packages/webview_flutter/webview_flutter_android/test/android_webview_controller_test.dart +++ b/packages/webview_flutter/webview_flutter_android/test/android_webview_controller_test.dart @@ -22,6 +22,7 @@ import 'package:webview_flutter_platform_interface/src/webview_platform.dart'; import 'android_navigation_delegate_test.dart'; import 'android_webview_controller_test.mocks.dart'; +import 'test_android_webview.g.dart'; @GenerateNiceMocks(>[ MockSpec(), @@ -39,10 +40,14 @@ import 'android_webview_controller_test.mocks.dart'; MockSpec(), MockSpec(), MockSpec(), + MockSpec(), ]) void main() { TestWidgetsFlutterBinding.ensureInitialized(); + // Mocks the call to clear the native InstanceManager. + TestInstanceManagerHostApi.setup(MockTestInstanceManagerHostApi()); + AndroidWebViewController createControllerWithMocks({ android_webview.FlutterAssetManager? mockFlutterAssetManager, android_webview.JavaScriptChannel? mockJavaScriptChannel, @@ -582,7 +587,7 @@ void main() { android_webview.WebView.detached(), android_webview.FileChooserParams.detached( isCaptureEnabled: false, - acceptTypes: ['png'], + acceptTypes: const ['png'], filenameHint: 'filenameHint', mode: android_webview.FileChooserMode.open, ), diff --git a/packages/webview_flutter/webview_flutter_android/test/android_webview_controller_test.mocks.dart b/packages/webview_flutter/webview_flutter_android/test/android_webview_controller_test.mocks.dart index 7cb79026b09..76160aa7a07 100644 --- a/packages/webview_flutter/webview_flutter_android/test/android_webview_controller_test.mocks.dart +++ b/packages/webview_flutter/webview_flutter_android/test/android_webview_controller_test.mocks.dart @@ -22,6 +22,8 @@ import 'package:webview_flutter_android/src/platform_views_service_proxy.dart' import 'package:webview_flutter_platform_interface/webview_flutter_platform_interface.dart' as _i3; +import 'test_android_webview.g.dart' as _i15; + // ignore_for_file: type=lint // ignore_for_file: avoid_redundant_argument_values // ignore_for_file: avoid_setters_without_getters @@ -2234,3 +2236,18 @@ class MockInstanceManager extends _i1.Mock implements _i5.InstanceManager { returnValueForMissingStub: false, ) as bool); } + +/// A class which mocks [TestInstanceManagerHostApi]. +/// +/// See the documentation for Mockito's code generation for more information. +class MockTestInstanceManagerHostApi extends _i1.Mock + implements _i15.TestInstanceManagerHostApi { + @override + void clear() => super.noSuchMethod( + Invocation.method( + #clear, + [], + ), + returnValueForMissingStub: null, + ); +} diff --git a/packages/webview_flutter/webview_flutter_android/test/android_webview_test.dart b/packages/webview_flutter/webview_flutter_android/test/android_webview_test.dart index a6184724e60..e1489f6ca88 100644 --- a/packages/webview_flutter/webview_flutter_android/test/android_webview_test.dart +++ b/packages/webview_flutter/webview_flutter_android/test/android_webview_test.dart @@ -18,6 +18,7 @@ import 'test_android_webview.g.dart'; DownloadListener, JavaScriptChannel, TestDownloadListenerHostApi, + TestInstanceManagerHostApi, TestJavaObjectHostApi, TestJavaScriptChannelHostApi, TestWebChromeClientHostApi, @@ -33,6 +34,9 @@ import 'test_android_webview.g.dart'; void main() { TestWidgetsFlutterBinding.ensureInitialized(); + // Mocks the call to clear the native InstanceManager. + TestInstanceManagerHostApi.setup(MockTestInstanceManagerHostApi()); + group('Android WebView', () { group('JavaObject', () { late MockTestJavaObjectHostApi mockPlatformHostApi; diff --git a/packages/webview_flutter/webview_flutter_android/test/android_webview_test.mocks.dart b/packages/webview_flutter/webview_flutter_android/test/android_webview_test.mocks.dart index 0de71948cb8..2cb0081ba8b 100644 --- a/packages/webview_flutter/webview_flutter_android/test/android_webview_test.mocks.dart +++ b/packages/webview_flutter/webview_flutter_android/test/android_webview_test.mocks.dart @@ -242,6 +242,25 @@ class MockTestDownloadListenerHostApi extends _i1.Mock ); } +/// A class which mocks [TestInstanceManagerHostApi]. +/// +/// See the documentation for Mockito's code generation for more information. +class MockTestInstanceManagerHostApi extends _i1.Mock + implements _i6.TestInstanceManagerHostApi { + MockTestInstanceManagerHostApi() { + _i1.throwOnMissingStub(this); + } + + @override + void clear() => super.noSuchMethod( + Invocation.method( + #clear, + [], + ), + returnValueForMissingStub: null, + ); +} + /// A class which mocks [TestJavaObjectHostApi]. /// /// See the documentation for Mockito's code generation for more information. diff --git a/packages/webview_flutter/webview_flutter_android/test/instance_manager_test.dart b/packages/webview_flutter/webview_flutter_android/test/instance_manager_test.dart index dd3dcca29fe..67caa96a62f 100644 --- a/packages/webview_flutter/webview_flutter_android/test/instance_manager_test.dart +++ b/packages/webview_flutter/webview_flutter_android/test/instance_manager_test.dart @@ -3,9 +3,18 @@ // found in the LICENSE file. import 'package:flutter_test/flutter_test.dart'; +import 'package:mockito/annotations.dart'; +import 'package:mockito/mockito.dart'; +import 'package:webview_flutter_android/src/android_webview.dart'; import 'package:webview_flutter_android/src/instance_manager.dart'; +import 'instance_manager_test.mocks.dart'; +import 'test_android_webview.g.dart'; + +@GenerateMocks([TestInstanceManagerHostApi]) void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + group('InstanceManager', () { test('addHostCreatedInstance', () { final CopyableObject object = CopyableObject(); @@ -132,6 +141,20 @@ void main() { )!; expect(identical(object, newWeakCopy), isFalse); }); + + test('globalInstanceManager clears native `InstanceManager`', () { + final MockTestInstanceManagerHostApi mockApi = + MockTestInstanceManagerHostApi(); + TestInstanceManagerHostApi.setup(mockApi); + + // Calls method to clear the native InstanceManager. + // ignore: unnecessary_statements + JavaObject.globalInstanceManager; + + verify(mockApi.clear()); + + TestInstanceManagerHostApi.setup(null); + }); }); } diff --git a/packages/webview_flutter/webview_flutter_android/test/instance_manager_test.mocks.dart b/packages/webview_flutter/webview_flutter_android/test/instance_manager_test.mocks.dart new file mode 100644 index 00000000000..b474bf1ff56 --- /dev/null +++ b/packages/webview_flutter/webview_flutter_android/test/instance_manager_test.mocks.dart @@ -0,0 +1,38 @@ +// Mocks generated by Mockito 5.3.2 from annotations +// in webview_flutter_android/test/instance_manager_test.dart. +// Do not manually edit this file. + +// ignore_for_file: no_leading_underscores_for_library_prefixes +import 'package:mockito/mockito.dart' as _i1; + +import 'test_android_webview.g.dart' as _i2; + +// ignore_for_file: type=lint +// ignore_for_file: avoid_redundant_argument_values +// ignore_for_file: avoid_setters_without_getters +// ignore_for_file: comment_references +// ignore_for_file: implementation_imports +// ignore_for_file: invalid_use_of_visible_for_testing_member +// ignore_for_file: prefer_const_constructors +// ignore_for_file: unnecessary_parenthesis +// ignore_for_file: camel_case_types +// ignore_for_file: subtype_of_sealed_class + +/// A class which mocks [TestInstanceManagerHostApi]. +/// +/// See the documentation for Mockito's code generation for more information. +class MockTestInstanceManagerHostApi extends _i1.Mock + implements _i2.TestInstanceManagerHostApi { + MockTestInstanceManagerHostApi() { + _i1.throwOnMissingStub(this); + } + + @override + void clear() => super.noSuchMethod( + Invocation.method( + #clear, + [], + ), + returnValueForMissingStub: null, + ); +} diff --git a/packages/webview_flutter/webview_flutter_android/test/test_android_webview.g.dart b/packages/webview_flutter/webview_flutter_android/test/test_android_webview.g.dart index f1a19ecf0ed..85acbcf8915 100644 --- a/packages/webview_flutter/webview_flutter_android/test/test_android_webview.g.dart +++ b/packages/webview_flutter/webview_flutter_android/test/test_android_webview.g.dart @@ -13,6 +13,34 @@ import 'package:flutter_test/flutter_test.dart'; import 'package:webview_flutter_android/src/android_webview.g.dart'; +/// Host API for managing the native `InstanceManager`. +abstract class TestInstanceManagerHostApi { + static const MessageCodec codec = StandardMessageCodec(); + + /// Clear the native `InstanceManager`. + /// + /// This is typically only used after a hot restart. + void clear(); + + static void setup(TestInstanceManagerHostApi? api, + {BinaryMessenger? binaryMessenger}) { + { + final BasicMessageChannel channel = BasicMessageChannel( + 'dev.flutter.pigeon.InstanceManagerHostApi.clear', codec, + binaryMessenger: binaryMessenger); + if (api == null) { + channel.setMockMessageHandler(null); + } else { + channel.setMockMessageHandler((Object? message) async { + // ignore message + api.clear(); + return []; + }); + } + } + } +} + /// Handles methods calls to the native Java Object class. /// /// Also handles calls to remove the reference to an instance with `dispose`.