Skip to content
This repository has been archived by the owner on Feb 22, 2023. It is now read-only.

[webview_flutter] Only call onWebResourceError for main frame #3078

Merged
merged 9 commits into from Aug 10, 2021

Conversation

bparrishMines
Copy link
Contributor

@bparrishMines bparrishMines commented Sep 24, 2020

Description

onWebResourceError is called only for main frame on iOS and Android version below 23. This makes Android versions 23+ only use this callback for main frame as well.

Related Issues

Fixes flutter/flutter#64925

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • My PR includes unit or integration tests for all changed/updated/fixed behaviors (See Contributor Guide).
  • All existing and new tests are passing.
  • I updated/added relevant documentation (doc comments with ///).
  • The analyzer (flutter analyze) does not report any problems on my PR.
  • I read and followed the Flutter Style Guide.
  • The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy.
  • I updated CHANGELOG.md to add a description of the change.
  • I signed the CLA.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

  • Yes, this is a breaking change (please indicate a breaking change in CHANGELOG.md and increment major revision).
  • No, this is not a breaking change.

@stuartmorgan stuartmorgan added the p: webview_flutter Edits files for a webview_flutter plugin label Jan 29, 2021
@stuartmorgan
Copy link
Contributor

Looks like this fell through the cracks. Could you add a version bump and resolve the conflict here? Other than that, it looks good to go.

@@ -1146,8 +1146,48 @@ void main() {
);

expect(errorCompleter.future, doesNotComplete);
await Future.delayed(Duration(seconds: 5));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I missed this when looking at the PR before; why is there a 5-second sleep at the end of these tests? At the very least this needs a clear comment, but is there really not some reliable thing we can wait for to do whatever this is intended to do, rather than relying on timing?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I believed there was a chance for an error to occur after the page finished loading, but I just tested it and that doesn't look like the case for the url provided. I changed it to wait for onPageFinished.

@bparrishMines bparrishMines changed the title [WIP] [webview_flutter] Only call onWebResourceError for main frame [webview_flutter] Only call onWebResourceError for main frame Aug 5, 2021
Copy link
Contributor

@stuartmorgan stuartmorgan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@bparrishMines bparrishMines added the waiting for tree to go green (Use "autosubmit") This PR is approved and tested, but waiting for the tree to be green to land. label Aug 10, 2021
@fluttergithubbot fluttergithubbot merged commit d31bd7d into flutter:master Aug 10, 2021
@bparrishMines bparrishMines deleted the webresource branch August 10, 2021 18:57
fotiDim pushed a commit to fotiDim/plugins that referenced this pull request Sep 13, 2021
amantoux pushed a commit to amantoux/plugins that referenced this pull request Sep 27, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes p: webview_flutter Edits files for a webview_flutter plugin platform-android waiting for tree to go green (Use "autosubmit") This PR is approved and tested, but waiting for the tree to be green to land.
Projects
None yet
5 participants