[webview_flutter_web] Migrate to package:web.

[ThomasAunvik]

• Migrates to package:web [webview_flutter] migrate to pkg:web flutter#139749
• Updates HttpRequestFactory.request to use package:http BrowserClient
• Updates ìndex.html in the example to use flutter_bootstrap.js
• Updates minimum dart sdk requirement to ^3.3.0

Would appreciate help with completing the mock tests if in case it does not work.
(I am somehow stuck with 'loading...' when attempting to test with mockito with --platform chrome)

Integration tests from what i was able to test passes.

Migrated to using BrowserClient for web due to issues creating mock tests with XMLHttpRequest which is returned from package:web's HttpRequest.request following error:

Bad state: Interface type 'XMLHttpRequest' which is nether an enum, nor a class, nor a mixin. This case is unknown, please report a bug.

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/packages repo does use dart format.)
• I signed the CLA.
• The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
• I linked to 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.

Co-authored-by: David Iglesias [email protected]
Co-authored-by: Navaron Bracke [email protected]

[ThomasAunvik]

The tests that seem to be failing now are just the ones in the legacy/ folder, that we require to create a mock HTMLIFrameElement.

And these two calls that i do not understand what it is trying to do.
Based on the lsp, argThat just returns null and mockElement.src cannot be set to a null, can only be set to String.

packages/packages/webview_flutter/webview_flutter_web/test/legacy/webview_flutter_web_test.dart

Line 84 in a598bef

verify(mockElement.src = argThat(contains('%23')) ?? '');

packages/packages/webview_flutter/webview_flutter_web/test/legacy/webview_flutter_web_test.dart

Line 178 in a598bef

verify(mockElement.src = argThat(contains('%23')) ?? '');

Legacy tests N-2 are failing due to the package:web having a higher dart sdk requirement than the test has installed.
currently web version is set to version ^0.5.1, it suggests it to be set to ^0.3.0. Reading the changelog of the package, i don't see any breaking changes on it. So i assume it should be fine to lower the versioning?

I can go down for web to ^0.3.0 and http to ^1.2.0, but HTMLIFrameElement is missing a src getter which the tests needs.
But if we going to need to create a mock for it, then I assume it should be fine to downgrade, if we can have the mock to have a getter on it.

nvm, package:web included an extension to use a getter to HTMLIFrameElement.src.

[ditman]

Thanks for the PR @ThomasAunvik! I'll take a look!

(Please take into account that next monday is a holiday in the US, so I won't be able to get to this until next tuesday at the earliest)

[uldall]

I have been experimenting with this PR for a few days, and run into the issue that the created <iframe> doesn't have the credentialless attribute set, and so won't load if the iframe'ed document doesn't have the proper headers when being GET'ed. See:
https://developer.mozilla.org/en-US/docs/Web/API/HTMLIFrameElement/credentialless and https://developer.mozilla.org/en-US/docs/Web/Security/IFrame_credentialless

I think it would make sense to make it possible to specify whether this attribute should be set from Flutter code?

[ThomasAunvik]

https://developer.mozilla.org/en-US/docs/Web/API/HTMLIFrameElement/credentialless and https://developer.mozilla.org/en-US/docs/Web/Security/IFrame_credentialless

Looking at the compatability, firefox and safari is not yet supported, but an option to enable it would be beneficial once it is fully supported.

Otherwise the current solution for all browsers is that the hosted webapp has the Cross-Origin-Embedder-Policy: credentialless header set.

[uldall]

Otherwise the current solution for all browsers is that the hosted webapp has the Cross-Origin-Embedder-Policy: credentialless header set.

In most cases people don't own the sites that they iframe, and it can be very hard to convince a third party to add support for these headers.

Looking at the compatability, firefox and safari is not yet supported, but an option to enable it would be beneficial once it is fully supported.

I don't see why we would wait. The WASM builds are already only supported in Chromium based browsers. But I also understand if you don't want to add it in this PR :)

[ThomasAunvik]

In most cases people don't own the sites that they iframe, and it can be very hard to convince a third party to add support for these headers.

Nevermind i got confused on how the cors weirdness works, as i did pain with it too. It would work on setting credentialless on the iframe.

It can be easily done by adding this right after loadRequest.

_webWebViewParams.iFrame.setAttribute('credentialless', 'true');

As the current HTMLIFrameElement doesn't have it implemented directly yet.

Would require me to update the LoadRequestParams in webview_flutter_platform_interface to include a boolean for credentialless.

[ThomasAunvik]

Here is a proposal on how setting the credentialless would be.
This is added under WebWebViewController

/// Sets the IFrame Credentialless
/// https://developer.mozilla.org/en-US/docs/Web/Security/IFrame_credentialless
void setIFrameCredentialless(bool flag) {
  _webWebViewParams.iFrame
      .setAttribute('credentialless', flag ? 'true' : 'false');
}

And to set it would be as such:

if(WebViewPlatform.instance is WebWebViewPlatform) {
    WebWebViewController(
        const PlatformWebViewControllerCreationParams(),
    )
    ..setIFrameCredentialless(true)
    ..loadRequest(
        LoadRequestParams(
            uri: Uri.parse('https://flutter.dev'),
        ),
    ))
}

setIFrameCredentialless is would most likely be the one that requires to be set before loadRequest, or at the same time.

I have set up a branch for it: https://github.com/ThomasAunvik/flutter-packages/tree/wasmcredentialless

Would like feedback.

Another issue would be that _updateIFrameFromXhr would still have a cors issue.

[uldall]

It seems that on other platforms these kind of platform specific parameters are set on the WebViewControllerCreationParams implementation.

An example from the Webkit impl. Here some boolean parameters like allowsInlineMediaPlayback and limitsNavigationsToAppBoundDomains are set in the WebKitWebViewControllerCreationParams. Seems similar to our credentialless use case.

I am by no means well versed in the Flutter codebase, but this was just my best effort on some feedback. I will be going on vacation for a week, but will test the new branch when I am back :)

[kevmoo]

What's the status here, friends?

[uldall]

I managed to test the credentialless branch just now, and it works as expected and the contents of the iframe is loaded:

[kevmoo]

@ditman would you take a look?

[uldall]

I have been testing the PR quite a bit, and created this issue because Godot games being loaded in the iframe resulted in an error: flutter/flutter#150119

However since then I have also started getting the error on other sites (for example https://flutter.dev/). It does however work great on a payment site that I use. (reepay.com). I am not sure what properties of the iframe'ed site makes it throw the error.

Maybe the error is unrelated to this PR, and probably a bug in the skwasm renderer?

[ditman]

This is next on my radar, after this lands:

• [ci] Add Wasm compilation to all_packages web app. #6981

[ditman]

@ThomasAunvik I've finally had some time to review this! Instead of adding a bunch of comments and back and forth with the code, I just force pushed (please, remember to git pull --rebase on your checkout) the changes I thought your branch needed.

(TL;DR: I removed the package:http dependency and used package:web fetch instead, and cleaned up the mocks in tests a little bit with the new tricks that we can do with the latest JS-Interop 😉)

Do you mind to make a quick re-review yourself, ensure that the changes I added make sense, and verify that the plugin still does what you need? Tests seem to be passing, and the example app seems to work as expected, but the more use cases, the merrier!

[navaronbracke]

I don't think we have support for the JS Map type yet (in JS interop that is), so perhaps we should add a TODO for that? Unless we have a constructor for web.HeadersInit that takes a Map<String, String> ?

In my opinion we can eventually get rid of the jsify() in the long run?

Not sure how @ditman feels about doing that, though?

[ditman]

(@navaronbracke I wrote the .jsify() bits :P)

Currently HeadersInit is just an alias of JSObject.

The init in JS itself is a vague object-like thing that doesn't have a very specific way of being constructed: Headers init parameter:

init
An object containing any HTTP headers that you want to pre-populate your Headers object with. This can be a simple object literal with String values, an array of name-value pairs, where each pair is a 2-element string array; or an existing Headers object. In the last case, the new Headers object copies its data from the existing Headers object.

I'm using the simplest/most legible way I've found so far of creating an object literal of a JS-interop type (a Map<String, Object?>.jsify()! as JsInteropType). I agree that a more semantic constructor in package:web could be better, but this way I can use all the power of Dart Maps until the very last moment. /cc @srujzs for The Ultimate Expert Opinion™!

[ditman]

(Maybe something with _createObjectLiteral? source)

[srujzs]

We could add a better constructor here if that's easier to read, but it's not going to do anything more complicated than just jsify :) e.g.

JSObject.fromMap(Map m) => m.jsify() as JSObject;

[ditman]

@srujzs yeah, I'd like to have a jsify in non-nullable-object so I don't have to Map.jsify()! and maybe a T jsifyAs<T>() that does the casting for me, so I could replace:

headers.jsify()! as web.HeadersInit

by

headers.jsifyAs<web.HeadersInit>();

(Not that it saves too much typing, though)

[ditman]

Just verified in CI that this package enables the webview_flutter package to compile to Wasm! 🎉

Exluding the following plugins from the combined build:
  plugin_platform_interface
  camera
  google_maps_flutter

If this works for @ThomasAunvik, I think it's ready to land!

[ThomasAunvik]

Works as expected.

Experiencing an issue when re-opening the iframe (and be using the same pdf blob data / url), iframe doesn't render properly, showing nothing just the background of the parent widget, until resizing the window. (on wasm only).
I'll have to test it a bit more.

[ThomasAunvik]

Created a reproducible example: ThomasAunvik/flutter_webview_wasm_error@f75fddb

Seems like this time I managed to make the iframe not appear at all when using Navigator.of(context).push()

Left is wasm on chrome, right is non-wasm using firefox.

error.mp4

Unless it is an issue with this package alone, i don't see it to be needing to not merge it with this issue, if that is also just a lower level flutter issue.

[ditman]

@ThomasAunvik I think you've encountered something like this:

• [dart2wasm] iFrame locks position on transition into view flutter#149909

Platform Views in Wasm have some positioning issues yet (it can also be seen with the default example app, I think)

[ditman]

Been here, seen this!

[ditman]

Let's try to autoland the PR!

[auto-submit]

auto label is removed for flutter/packages/6792, due to This PR has not met approval requirements for merging. The PR author is not a member of flutter-hackers and needs 1 more review(s) in order to merge this PR.

• Merge guidelines: A PR needs at least one approved review if the author is already part of flutter-hackers or two member reviews if the author is not a flutter-hacker before re-applying the autosubmit label. Reviewers: If you left a comment approving, please use the "approve" review action instead.

[kevmoo]

Rerunning failures and adding back auto-submit!

[auto-submit]

auto label is removed for flutter/packages/6792, due to This PR has not met approval requirements for merging. The PR author is not a member of flutter-hackers and needs 1 more review(s) in order to merge this PR.

• Merge guidelines: A PR needs at least one approved review if the author is already part of flutter-hackers or two member reviews if the author is not a flutter-hacker before re-applying the autosubmit label. Reviewers: If you left a comment approving, please use the "approve" review action instead.