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

[webview_flutter] Added Android implementation of PlatformWebViewController #6674

Merged

Conversation

mvanbeusekom
Copy link
Contributor

Adds the Android implementation of the PlatformWebViewController to the v4 Android implementation.

Part of issue flutter/flutter#94051

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.
  • [ 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.)
  • 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.

Copy link
Contributor

@stuartmorgan-g stuartmorgan-g left a comment

Choose a reason for hiding this comment

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

Mostly looks good, but with the caveat that as I mentioned I'm not immersed in this code so I would likely not catch subtle errors (e.g., around the weak handling, or proxying). @bparrishMines should ideally take a look, but if he isn't able to review soon we can land it to unblock the next steps, and he can do a post-landing review for any follow-up since presumably issue would be internal details, not API-level.

Copy link
Contributor

@bparrishMines bparrishMines left a comment

Choose a reason for hiding this comment

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

LGTM

@stuartmorgan this and other PRs are getting the error:

❗ Failed to stop: Error getting access token for service account: 400 Bad Request
POST https://oauth2.googleapis.com/token
{"error":"invalid_grant","error_description":"Invalid JWT Signature."}, iss: [[email protected]](mailto:[email protected])

Could this be caused by this PR being merge to a branch that isn't main?

@bparrishMines
Copy link
Contributor

It looks like pulling in the latest changes from the main branch into v4_webview fixes these test errors: #6713. Could you review this PR?

@stuartmorgan-g
Copy link
Contributor

That was probably due to a credential rotation.

@mvanbeusekom mvanbeusekom merged commit 341fe29 into flutter:v4_webview Nov 19, 2022
@mvanbeusekom mvanbeusekom deleted the v4_webview_android_webview_controller branch November 19, 2022 16:29
@bparrishMines
Copy link
Contributor

@mvanbeusekom It looks like this PR still had some failing analysis errors. Can you fix them in your next PR?

Analyzing webview_flutter_android...

   info - lib/src/v4/src/android_webview_controller.dart:17:8 - Unused import: '../../android_webview.dart'. Try removing the import directive. - unused_import
   info - lib/src/v4/src/android_webview_controller.dart:18:8 - Unused import: '../../instance_manager.dart'. Try removing the import directive. - unused_import

@mvanbeusekom
Copy link
Contributor Author

@mvanbeusekom It looks like this PR still had some failing analysis errors. Can you fix them in your next PR?

Analyzing webview_flutter_android...

   info - lib/src/v4/src/android_webview_controller.dart:17:8 - Unused import: '../../android_webview.dart'. Try removing the import directive. - unused_import
   info - lib/src/v4/src/android_webview_controller.dart:18:8 - Unused import: '../../instance_manager.dart'. Try removing the import directive. - unused_import

Sorry about that, completely missed it. PR #6732 should fix this.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
p: webview_flutter Edits files for a webview_flutter plugin platform-android
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants