[webview_flutter] Proposal to share local storage across multiple webview

[EArminjon2]

trafficstars

This PR aim to respect the web convention about sharing the local storage between multiple webview instances.

https://github.com/flutter/flutter/issues/94591

If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.

Pre-launch Checklist

• [x] I read the Contributor Guide and followed the process outlined there for submitting PRs.
• [x] I read the Tree Hygiene wiki page, which explains my responsibilities.
• [x] I read and followed the relevant style guides and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/plugins repo does use dart format.)
• [x] I signed the CLA.
• [x] The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
• [x] I listed at least one issue that this PR fixes in the description above.
• [x] I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.
• [x] I updated CHANGELOG.md to add a description of the change, following repository CHANGELOG style.
• [x] I updated/added relevant documentation (doc comments with ///).
• [ ] I added new tests to check the change I am making, or this PR is test-exempt.
• [x] All existing and new tests are passing.

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

[flutter-dashboard[bot]]

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

[bparrishMines]

@EA-YOUHOU This looks like a feature that we would want to be configurable. We are currently in the process of restructuring the plugin to make this possible: https://github.com/flutter/flutter/issues/93732 https://github.com/flutter/flutter/issues/94051

However, it is concerning that Android and iOS have different default behavior. That is something we could fix now, but I'm not familiar enough with web development to know which is better as the default.

It looks like this is unchangeable on Android, but it depends on the version. And there is a way to check if it is: https://developer.android.com/about/versions/oreo/android-8.0-changes#security-all https://developer.android.com/reference/android/webkit/WebView#getWebViewRenderProcess(). I would still need to verify this is referring to the same feature.

Assuming that getWebViewRenderProcess is referring to the same feature as setting WKProcessPool, it would be safer to make this configurable after the two issues I stated at the top are finished. Mainly because the default behavior would remain inconsistent across certain versions of Android.

cc @stuartmorgan

[stuartmorgan-g]

but I'm not familiar enough with web development to know which is better as the default.

It depends entirely on the use case. If it's loading arbitrary web content from arbitrary sources (e.g., a browser), the less process sharing the better*; separate processes provide an extra firewall in case of an exploit against the web view, and also avoid a single crash bringing down all other web views in the application. If it's, say, several pieces of content from a specific domain, sharing a single render process is probably a better option. (The separation isn't perfect even if we do nothing; as the docs say, eventually it will share regardless).

I think there's probably a reasonable argument for sharing as the default behavior as long as it remains configurable.

(* even this depends on what the metric is; it's better for security and stability, but almost certainly worse for resource usage.)

I would still need to verify this is referring to the same feature.

Not exactly, from looking at the Android docs:

In Build.VERSION_CODES.O and above, WebView may run in "multiprocess" mode. In multiprocess mode, rendering of web content is performed by a sandboxed renderer process separate to the application process. This renderer process may be shared with other WebViews in the application, but is not shared with other application processes.

WKWebView always runs in what these docs call "multiprocess mode". What WKProcessPool allows more detailed control over is the "This renderer process may be shared with other WebViews in the application" part.

[jmagman]

If I'm following, this PR needs to be updated to make the shared behavior configurable? Or is that blocked on the work @bparrishMines mentioned?

[stuartmorgan-g]

Yes, since making it configurable will require various Dart-side changes, it's blocked on the in-progress reworking of the plugin's Dart code (both platform interface and iOS implementation).

[stuartmorgan-g]

https://github.com/flutter/flutter/issues/93732 is complete, so this is unblocked; you'll just need to update the implementation for the new Dart-based implementation (and make it a configuration option at creation time, per the discussion above).

[stuartmorgan-g]

@EArminjon2 Are you still planning on updating this per the discussion above?

[EArminjon]

@stuartmorgan ,

I didn't know enough objective C to do that ...

[stuartmorgan-g]

Okay, I'm going to go ahead and close this then, since it's not something we can proceed with without those changes. Please feel free to open a new PR if you decide to revisit this later!