[ios] Address bad access exception in MGLAttributionInfo

[jmkiley]

Continues discussion from #13298 in order to add the fix to master.

• Pregenerate the NSMutableAttributionString
• Measure performance ([iOS] Move creation of attributed string to global queue #13298 (comment))

This PR addresses a bad access exception with MGLMapSnapshotter on iOS 8. Per Apple docs:

The HTML importer should not be called from a background thread (that is, the options dictionary includes NSDocumentTypeDocumentAttribute with a value of NSHTMLTextDocumentType). It will try to synchronize with the main thread, fail, and time out. Calling it from the main thread works (but can still time out if the HTML contains references to external resources, which should be avoided at all costs). The HTML import mechanism is meant for implementing something like markdown (that is, text styles, colors, and so on), not for general HTML import.

cc @julianrex @ChrisLoer

[1ec5]

Note that this method is called as part of -[MGLTileSource attributionInfos], which is a public method that can be called at any time. Even internal uses of this method tend to occur in a for loop, since there are multiple sources to inspect at the same time.

If performance does turn out to be a problem, an alternative to blocking on the main thread here would be to do so further up the call stack, wherever it’s expected that the caller could be on a thread other than the main thread:

mapbox-gl-native/platform/darwin/src/MGLMapSnapshotter.mm

Line 213 in bb55bae

NSArray<MGLAttributionInfo *>* attributionInfo = [MGLMapSnapshotter generateAttributionInfos:attributions];

This would require documenting other callers, such as -[MGLTileSource attributionInfos], as needing to run on the main thread. There is precedent for such a requirement.

[jmkiley]

Updated to check whether it's being called on the main queue. Would it be better to call the NSMutableAttributedString initializer earlier than to fiddle with queues? @julianrex and I found that the exception did not occur when we created an NSMutableAttributedString in the initializer for a map view.

[julianrex]

@julianrex and I found that the exception did not occur when we created an NSMutableAttributedString in the initializer for a map view.

We can't rely on that behavior as it's not documented.

[ChrisLoer]

Sorry if I'm just parading my iOS ignorance here, but isn't MGLMapView essentially required to be created and used on the main thread? And if that's the case then creating the attributed string in the initializer should be good, right?

If we can find a way to avoid "fiddling with queues" we should. It's not the queues themselves that are the problem -- but hidden/unexpected cross-thread blocking calls are trouble.

[julianrex]

@ChrisLoer @jmkiley was referring to the fact that we can avoid the crash if we initialize any old NSMutableAttributedString on the main thread (it ends up calling WebKitInitialize using a dispatch_once - I suspect it's that initializer that needs to be on the main thread) - not the specific one.

[jmkiley]

This PR currently checks whether the NSMutableAttributedString is being initialized on the main thread.

[jmkiley]

Addressed a memory leak that popped up during profiling. Thank you @fabian-guerra for walking through the Instruments trace with me.

[julianrex]

Does initWithHTML... have the same concern?

[jmkiley]

Nothing in the documentation: https://developer.apple.com/documentation/foundation/nsattributedstring/1535412-initwithhtml

[julianrex]

Let's keep an eye on the performance changes; we may need to either move the generation of these strings earlier, consider caching - or both.

Please add a ticket for a performance test for snapshotters. 🙇