[google_maps_flutter] Add support to request map renderer for Android #6619

jokerttu · 2022-10-25T10:57:44Z

This adds support to prefer map renderer for Android.

Added API method initializeWithRenderer for GoogleMapsFlutterAndroid instance to initialize Google Maps SDK with preferred renderer.
Renderer can be initialized only once per Application context and cannot be changed after it is initialized.

New map renderer documentation

Resolves flutter/flutter#114004

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

flutter-dashboard · 2022-10-25T10:57:47Z

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

jokerttu · 2022-10-25T12:48:10Z

As this change seems to affect the integration tests of the main plugin at google_maps_flutter/google_maps_flutter/example this is now converted as first phase of the multi-package PR process.

jokerttu · 2022-10-25T12:51:17Z

@stuartmorgan
Tests are not runned against this feature at the moment and default preferred renderer is used. The issue is that map renderer initialization can be done only once in application context. And currently tests are driven in single application context. There is not possible way to change the renderer between default, legacy and latest between test runs.

It is as important to run all google_maps_flutter tests against legacy renderer as it is against latest renderer.

One option could be to set preferred renderer for tests by setting MAPS_PREFERRED_RENDERER environment variable for test job at .cirrus.yml#L247
But duplicating the whole android test runs for latest and legacy renderers would not be good option.
Could there be separate task that drives only google_maps_flutter plugin tests with latest renderer separately?

stuartmorgan

For CI tests, I believe we should be able to make a second driver in google_maps_flutter_android/example/test_driver/, and have each driver set a different renderer.

packages/google_maps_flutter/google_maps_flutter_android/README.md

...flutter_android/android/src/main/java/io/flutter/plugins/googlemaps/GoogleMapController.java

jokerttu · 2022-10-31T21:23:44Z

For CI tests, I believe we should be able to make a second driver in google_maps_flutter_android/example/test_driver/, and have each driver set a different renderer.

Seems like drivers are not runned at all on for the Android integration tests; instead integration tests are runned with firebase-test-lab which skips test_driver folder totally and runs tests directly from integration_tests file without drivers.
Implemented approach was legacy way on doing this: flutter/flutter#85224

Moved tests under integration_test folder. Files in this folder are each build and tested separately so those should have separate application contexts as well.

stuartmorgan

@GaryQian Could you review the Java part of this?

packages/google_maps_flutter/google_maps_flutter/CHANGELOG.md

packages/google_maps_flutter/google_maps_flutter_ios/CHANGELOG.md

packages/google_maps_flutter/google_maps_flutter_platform_interface/CHANGELOG.md

packages/google_maps_flutter/google_maps_flutter_android/README.md

...ges/google_maps_flutter/google_maps_flutter_android/lib/src/google_map_renderer_android.dart

...ges/google_maps_flutter/google_maps_flutter_android/lib/src/google_maps_flutter_android.dart

…android

jokerttu · 2022-11-17T13:54:36Z

...gle_maps_flutter/google_maps_flutter_android/example/integration_test/google_maps_tests.dart

-                (LatLngBounds bounds) => bounds != zeroLatLngBounds) ??
+                (LatLngBounds bounds) =>
+                    bounds != zeroLatLngBounds &&
+                    bounds.northeast != bounds.southwest) ??


This change is done, as otherwise test fails with latest renderer, as bounds with zero area is given at start.

stuartmorgan

This looks good to me from a code standpoint; I'm going to push a change to the branch so that the full Android test suite will run in CI.

stuartmorgan · 2022-11-21T20:50:53Z

I forgot we can't push to org-repo-owned branches; created #6731 instead.

stuartmorgan

Sorry for the long delay here; I forgot that this was just waiting for the results of my test run PR (which passed).

…or Android (flutter/plugins#6619)

* 93a63b36e [google_maps_flutter] Add support to request map renderer for Android (flutter/plugins#6619) * ca3d56973 Roll Flutter from e2fb672 to a570fd2 (15 revisions) (flutter/plugins#6804) * 51434ec83 [google_maps_flutter] Roll cupertino_icons for compatibility with Dart 3 (flutter/plugins#6807)

…#116728) * 93a63b36e [google_maps_flutter] Add support to request map renderer for Android (flutter/plugins#6619) * ca3d56973 Roll Flutter from e2fb672 to a570fd2 (15 revisions) (flutter/plugins#6804) * 51434ec83 [google_maps_flutter] Roll cupertino_icons for compatibility with Dart 3 (flutter/plugins#6807)

…flutter#6619) * [google_maps_flutter] support to request a specific map renderer for android * [google_maps_flutter] Minor fixes to comments and error messages * [google_maps_flutter] fix linting issues

jokerttu requested a review from GaryQian as a code owner October 25, 2022 10:57

flutter-dashboard bot added the needs tests label Oct 25, 2022

github-actions bot added p: google_maps_flutter platform-android labels Oct 25, 2022

jokerttu mentioned this pull request Oct 25, 2022

[google_maps_flutter] Cloud-based map styling support #6553

Closed

11 tasks

jokerttu force-pushed the google_maps_flutter_preferred_renderer branch from 880e0c5 to 384376d Compare October 25, 2022 11:04

jokerttu requested a review from stuartmorgan as a code owner October 25, 2022 12:38

jokerttu changed the title ~~[google_maps_flutter_android] Add support to prefer renderer~~ [google_maps_flutter] Add support to prefer renderer Oct 25, 2022

stuartmorgan suggested changes Oct 25, 2022

View reviewed changes

packages/google_maps_flutter/google_maps_flutter_android/README.md Outdated Show resolved Hide resolved

...flutter_android/android/src/main/java/io/flutter/plugins/googlemaps/GoogleMapController.java Outdated Show resolved Hide resolved

jokerttu requested review from cyanglaz and ditman as code owners October 27, 2022 15:47

github-actions bot added platform-ios platform-web labels Oct 27, 2022

This comment was marked as outdated.

Sign in to view

jokerttu requested review from stuartmorgan and removed request for ditman, GaryQian and cyanglaz October 27, 2022 21:26

jokerttu changed the title ~~[google_maps_flutter] Add support to prefer renderer~~ [google_maps_flutter] Add support to request map renderer for Android Oct 28, 2022

stuartmorgan suggested changes Nov 10, 2022

View reviewed changes

[google_maps_flutter] support to request a specific map renderer for …

975f6cc

…android

jokerttu force-pushed the google_maps_flutter_preferred_renderer branch from 410db81 to 975f6cc Compare November 17, 2022 13:26

github-actions bot removed platform-ios platform-web labels Nov 17, 2022

[google_maps_flutter] Minor fixes to comments and error messages

44f33b1

jokerttu requested a review from stuartmorgan November 17, 2022 13:51

jokerttu commented Nov 17, 2022

View reviewed changes

stuartmorgan reviewed Nov 21, 2022

View reviewed changes

stuartmorgan mentioned this pull request Nov 21, 2022

[google_maps_flutter] CI test for #6619 #6731

Closed

jokerttu added 2 commits November 23, 2022 10:25

Merge branch 'main' into google_maps_flutter_preferred_renderer

65a5c17

[google_maps_flutter] fix linting issues

c637317

jokerttu requested a review from stuartmorgan November 24, 2022 06:45

stuartmorgan approved these changes Dec 7, 2022

View reviewed changes

stuartmorgan added the autosubmit Merge PR when tree becomes green via auto submit App label Dec 7, 2022

auto-submit bot merged commit 93a63b3 into flutter:main Dec 7, 2022

engine-flutter-autoroll mentioned this pull request Dec 8, 2022

Roll Plugins from 7b5d8323efe3 to 51434ec83dde (3 revisions) flutter/flutter#116728

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Dec 8, 2022

93a63b36e [google_maps_flutter] Add support to request map renderer f…

7b89dd2

…or Android (flutter/plugins#6619)

paulcappadona mentioned this pull request Dec 13, 2022

[google_maps_flutter] java.lang.SecurityException: GoogleCertificatesRslt: not allowed exception: pkg={package_name} flutter/flutter#109115

Closed

jokerttu deleted the google_maps_flutter_preferred_renderer branch December 14, 2022 13:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[google_maps_flutter] Add support to request map renderer for Android #6619

[google_maps_flutter] Add support to request map renderer for Android #6619

jokerttu commented Oct 25, 2022 •

edited

Loading

flutter-dashboard bot commented Oct 25, 2022

jokerttu commented Oct 25, 2022

jokerttu commented Oct 25, 2022 •

edited

Loading

stuartmorgan left a comment

This comment was marked as outdated.

jokerttu commented Oct 31, 2022 •

edited

Loading

stuartmorgan left a comment

jokerttu Nov 17, 2022 •

edited

Loading

stuartmorgan left a comment

stuartmorgan commented Nov 21, 2022

stuartmorgan left a comment

[google_maps_flutter] Add support to request map renderer for Android #6619

[google_maps_flutter] Add support to request map renderer for Android #6619

Conversation

jokerttu commented Oct 25, 2022 • edited Loading

Pre-launch Checklist

flutter-dashboard bot commented Oct 25, 2022

jokerttu commented Oct 25, 2022

jokerttu commented Oct 25, 2022 • edited Loading

stuartmorgan left a comment

Choose a reason for hiding this comment

This comment was marked as outdated.

jokerttu commented Oct 31, 2022 • edited Loading

stuartmorgan left a comment

Choose a reason for hiding this comment

jokerttu Nov 17, 2022 • edited Loading

Choose a reason for hiding this comment

stuartmorgan left a comment

Choose a reason for hiding this comment

stuartmorgan commented Nov 21, 2022

stuartmorgan left a comment

Choose a reason for hiding this comment

jokerttu commented Oct 25, 2022 •

edited

Loading

jokerttu commented Oct 25, 2022 •

edited

Loading

jokerttu commented Oct 31, 2022 •

edited

Loading

jokerttu Nov 17, 2022 •

edited

Loading