iOS: overlay controllers record markers as drawn before checking the map view is there #157

Problem

Both iOS overlay controllers commit "these markers are on the map" before checking that there
is a map to put them on — the same write-before-check shape as #127 on Android.

MapOverlayController.setMarkers (ios/MapOverlayController.swift:61):

func setMarkers(_ descriptors: [MarkerDescriptor]?) {
  guard markerPipeline.setMarkers(descriptors) else { return }   // records the fingerprint
  reapplyMarkers()                                               // `guard let mapView else { return }`
}

MarkerRenderPipeline.setMarkers (ios/MarkerClusterEngine.swift:402–413) writes
markersFingerprint = fingerprint at :409 and returns true; reapplyMarkers
(MapOverlayController.swift:68) then gives up at guard let mapView. With no map view the
pipeline believes that descriptor set is applied while nothing was drawn, and because the next
delivery of the same array early-returns on the matching fingerprint, there is no recovery —
exactly the mechanism described in #127.

GoogleMapOverlayController has the same pair: setMarkers at ios/GoogleMapOverlayController.swift:73,
reapplyMarkers with the same guard at :195.

It is not limited to markers. setClusteringEnabled has the identical shape on both
controllers (MapOverlayController.swift:40, GoogleMapOverlayController.swift:66): the
pipeline flips clusteringEnabled and returns true, then the redraw is dropped. Every
guard pipeline.xxx() else { return } / reapply() pair carries it.

Why this is unreachable today — and why the usual explanation does not close it

The reason given so far has been that the map views are created synchronously, so unlike
Android there is no "map not ready" window. That is true of construction and it is not
enough, because both controllers hold the view weakly:

• private weak var mapView: MKMapView? (MapOverlayController.swift:21)
• private weak var mapView: GMSMapView? (GoogleMapOverlayController.swift:31)

A non-optional init(mapView:) says the view existed at construction. It says nothing about
whether it still exists at the next setMarkers.

What actually rules the bug out is ownership, and it holds:

• each adapter owns both, as strong stored properties, and is the only place either is built —
  fileprivate lazy var overlayController = MapOverlayController(mapView: view) next to
  lazy var view: MKMapView (AppleMapProviderAdapter.swift:11, :17), and the same pair in
  GoogleMapProviderAdapter.swift:22–28, :34;
• the controller is reachable only through its adapter, so it cannot outlive it;
• and the adapter is never reused across views — HybridMapView.prepareForRecycle discards it
  whole (HybridMapView.swift:289–298 — adapter?.prepareForRecycle() at :296, then
  adapter = nil at :298), and installAdapter replaces it the same way (:363, :366–370).

So today the weak reference can never be nil while the controller is alive. That guarantee is
incidental, it lives three files away from the code that depends on it, and nothing states it.
A future change that keeps a controller across a view recycle — reusing it to avoid rebuilding
the annotation caches, say — reintroduces #127 on iOS as a silently empty map.

Impact

None today. This is a latent invariant, not a user-visible defect. Filed so the invariant is
written down instead of rediscovered.

Fix

Preferred — make it unrepresentable. The view is never reassigned (self.mapView = mapView
in init is the only write in either controller), and the adapter already keeps it alive, so
the weak optional buys nothing:

private let mapView: MKMapView

No retain cycle: the view holds its delegate weakly (MKMapView.delegate), and
HybridMapViewDelegate.parent is weak too (HybridMapViewDelegate.swift:6), so nothing
points back at the adapter. This also deletes the 13 guard let mapView else { return } sites
across the two controllers and removes the whole write-before-check question, rather than
documenting around it.

Minimum — say what keeps it alive. If the weak reference is deliberate, a comment at each
declaration naming the guarantee:

/// Owned by the adapter, which also owns this controller and is thrown away whole in
/// `prepareForRecycle`, so this is never nil while the controller lives. Reusing a
/// controller across views would break `MarkerRenderPipeline`'s "applied" bookkeeping
/// the way #127 broke Android's.
private weak var mapView: MKMapView?

Not recommended for now — mirror Android's state machine. #127's fix adds an explicit
isMapAttached / isDrawn pair to MarkerRenderState so the fingerprint means drawn, not
received. MarkerRenderPipeline has no equivalent. Porting it would need an attach/detach
moment on iOS, and there is none, so it would be machinery with no caller. Worth revisiting
only if a controller ever does outlive a view.

Notes

• Related: Android: markers never appear when clusteringEnabled is applied before the Google map is ready #127 (the Android instance of this shape, currently open).
• MarkerRenderPipeline is the shared engine behind both iOS controllers, so whichever option
  is taken should be applied to both MapOverlayController and GoogleMapOverlayController in
  one go.
• Found while reviewing the fix for Android: markers never appear when clusteringEnabled is applied before the Google map is ready #127.