[webview_flutter_android] Fixes iframe navigation with onNavigationRequest

[petermnt]

This PR reintroduces the check on WebResourceRequest.isForMainFrame in the shouldOverrideUrlLoading callback.
This check already existed in the implementation before the change to Pigeon, but was not taken over. See

packages/packages/webview_flutter/webview_flutter_android/android/src/main/java/io/flutter/plugins/webviewflutter/FlutterWebViewClient.java

Line 99 in 418bef0

return request.isForMainFrame();

The reasoning was already already in comment there:

      // Since we cannot call loadUrl for a subframe, we currently only allow the delegate to stop
      // navigations that target the main frame, if the request is not for the main frame
      // we just return false to allow the navigation.
      //
      // For more details see: https://github.com/flutter/flutter/issues/25329#issuecomment-464863209

Currently when a NavigationDeletage.onNavigationRequest is set and a navigation request for an iframe happens it will automatically be blocked and handed to the Flutter onNavigationRequest.
If the request gets a NavigationDecision.navigate it will be loaded into the mainframe through a loadUrl.
If the request gets a NavigationDecision.prevent it will never be executed.

This causes an issue when for example an iframe loads local data.
Example
data:text/html,<script>onresize=function(){parent.postMessage(0,'*')}<\/script>

An iframe is allowed to do this, but with the current implementation this data will be loaded in the main frame, leading to a white page.

Issue: flutter/flutter#145208

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.

[bparrishMines]

This is a good fix! It looks like I missed this code in the pre webview v4 code: https://github.com/flutter/plugins/pull/1236/files#diff-8ba24b285f0228d5dd613ad64b1e63f6ce4be9819ec1a7e102a2dd269de569c6R53

I think we should also change AndroidWebViewController to only call onNavigationRequest for the main frame. Otherwise, someone may return prevent to the callback and it would be ignored for an unknown reason.

[petermnt]

I think we should also change AndroidWebViewController to only call onNavigationRequest for the main frame. Otherwise, someone may return prevent to the callback and it would be ignored for an unknown reason.

I was thinking the same, but for now restored the 'old' behaviour. I guess it was like that to still inform the flutter side.

My current fix indeed needs this in our implementation, which doesn't make much sense.

  if (!navigation.isMainFrame) {
      return _platformRepo.isIOS ? NavigationDecision.navigate : NavigationDecision.prevent;
    }

I will apply your suggestion tomorrow.

[bparrishMines]

Most of this looks good. Just a few more comments.

@stuartmorgan This is ready for a secondary review. And what are your thoughts on this being a bug fix on needing to be a breaking change?

[bparrishMines]

LGTM

@stuartmorgan for secondary review

[stuartmorgan]

LGTM