[webview_flutter] Implement loadRequest in Android package.

[BeMacized]

This PR supersedes #4479.

It implements the loadRequest method added in #4450 for the Android package.

Related issue:

• WebView for Flutter - Allow the POST of data on initial URL (instead of a GET) flutter#27730

Pre-launch Checklist

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• I read the Tree Hygiene wiki page, which explains my responsibilities.
• 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.)
• I signed the CLA.
• The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
• I listed at least one issue that this PR fixes in the description above.
• I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.
• I updated CHANGELOG.md to add a description of the change, following repository CHANGELOG style.
• 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.
• All existing and new tests are passing.

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

[stuartmorgan-g]

We try to limit dependencies as much as possible. It seems like we should strongly consider having this just be a documented limitation on Android, and list this as a workaround. Someone could even publish it as a separate package.

[BeMacized]

Just for context: The native postUrl method for webviews on Android lacks the ability to set http headers. As such, when headers are to be set, the request has to be made manually and its response be loaded in with loadDataWithBaseURL.

@stuartmorgan I'm not sure documenting this issue instead of providing a working implementation would be the right way to go:

Personally I'd believe not being able to use loadRequest for making post requests with headers on Android is a pretty major oversight, as it would prevent users from, for example, passing tokens through Authorization headers without doing their own workaround.

Coupled with the fact that the http package is not just any random package, but an official dart-lang package, I'd urge you to consider it an acceptable tradeoff, having it as a dependency for users not having to deal with a platform specific shortcoming.

[stuartmorgan-g]

Personally I'd believe not being able to use loadRequest for making post requests with headers on Android is a pretty major oversight, as it would prevent users from, for example, passing tokens through Authorization headers without doing their own workaround.

I understand the use case, but I'm not seeing what compelling advantage is coming from having the workaround be embedded into the package rather than in another package.

I'm also concerned about unexpected side-effects here: I suspect that there are cases where loading data isn't going to have identical behavior to loading a site normally would, and with this approach there's no indication to clients of this package that those things would happen. If they are using a separate package for the workaround, they will be explicitly opting into those differences.

Coupled with the fact that the http package is not just any random package, but an official dart-lang package, I'd urge you to consider it an acceptable tradeoff

My concern in this case with the dependency is not about random vs. official, but about compatibility. Version conflicts in transitive dependencies with http is a very real possibility.

having it as a dependency for users not having to deal with a platform specific shortcoming.

Embedding the workaround into the package forces every user of webview_flutter—most of whom will never do post requests with headers—to suffer the potential consequences of version conflicts with http.

[stuartmorgan-g]

Also: we can easily add this later if it turns out that having it be a separate package is extremely onerous for lots of users. Removing it if we add it now and regret it, on the other hand, is a breaking change.

[BeMacized]

I understand the use case, but I'm not seeing what compelling advantage is coming from having the workaround be embedded into the package rather than in another package.

@stuartmorgan The advantage here is usability. As a user, finding out I have to resort to workarounds or separate packages to just basically "patch" functionality that I expected to work right out of the box, isn't a great experience. Having it embedded would therefore (to me) be a compelling advantage.

That said, I do understand your concerns now, so I've removed the workaround and documented the limitation + workaround in the method doc, as well as for the method and readme for the app facing package (#4573).

[BeMacized]

Justs some extra thoughts:

The need for this dependency stemmed from our move to pigeon. Before mapping the native api's one to one, Android's native HTTP client was used for this workaround.

Maybe a possibility for the future could be to also map parts of Android's bundled HttpClient so that this workaround can still be implemented without requiring a new dependency.

I suspect that there are cases where loading data isn't going to have identical behavior to loading a site normally would

We could potentially get around this by making the request manually for every call to loadRequest? Although at this point that would probably be a breaking change on iOS.

[stuartmorgan-g]

Maybe a possibility for the future could be to also map parts of Android's bundled HttpClient so that this workaround can still be implemented without requiring a new dependency.

That would definitely be much better than adding http.

We could potentially get around this by making the request manually for every call to loadRequest?

My concern is that there are going to be subtle things that are broken; unnecessarily breaking those things all the time for consistency doesn't address my core concern, it makes it much worse.

[stuartmorgan-g]

Just some nits.

[stuartmorgan-g]

LGTM!

[fluttergithubbot]

This pull request is not suitable for automatic merging in its current state.

• This commit is not mergeable and has conflicts. Please rebase your PR and fix all the conflicts.

[stuartmorgan-g]

Overriding incorrect submit-queue status.