[webview_flutter_android] [camera_android_camerax] Updates internal Java InstanceManager to only stop finalization callbacks when stopped #3571

bparrishMines · 2023-03-28T16:36:43Z

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

…ger_update

camsim99 · 2023-03-29T15:49:31Z

...camera_android_camerax/android/src/main/java/io/flutter/plugins/camerax/InstanceManager.java

    handler.removeCallbacks(this::releaseAllFinalizedInstances);
-    isClosed = true;
-    clear();
+    hasFinalizationListenerStopped = true;


Just to make sure I understand correctly, can you confirm the following is correct?

After this method called, objects that have been added to this instance manager can be garbage collected. Thus, objects might be contained by the instance manager (because we aren't releasing the resources/calling clear() anymore), but also might not. So basically, the instance manager is now unreliable, but with this change, it's up the developer to be aware of that versus getting a null, -1, etc. value back when attempting to call a method?

When a new instance is added to the manager, the manager creates a strong reference and a weak reference to the instance. When remove(Object) is called, the strong reference is removed. Therefore, the weak reference to the instance will remain until it is garbage collected. The manager now functions this way whether this value is true or false. The only difference now is that the finalizationListener that was passed in InstanceManager.create is no longer called when the weak reference to an instance is claimed by garbage collection.

Oh, I see. This makes sense!

Is it possible to test the case where the weak reference was claimed by garbage collection to see that the finalizationListener is no longer called? I figure it's not but asking just in case!

It is possible with the Runtime.getRuntime().gc(); method. I tried to add a unit test, but the Android Handler class is just a stub when running unit tests. So I created an integration test.

…ger_update

camsim99

LGTM! Just left a suggestion about a test

camsim99 · 2023-04-14T17:18:24Z

...camera_android_camerax/android/src/main/java/io/flutter/plugins/camerax/InstanceManager.java

    handler.removeCallbacks(this::releaseAllFinalizedInstances);
-    isClosed = true;
-    clear();
+    hasFinalizationListenerStopped = true;


Oh, I see. This makes sense!

packages/webview_flutter/webview_flutter_android/CHANGELOG.md

camsim99 · 2023-04-14T17:20:55Z

...camera_android_camerax/android/src/main/java/io/flutter/plugins/camerax/InstanceManager.java

    handler.removeCallbacks(this::releaseAllFinalizedInstances);
-    isClosed = true;
-    clear();
+    hasFinalizationListenerStopped = true;


Is it possible to test the case where the weak reference was claimed by garbage collection to see that the finalizationListener is no longer called? I figure it's not but asking just in case!

packages/webview_flutter/webview_flutter_android/CHANGELOG.md

...flutter_android/android/src/main/java/io/flutter/plugins/webviewflutter/InstanceManager.java

reidbaker · 2023-04-14T17:57:29Z

...flutter_android/android/src/main/java/io/flutter/plugins/webviewflutter/InstanceManager.java

    handler.removeCallbacks(this::releaseAllFinalizedInstances);
-    isClosed = true;
-    clear();
+    hasFinalizationListenerStopped = true;


Is it possible to derive this value instead of using a boolean?
For example if handler.removeCallbacks does not have any more callbacks?

It looks like there is a Handler.hasCallbacks, but it requires Android 29+ unfortunately.

…ternal Java InstanceManager to only stop finalization callbacks when stopped (flutter/packages#3571)

flutter/packages@e4ec155...0277f2a 2023-04-16 engine-flutter-autoroll@skia.org Roll Flutter from 00171b0 to 50171bb (7 revisions) (flutter/packages#3723) 2023-04-15 engine-flutter-autoroll@skia.org Roll Flutter from f740544 to 00171b0 (17 revisions) (flutter/packages#3717) 2023-04-15 49699333+dependabot[bot]@users.noreply.github.com Bump github/codeql-action from 2.2.9 to 2.2.12 (flutter/packages#3711) 2023-04-15 49699333+dependabot[bot]@users.noreply.github.com Bump actions/checkout from 3.5.0 to 3.5.2 (flutter/packages#3710) 2023-04-14 stuartmorgan@google.com [path_provider] Fix Android lint warnings (flutter/packages#3706) 2023-04-14 10687576+bparrishMines@users.noreply.github.com [webview_flutter_android] [camera_android_camerax] Updates internal Java InstanceManager to only stop finalization callbacks when stopped (flutter/packages#3571) 2023-04-14 stuartmorgan@google.com [shared_preferences] Fix Android Java version issue (flutter/packages#3712) 2023-04-14 48185017+andreisas06@users.noreply.github.com [image_picker][android] Non-bitmap images now return path instead of null (flutter/packages#3590) 2023-04-14 engine-flutter-autoroll@skia.org Roll Flutter from be45eb2 to f740544 (24 revisions) (flutter/packages#3713) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC flutter-ecosystem@google.com,rmistry@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

…ava InstanceManager to only stop finalization callbacks when stopped (flutter#3571)

bparrishMines added 5 commits March 28, 2023 11:09

update webview_flutter instance mangaer

1fe8a4e

camerax side of change

412036c

version bump

ee44f2f

version bump camerax

f73f110

stopped

c5fe12e

bparrishMines requested a review from camsim99 as a code owner March 28, 2023 16:36

Merge branch 'main' of github.com:flutter/packages into instance_mana…

10899d0

…ger_update

github-actions bot added p: camera p: webview_flutter platform-android labels Mar 28, 2023

bparrishMines changed the title ~~Instance manager update~~ [webview_flutter_android] [camera_android_camerax] Updates internal Java Mar 28, 2023

bparrishMines changed the title ~~[webview_flutter_android] [camera_android_camerax] Updates internal Java~~ [webview_flutter_android] [camera_android_camerax] Updates internal Java InstanceManager to only stop finalization callbacks when stopped Mar 28, 2023

bparrishMines added 2 commits March 28, 2023 12:45

add test

c42d4da

remove tests

cb0c128

bparrishMines requested a review from reidbaker March 28, 2023 19:11

bparrishMines mentioned this pull request Mar 28, 2023

WebViewClientFlutterApiImpl.requestLoading: java.lang.NullPointerException flutter/flutter#123004

Closed

camsim99 reviewed Mar 29, 2023

View reviewed changes

gmackall requested a review from camsim99 April 13, 2023 18:43

Merge branch 'main' of github.com:flutter/packages into instance_mana…

7379b40

…ger_update

camsim99 approved these changes Apr 14, 2023

View reviewed changes

reidbaker approved these changes Apr 14, 2023

View reviewed changes

bparrishMines added 2 commits April 14, 2023 15:46

add integration test

2ec0a60

make constant private again

c559338

bparrishMines added the autosubmit Merge PR when tree becomes green via auto submit App label Apr 14, 2023

auto-submit bot merged commit 44207e7 into flutter:main Apr 14, 2023

bparrishMines deleted the instance_manager_update branch April 14, 2023 20:57

engine-flutter-autoroll mentioned this pull request Apr 17, 2023

Roll Packages from e4ec15590935 to 0277f2ad9d92 (9 revisions) flutter/flutter#124972

Merged

reidbaker mentioned this pull request Apr 25, 2023

[various] update agp and gradle for all examples in packages #3822

Merged

11 tasks

nploi pushed a commit to nploi/packages that referenced this pull request Jul 16, 2023

[webview_flutter_android] [camera_android_camerax] Updates internal J…

058bbe8

…ava InstanceManager to only stop finalization callbacks when stopped (flutter#3571)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[webview_flutter_android] [camera_android_camerax] Updates internal Java InstanceManager to only stop finalization callbacks when stopped #3571

[webview_flutter_android] [camera_android_camerax] Updates internal Java InstanceManager to only stop finalization callbacks when stopped #3571

Uh oh!

bparrishMines commented Mar 28, 2023 •

edited

Loading

Uh oh!

camsim99 Mar 29, 2023

Uh oh!

bparrishMines Mar 30, 2023

Uh oh!

camsim99 Apr 14, 2023

Uh oh!

camsim99 Apr 14, 2023

Uh oh!

bparrishMines Apr 14, 2023

Uh oh!

camsim99 left a comment

Uh oh!

camsim99 Apr 14, 2023

Uh oh!

Uh oh!

camsim99 Apr 14, 2023

Uh oh!

Uh oh!

Uh oh!

reidbaker Apr 14, 2023

Uh oh!

bparrishMines Apr 14, 2023

Uh oh!

Uh oh!

[webview_flutter_android] [camera_android_camerax] Updates internal Java InstanceManager to only stop finalization callbacks when stopped #3571

[webview_flutter_android] [camera_android_camerax] Updates internal Java InstanceManager to only stop finalization callbacks when stopped #3571

Uh oh!

Conversation

bparrishMines commented Mar 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

camsim99 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bparrishMines commented Mar 28, 2023 •

edited

Loading