Skip to content

[google_maps_flutter] Refactor the JavaScript interaction and verify integration tests - #1066

Open
seungsoo47 wants to merge 6 commits into
flutter-tizen:masterfrom
seungsoo47:google_maps_flutter-add-integration-tests
Open

[google_maps_flutter] Refactor the JavaScript interaction and verify integration tests#1066
seungsoo47 wants to merge 6 commits into
flutter-tizen:masterfrom
seungsoo47:google_maps_flutter-add-integration-tests

Conversation

@seungsoo47

@seungsoo47 seungsoo47 commented Jul 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor
  • Refactor the JavaScript interaction layer by introducing GoogleMapsJsBridge, replacing direct WebViewController calls.
  • Update webview_flutter to ^4.13.1 and webview_flutter_lwe to ^0.5.3.
  • Verify integration tests pass against upstream google_maps_flutter v2.17.0.

@seungsoo47
seungsoo47 force-pushed the google_maps_flutter-add-integration-tests branch from 370807c to 39802fe Compare July 15, 2026 12:09
gemini-code-assist[bot]
gemini-code-assist Bot reviewed Jul 15, 2026
View reviewed changes

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the JavaScript interaction layer of the google_maps_flutter_tizen plugin by introducing a GoogleMapsJsBridge to replace direct WebViewController calls. The review feedback highlights a critical serialization issue in the bridge where arguments are directly interpolated, potentially causing runtime crashes and security vulnerabilities. To resolve this, the reviewer suggests introducing a JsExpression class to handle raw JS code and properly encoding other arguments. Additionally, the feedback recommends marking MapsJsEvent as sealed for exhaustiveness, utilizing the new bridge.addListener abstraction across controllers instead of executing raw JavaScript, and wrapping JS references in JsExpression to prevent them from being treated as string literals.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread packages/google_maps_flutter/lib/src/util.dart
Comment thread packages/google_maps_flutter/lib/src/util.dart
Comment thread packages/google_maps_flutter/lib/src/util.dart
Comment thread packages/google_maps_flutter/lib/src/google_maps_js_bridge.dart
Comment thread packages/google_maps_flutter/lib/src/util.dart
Comment thread packages/google_maps_flutter/lib/src/google_maps_js_bridge.dart
Comment thread packages/google_maps_flutter/lib/src/circle.dart
Comment thread packages/google_maps_flutter/lib/src/polygon.dart
Comment thread packages/google_maps_flutter/lib/src/google_maps_js_bridge.dart Outdated
Comment thread packages/google_maps_flutter/lib/src/ground_overlay.dart
@seungsoo47 seungsoo47 changed the title Google maps flutter add integration tests [google_maps_flutter] Refactor the JavaScript interaction and verify integration tests Jul 16, 2026
@seungsoo47 seungsoo47 mentioned this pull request Jul 21, 2026
Merged
JSUYA
JSUYA reviewed Jul 24, 2026
View reviewed changes

@JSUYA JSUYA left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please review gemini's review and modify the code.

@seungsoo47

seungsoo47 commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

Please review gemini's review and modify the code.

I will update the review and the code after merging #1071.

seungsoo47 added a commit to seungsoo47/plugins that referenced this pull request Aug 4, 2026
@seungsoo47
[google_maps_flutter] Fix unsafe JS argument serialization and use ad…
068219e 
…dListener consistently

Address Gemini Code Assist review on PR flutter-tizen#1066.

- Fix a critical bug where callMethod/setProperty interpolated
  arguments via List.toString(), so a plain String like a marker
  title was spliced into the JS snippet unquoted (e.g. ['My Title']
  instead of ["My Title"]), breaking at runtime for any string
  containing whitespace or special characters and risking JS
  injection. Add JsExpression to mark raw JS code that must not be
  quoted, and JSON-encode plain String arguments so they're safely
  treated as string literals.
- Wrap the few call sites that relied on the old unquoted behavior
  (setMap's 'map' JS-global reference, setPosition's LatLng
  constructor, setPixelOffset's GSize object literal) in JsExpression
  so they keep working under the new serialization.
- Mark MapsJsEvent sealed so the exhaustive switch in
  GoogleMapsController._onJsEvent is statically checked when new
  event types are added.
- Use the bridge.addListener abstraction in polyline/circle/polygon/
  ground_overlay instead of hand-built runJavaScript strings, for
  consistency with the rest of the bridge.
@seungsoo47
seungsoo47 force-pushed the google_maps_flutter-add-integration-tests branch from 39802fe to 174d20a Compare August 4, 2026 10:52
@seungsoo47
[google_maps_flutter] Refactor JS bridge and verify integration tests…
9037849 
… based on upstream v2.17.0
@seungsoo47
[google_maps_flutter] Bump version to 0.1.15
08a1f78
@seungsoo47
[google_maps_flutter] Fix unsafe JS argument serialization and use ad…
e44f1da 
…dListener consistently

Address Gemini Code Assist review on PR flutter-tizen#1066.

- Fix a critical bug where callMethod/setProperty interpolated
  arguments via List.toString(), so a plain String like a marker
  title was spliced into the JS snippet unquoted (e.g. ['My Title']
  instead of ["My Title"]), breaking at runtime for any string
  containing whitespace or special characters and risking JS
  injection. Add JsExpression to mark raw JS code that must not be
  quoted, and JSON-encode plain String arguments so they're safely
  treated as string literals.
- Wrap the few call sites that relied on the old unquoted behavior
  (setMap's 'map' JS-global reference, setPosition's LatLng
  constructor, setPixelOffset's GSize object literal) in JsExpression
  so they keep working under the new serialization.
- Mark MapsJsEvent sealed so the exhaustive switch in
  GoogleMapsController._onJsEvent is statically checked when new
  event types are added.
- Use the bridge.addListener abstraction in polyline/circle/polygon/
  ground_overlay instead of hand-built runJavaScript strings, for
  consistency with the rest of the bridge.
@seungsoo47
[google_maps_flutter] Update webview_flutter_lwe to 0.5.2
6ac2147
@seungsoo47
[google_maps_flutter] JSON-encode InfoWindow content and seal Message…
de583b5 
…JsEvent

Follow-up to 8b9c3079: GInfoWindowOptions.toString() still spliced the
info window content string directly into the JS snippet unquoted, the
same unsafe-serialization issue fixed elsewhere in that commit. Also
mark MessageJsEvent sealed, matching MapsJsEvent, so exhaustive
switches over its subclasses stay statically checked.
@seungsoo47
seungsoo47 force-pushed the google_maps_flutter-add-integration-tests branch from 174d20a to c04ae3a Compare August 6, 2026 00:49
@seungsoo47
seungsoo47 force-pushed the google_maps_flutter-add-integration-tests branch from c04ae3a to e8d9e43 Compare August 6, 2026 00:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JSUYA JSUYA JSUYA left review comments
+1 more reviewer
@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments
Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

p: google_maps_flutter

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@seungsoo47 @JSUYA