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

[webview_flutter] Fix debuggingEnabled on Android #4859

Merged

Conversation

stuartmorgan-g
Copy link
Contributor

The recent Dart rewrite accidentally dropped a parameter value in the
call chain, causing disabling debugging to enabled it instead.

This fixes the issue, and adds testing of that portion of the call
chain (which was entirely missing), as well as adding testing of false
at various other points just for added coverage.

As opportunistic cleanup, changes the test group naming, as the pattern
of using '$FooClass' breaks test result display in VSCode.

Fixes flutter/flutter#98521

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.

The recent Dart rewrite accidentally dropped a parameter value in the
call chain, causing disabling debugging to enabled it instead.

This fixes the issue, and adds testing of that portion of the call
chain (which was entirely missing), as well as adding testing of `false`
at various other points just for added coverage.

As opportunistic cleanup, changes the test group naming, as the pattern
of using `'$FooClass'` breaks test result display in VSCode.

Fixes flutter/flutter#98521
@github-actions github-actions bot added p: webview_flutter Edits files for a webview_flutter plugin platform-android labels Feb 15, 2022
@stuartmorgan-g
Copy link
Contributor Author

We should look into re-running code coverage analysis of this, and (before switching to it) running it on the new iOS implementation, and look for gaps like this class being completely untested. The downside of this new structure is that there are more layers where a bug like this can sneak in, and coverage analysis will help catch things like this in advance.

(Although in this case all the tests of the rest of the chain of calls also only tested true, so coverage wouldn't have found that anyway. But at least it would have flagged the need to have tests of this object.)

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-g stuartmorgan-g 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 Feb 15, 2022
@fluttergithubbot fluttergithubbot merged commit ce0870e into flutter:main Feb 15, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 16, 2022
debokarmakar pushed a commit to nurture-farm/plugins that referenced this pull request Feb 17, 2022
* google/master: (340 commits)
  [camera]remove "selfRef" for SavePhotoDelegate and ensure thread safety (flutter#4780)
  Roll Flutter from 919d205 to adafd66 (5 revisions) (flutter#4876)
  [google_sign_in] Update platform interface analysis options (flutter#4872)
  Roll Flutter from b623279 to 919d205 (2 revisions) (flutter#4871)
  Roll Flutter from 14a2b13 to b623279 (5 revisions) (flutter#4870)
  [image_picker] Update platform interface analysis options (flutter#4837)
  [ci] Re-enable stable webview Android tests (flutter#4867)
  Roll Flutter from 286c975 to 14a2b13 (1 revision) (flutter#4865)
  [local_auth] support localizedFallbackTitle in IOSAuthMessages (flutter#3806)
  [image_picker] Update app-facing and web analysis options (flutter#4838)
  Roll Flutter from 8386344 to 286c975 (4 revisions) (flutter#4864)
  Roll Flutter from e9f83cf to 8386344 (6 revisions) (flutter#4862)
  [camera] Switch web package to new analysis options (flutter#4834)
  [flutter_plugin_tool] Fix iOS/macOS naming (flutter#4861)
  [webview_flutter] Fix debuggingEnabled on Android (flutter#4859)
  Roll Flutter from ca2a751 to e9f83cf (10 revisions) (flutter#4857)
  fix license (flutter#4858)
  [video_player] add allowBackgroundPlayback option platform interface changes (flutter#4807)
  [video_player] Updated Pigeon version (flutter#4726)
  Roll Flutter from 381cb28 to ca2a751 (5 revisions) (flutter#4850)
  ...

# Conflicts:
#	packages/camera/camera/CHANGELOG.md
#	packages/camera/camera/android/build.gradle
#	packages/camera/camera/android/src/main/java/io/flutter/plugins/camera/Camera.java
#	packages/camera/camera/android/src/main/java/io/flutter/plugins/camera/MethodCallHandlerImpl.java
#	packages/camera/camera/ios/Classes/CameraPlugin.m
#	packages/camera/camera/ios/camera.podspec
#	packages/camera/camera/lib/camera.dart
#	packages/camera/camera/lib/src/camera_controller.dart
#	packages/camera/camera/pubspec.yaml
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 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
Development

Successfully merging this pull request may close these issues.

[webview_flutter] Webview debugging is always enabled on Android
3 participants