Commit 37c906bd authored by rlanday's avatar rlanday Committed by Commit bot

Fix bug causing DCHECKs to be hit in DocumentMarkerController

These DCHECKs check that we're resetting the possibly_has_marker_types_ flag
when markers_ becomes empty. Resetting this flag properly enables a performance
optimization. These DCHECKs are causing sporadic crashes in debug builds; we
believe this is because the markers_ map is holding weak references to its keys
(Node objects), so it's possible for the map to become empty through garbage
collection, which doesn't reset possibly_has_marker_types_.

This CL modifies DocumentMarkerController::PossiblyHasMarkers() to catch this
case. Alternatively, we could handle this case at garbage collection time, but
we believe this approach has better performance characteristics.

BUG=685755

Review-Url: https://codereview.chromium.org/2891843003
Cr-Commit-Position: refs/heads/master@{#473400}
parent 44f77685
...@@ -95,6 +95,20 @@ Member<DocumentMarkerList>& DocumentMarkerController::ListForType( ...@@ -95,6 +95,20 @@ Member<DocumentMarkerList>& DocumentMarkerController::ListForType(
inline bool DocumentMarkerController::PossiblyHasMarkers( inline bool DocumentMarkerController::PossiblyHasMarkers(
DocumentMarker::MarkerTypes types) { DocumentMarker::MarkerTypes types) {
if (markers_.IsEmpty()) {
// It's possible for markers_ to become empty through garbage collection if
// all its Nodes are GC'ed since we only hold weak references, in which case
// possibly_existing_marker_types_ isn't reset to 0 as it is in the other
// codepaths that remove from markers_. Therefore, we check for this case
// here.
// Alternatively, we could handle this case at the time the Node is GC'ed,
// but that operation is more performance-sensitive than anywhere
// PossiblyHasMarkers() is used.
possibly_existing_marker_types_ = 0;
return false;
}
return possibly_existing_marker_types_.Intersects(types); return possibly_existing_marker_types_.Intersects(types);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment