Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

[webview_flutter_android] Fixes crash when the Java InstanceManager was used after plugin was removed from engine #6943

Merged
merged 7 commits into from
Jan 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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.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`.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -79,11 +82,15 @@ private InstanceManager(FinalizationListener finalizationListener) {
*
* @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);
}

Expand All @@ -95,11 +102,14 @@ public <T> 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);
Expand All @@ -114,22 +124,30 @@ 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.
*
* <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);
}

/**
* 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;
Expand All @@ -141,11 +159,14 @@ public long addHostCreatedInstance(Object instance) {
* @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();
Expand All @@ -157,22 +178,38 @@ public <T> 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.
*
* <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() {
Expand All @@ -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.");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down