[google_maps_flutter] Avoid calling null callbacks

[matthewlloyd]

Description

Avoid calling null callbacks.

There is no requirement that the callback parameters need to be non-null,
so without these checks, the plugin can generate exceptions, e.g. when
overlays that have no onTap callback are clicked.

Also guards against overlay or marker IDs not being present in the
dictionaries, which could happen by race condition (e.g. a click event
could be slightly delayed and get handled after the associated object has
already been removed).

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

• I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
• My PR includes unit or integration tests for all changed/updated/fixed behaviors (See [Contributor Guide]).
• All existing and new tests are passing.
• I updated/added relevant documentation (doc comments with ///).
• The analyzer (flutter analyze) does not report any problems on my PR.
• I read and followed the [Flutter Style Guide].
• The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
• I updated pubspec.yaml with an appropriate new version according to the [pub versioning philosophy].
• I updated CHANGELOG.md to add a description of the change.
• I signed the [CLA].
• I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

• Yes, this is a breaking change (please indicate a breaking change in CHANGELOG.md and increment major revision).
• No, this is not a breaking change.

[iskakaushik]

Curious, when would this get called on a markerId that has been removed?

Also, is there a way to write an integration test for this over at: https://github.com/flutter/plugins/blob/master/packages/google_maps_flutter/example/test_driver/google_maps.dart ?

[matthewlloyd]

Suppose a marker (or polyline, soon polygon, circle, etc.) gets tapped: the tap event is received on the iOS side of the bridge and forwarded and put on Dart's event queue for processing by some thread (I don't know enough about iOS / Flutter architecture to specify which one). Then suppose that at the same time, before Dart handles that message, it either is already processing, or now processes, some other previously queued event or state change that causes the marker to be removed from _markers. After the marker is removed, the tap event doesn't also get removed from Dart's event queue, so onMarkerTap would be called right after the marker has been removed from _markers. It is unlikely, but possible, and perhaps the additional null check it's unnecessary if someone can prove that it cannot happen in today's (or tomorrow's) Flutter architecture. However, given the concurrent nature of Flutter's architecture, with event loops on both sides of a message-passing bridge, it may not be a risk worth taking, and the null check is cheap anyway.

I think it would be hard to reproduce a race condition like that reliably an integration test...

[iskakaushik]

@matthewlloyd thanks for the explanation. LGTM

[iskakaushik]

I just fixed the merge conflicts.