This was on a tip of tree developer build (58.0.2994.0)

Ryan, you made a change recently related to DocumentMarkerController::m_markers. Do you know someone familiar with DocumentMarkerController who could look at the cause of DCHECK in DocumentMarkerController::renderedRectsForMarkers?

This appears to be a bug in DocumentMarkerController, m_possiblyExistingMarkerTypes should be getting reset whenever m_markers becomes empty. It's unclear to me whether this is just the code missing out on a performance optimization somewhere (resetting the list possibly existing marker types) or if we have a bug causing data corruption somewhere (similar to the bug in addMarker() I fixed: https://codereview.chromium.org/2594123003 )

Doesn't seem hi-pri though

It seems DocumentMarkerController::removeMarkers(DocumentMarker::MarkerTypes markerTypes)|, called by |SpellChecerk|, doesn't reset m_possiblyExistingMarkerTypes.

|DOcumentMarkerController| is very old code and it is over abstraction, we should simplify its implementation.

Doesn't it get reset right here?
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp?sq=package:chromium&rcl=1485491011&l=648

I just ran into this DCHECK on Android while testing my Voice IME work...interesting

I just saw another location of this DCHECK get hit in RemoveMarkers():
https://chromium.googlesource.com/chromium/src/+/3639139a6216af6f66840f8348622d45ba06d5b9/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp#165

We should track this down as part of the DocumentMarkerController refactor efforts

It's unclear to me how this is happening. The DCHECK verifies that markers_ is not empty if possibly_existing_marker_types_ != 0. The only way this can fail is if either we have some method on DocumentMarkerController that violates this invariant by setting possibly_existing_marker_types to a non-zero value while markers_ is empty(), or making markers_ empty without resetting possibly_existing_marker_types_ to 0, and I can't find any way this should be able to happen unless we're experiencing memory corruption somehow.

I talked to xiaochengh and he pointed out that the map in question only stores weak references to its keys:
https://chromium.googlesource.com/chromium/src/+/d38becdb2a4dda654d795cb841111778673c0b1d/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.h#119

We think what's happening is that the map is becoming empty due to garbage collection of the Nodes in question. Garbage collection doesn't update possibly_has_marker_types_. This doesn't actually break anything, but it means the condition the DCHECK checks fails in rare cases. We think the only realistic solution is to remove the DCHECKs.

This isn't regression. From beginning of migrating to Oilpan[1], this issue has existed.

[1] http://crrev.com/288173004: Oilpan: Move DocumentMarkerController to the Oilpan heap and use builtin

FYI

DocumentMarkerController in WebKit[1] uses 
HashMap<RefPtr<Node>, std::unique_ptr<MarkerList>> with node removal callbacks[2] on 2015.


Blink uses HashMap<const Node*, OwnPtr<MarkerLists>[3] before Oilpan GC with remove callback.
And it remove callback was moved to Node::WillBeDeletedFromDocument()[4].

When Node::WillBeDeletedFromDocument() removed by Oilpan, Blink DMC doesn't remove entry
for removed node.

Interesting history. (^_^)

[1] https://trac.webkit.org/browser/webkit/trunk/Source/WebCore/dom/DocumentMarkerController.h
[2] http://wkb.ug/140999 Removing text node does not remove its associated markers
[3] http://crrev.com/82633002: Use const Node* in HashMap of DocumentMarkerController
[4] http://crrev.com/98273002: Remove node key from document marker set in Node::willBeDeletedFromDocument()

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/37c906bd514195222ab091fa98a461cc40b25577

commit 37c906bd514195222ab091fa98a461cc40b25577
Author: rlanday <rlanday@chromium.org>
Date: Sat May 20 01:17:16 2017

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}

[modify] https://crrev.com/37c906bd514195222ab091fa98a461cc40b25577/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp