Harden std::map usage in MGLMapVIew #8637
Conversation
Replaced std::map::operator[] usage with std::map::at(). Backed up an assertion on iOS with a guard when -annotationTagsInRect: returns the tag of a nonexistent annotation, for consistency with macOS. Removed an unnecessary and risky subscript into _annotationContextsByAnnotationTag in -positioningRectForCalloutForAnnotationWithTag: on iOS.
1ec5
added
annotations
| @@ -3550,8 +3550,12 @@ - (MGLAnnotationTag)annotationTagAtPoint:(CGPoint)point persistingResults:(BOOL) | |||
| { | |||
| id <MGLAnnotation> annotation = [self annotationWithTag:annotationTag]; | |||
| NSAssert(annotation, @"Unknown annotation found nearby tap"); | |||
There was a problem hiding this comment.
Just noting that this assertion will fail in a debug build if you perform the reproduction steps in #8163 (comment). The issue is that mbgl notion of visible annotations will always lag a bit behind the platforms' notion of the same when annotations are removed.
There was a problem hiding this comment.
Should we consider that a bug, or is it an edge case we want to fail gracefully around from now on?
There was a problem hiding this comment.
I think it is a bug that is an edge case 😄
But, the fix would require a callback feature for at least some of the mbgl APIs right? The platforms should not actually update their state until after core has completed updating its state (i.e. removing annotations so that queries of visible annotations won't include them)
There was a problem hiding this comment.
I agree, and we should make sure the map remains usable even in debug builds. That could be tail work, though; it seems like #8513 would have the same considerations.
Replaced
std::map::operator[]usage withstd::map::at(). Backed up an assertion on iOS with a guard when-annotationTagsInRect:returns the tag of a nonexistent annotation, for consistency with macOS. Removed an unnecessary and risky subscript into_annotationContextsByAnnotationTagin-positioningRectForCalloutForAnnotationWithTag:on iOS./ref #8163 (comment)