Skip to content

[webview_flutter_android] Updates Dart and Java InstanceManagers #3282

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 30 commits into from
Mar 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
eaae468
add changed files
bparrishMines Feb 22, 2023
9d90d9f
some more changes
bparrishMines Feb 23, 2023
1094620
update tests
bparrishMines Feb 23, 2023
ce1634c
fix tests
bparrishMines Feb 23, 2023
3fae384
Merge branch 'main' of github.com:flutter/packages into instance_mana…
bparrishMines Feb 23, 2023
44e6f81
update instancemanager
bparrishMines Feb 23, 2023
ce6550b
ensure callback methods are passed to copy
bparrishMines Feb 23, 2023
def8956
version bump
bparrishMines Feb 23, 2023
ca9993e
update tests since objects can't be added multiple times
bparrishMines Feb 23, 2023
1408d00
Merge branch 'main' of github.com:flutter/packages into instance_mana…
bparrishMines Feb 27, 2023
1c48779
fix test
bparrishMines Feb 28, 2023
a552de9
change back to using copyable
bparrishMines Feb 28, 2023
9d58bd8
probably fix tests
bparrishMines Feb 28, 2023
2a05f7c
some reverts
bparrishMines Feb 28, 2023
73ea794
update changelog
bparrishMines Feb 28, 2023
625146d
Merge branch 'main' of github.com:flutter/packages into instance_mana…
bparrishMines Feb 28, 2023
32ff598
add a copy
bparrishMines Feb 28, 2023
b848c1f
raise minimum meta version
bparrishMines Feb 28, 2023
8efddc9
remove meta
bparrishMines Feb 28, 2023
e2b53b1
remove copy override
bparrishMines Feb 28, 2023
a9a9aa6
remove print
bparrishMines Feb 28, 2023
0ea91f7
remove unneeded changes
bparrishMines Feb 28, 2023
8141e9a
include raising of pigeon version
bparrishMines Feb 28, 2023
4a5aab1
Merge branch 'main' of github.com:flutter/packages into instance_mana…
bparrishMines Mar 6, 2023
f196771
update globalInstanceManager
bparrishMines Mar 6, 2023
7e32787
move location of asserts
bparrishMines Mar 6, 2023
9b572e2
Merge branch 'main' of github.com:flutter/packages into instance_mana…
bparrishMines Mar 9, 2023
1062ddb
lint
bparrishMines Mar 9, 2023
9605570
use a constant
bparrishMines Mar 16, 2023
d786495
Merge branch 'main' of github.com:flutter/packages into instance_mana…
bparrishMines Mar 16, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -439,6 +439,51 @@ public interface Result<T> {

void error(Throwable error);
}
/**
* Host API for managing the native `InstanceManager`.
*
* <p>Generated interface from Pigeon that represents a handler of messages from Flutter.
*/
public interface InstanceManagerHostApi {
/**
* Clear the native `InstanceManager`.
*
* <p>This is typically only used after a hot restart.
*/
void clear();

/** The codec used by InstanceManagerHostApi. */
static MessageCodec<Object> getCodec() {
return new StandardMessageCodec();
}
/**
* Sets up an instance of `InstanceManagerHostApi` to handle messages through the
* `binaryMessenger`.
*/
static void setup(BinaryMessenger binaryMessenger, InstanceManagerHostApi api) {
{
BasicMessageChannel<Object> channel =
new BasicMessageChannel<>(
binaryMessenger, "dev.flutter.pigeon.InstanceManagerHostApi.clear", getCodec());
if (api != null) {
channel.setMessageHandler(
(message, reply) -> {
ArrayList<Object> wrapped = new ArrayList<Object>();
try {
api.clear();
wrapped.add(0, null);
} catch (Error | RuntimeException exception) {
ArrayList<Object> wrappedError = wrapError(exception);
wrapped = wrappedError;
}
reply.reply(wrapped);
});
} else {
channel.setMessageHandler(null);
}
}
}
}
/**
* Handles methods calls to the native Java Object class.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -87,8 +90,7 @@ private InstanceManager(FinalizationListener finalizationListener) {
*/
@Nullable
public <T> T remove(long identifier) {
if (isClosed()) {
Log.w(TAG, CLOSED_WARNING);
if (assertNotClosed()) {
return null;
}
return (T) strongInstances.remove(identifier);
Expand All @@ -97,17 +99,22 @@ public <T> T remove(long identifier) {
/**
* Retrieves the identifier paired with an instance.
*
* <p>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)}.
* <p>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)}.
*
* <p>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
* null if the manager doesn't contain the value or the manager is closed.
*/
@Nullable
public Long getIdentifierForStrongReference(Object instance) {
if (isClosed()) {
Log.w(TAG, CLOSED_WARNING);
if (assertNotClosed()) {
return null;
}
final Long identifier = identifiers.get(instance);
Expand All @@ -120,18 +127,18 @@ public Long getIdentifierForStrongReference(Object instance) {
/**
* Adds a new instance that was instantiated from Dart.
*
* <p>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.
* <p>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.
*
* <p>If the manager is closed, the addition is ignored.
* <p>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);
Expand All @@ -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);
Expand All @@ -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 <T> 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> T getInstance(long identifier) {
if (isClosed()) {
Log.w(TAG, CLOSED_WARNING);
if (assertNotClosed()) {
return null;
}

final WeakReference<T> instance = (WeakReference<T>) weakInstances.get(identifier);
if (instance != null) {
return instance.get();
}
return (T) strongInstances.get(identifier);
return null;
}

/**
Expand All @@ -182,8 +194,7 @@ public <T> 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);
Expand All @@ -197,14 +208,23 @@ public boolean containsInstance(Object instance) {
public void close() {
handler.removeCallbacks(this::releaseAllFinalizedInstances);
isClosed = true;
clear();
}

/**
* Removes all of the instances from this manager.
*
* <p>The manager will be empty after this call returns.
*/
public void clear() {
identifiers.clear();
weakInstances.clear();
strongInstances.clear();
weakReferencesToIdentifiers.clear();
}

/**
* Whether the manager has released resources and is not longer usable.
* Whether the manager has released resources and is no longer usable.
*
* <p>See {@link #close()}.
*/
Expand All @@ -228,12 +248,24 @@ 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<Object> weakReference = new WeakReference<>(instance, referenceQueue);
identifiers.put(instance, identifier);
weakInstances.put(identifier, weakReference);
weakReferencesToIdentifiers.put(weakReference, identifier);
strongInstances.put(identifier, instance);
}

private boolean assertNotClosed() {
if (isClosed()) {
Log.w(TAG, CLOSED_WARNING);
return true;
}
return false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -111,6 +112,11 @@ Future<void> 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,
);
Expand Down
Loading