Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[google_maps_flutter] Fix Circle update regression #7056

Merged
merged 4 commits into from
Jul 3, 2024

Conversation

stuartmorgan
Copy link
Contributor

@stuartmorgan stuartmorgan commented Jul 3, 2024

Fixes a regression introduced in the last PR that did shallow Pigeon conversions of the maps calls, where the new Pigeon wrapper object was being passed directly to one of the not-yet-converted methods that was expecting the JSON data (i.e., circle.getJson() rather than circle). Also adds a native unit test exercising that codepath, which reproduced the cast error.

As a secondary step to improve safety is this hybrid state, for all of the map object controllers, I converted most of the methods taking Objects to taking Map<String, ?> instead, which is in practice what the JSON serialization of all of those objects is, and further pushed that through the Pigeon layer so that instead of casting throughout the Java code, the cast happens once on the Dart side. While the Dart cast is technically unsafe:

  • It's the same level of unsafe as the previous situation, just centralized in a small amount of Dart code instead of spread through the Java code.
  • In practice, the JSON serialization in the platform interface package, which is what this cast and all the previous Java code was making assumptions about, can never change, because it is a de-facto API surface. (The fact that we are relying on the JSON serialization code from what is now a different package was a mistake during federation, which has existing TODOs to fix; in practice the fix will almost certainly be to finish the Pigeon migration rather than to copy the JSON serialization.)

Part of flutter/flutter#117907

Pre-launch Checklist

@stuartmorgan stuartmorgan added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 3, 2024
@auto-submit auto-submit bot merged commit 1768c17 into flutter:main Jul 3, 2024
74 checks passed
@stuartmorgan stuartmorgan deleted the maps-pigeon-android-bugfix branch July 3, 2024 19:52
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 4, 2024
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Jul 4, 2024
flutter/packages@d2705fb...754de19

2024-07-03 49699333 dependabot[bot]@users.noreply.github.com [interactive_media_ads]: Bump com.google.ads.interactivemedia.v3:interactivemedia from 3.33.0 to 3.34.0 in /packages/interactive_media_ads/android (flutter/packages#7037)
2024-07-03 [email protected] [google_maps_flutter] Fix Circle update regression (flutter/packages#7056)
2024-07-03 [email protected] Roll Flutter from 99bb2ff to af913a7 (30 revisions) (flutter/packages#7047)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC [email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/ doc/main/autoroll/README.md
TahaTesser pushed a commit to TahaTesser/flutter that referenced this pull request Jul 8, 2024
…r#151315)

flutter/packages@d2705fb...754de19

2024-07-03 49699333 dependabot[bot]@users.noreply.github.com [interactive_media_ads]: Bump com.google.ads.interactivemedia.v3:interactivemedia from 3.33.0 to 3.34.0 in /packages/interactive_media_ads/android (flutter/packages#7037)
2024-07-03 [email protected] [google_maps_flutter] Fix Circle update regression (flutter/packages#7056)
2024-07-03 [email protected] Roll Flutter from 99bb2ff to af913a7 (30 revisions) (flutter/packages#7047)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC [email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/ doc/main/autoroll/README.md
victorsanni pushed a commit to victorsanni/flutter that referenced this pull request Jul 8, 2024
…r#151315)

flutter/packages@d2705fb...754de19

2024-07-03 49699333 dependabot[bot]@users.noreply.github.com [interactive_media_ads]: Bump com.google.ads.interactivemedia.v3:interactivemedia from 3.33.0 to 3.34.0 in /packages/interactive_media_ads/android (flutter/packages#7037)
2024-07-03 [email protected] [google_maps_flutter] Fix Circle update regression (flutter/packages#7056)
2024-07-03 [email protected] Roll Flutter from 99bb2ff to af913a7 (30 revisions) (flutter/packages#7047)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC [email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/ doc/main/autoroll/README.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autosubmit Merge PR when tree becomes green via auto submit App p: google_maps_flutter platform-android
Projects
None yet
2 participants