| Summary: | Add more assertions to ElementQueue diagnose a bug | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Ryosuke Niwa <rniwa> | ||||||||
| Component: | DOM | Assignee: | Ryosuke Niwa <rniwa> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | ap, cdumez, commit-queue, dbates, esprehn+autocc, kangil.han, koivisto | ||||||||
| Priority: | P2 | ||||||||||
| Version: | WebKit Nightly Build | ||||||||||
| Hardware: | Unspecified | ||||||||||
| OS: | Unspecified | ||||||||||
| Bug Depends on: | |||||||||||
| Bug Blocks: | 162029 | ||||||||||
| Attachments: |
|
||||||||||
Created attachment 294918 [details]
Tightens assertions
Comment on attachment 294918 [details]
Tightens assertions
r=me
Created attachment 294923 [details]
Patch for landing
Comment on attachment 294923 [details] Patch for landing Clearing flags on attachment: 294923 Committed r208785: <http://trac.webkit.org/changeset/208785> All reviewed patches have been landed. Closing bug. Reopening to attach new patch. Created attachment 296267 [details]
Set m_invoking in release builds
Comment on attachment 296267 [details] Set m_invoking in release builds Clearing flags on attachment: 296267 Committed r209426: <http://trac.webkit.org/changeset/209426> All reviewed patches have been landed. Closing bug. |
We seem to be sporadically hitting an assertion in CustomElementReactionStack::ElementQueue on our bots. I don’t quite understand how this is possible. We swap the vector at the beginning of the function before start invoking on elements. So the only way m_elements can be not empty is if an item was added to m_elements while we’re calling invokeAll: inline void CustomElementReactionStack::ElementQueue::invokeAll() { #if !ASSERT_DISABLED TemporaryChange<bool> invoking(m_invoking); #endif Vector<Ref<Element>> elements; elements.swap(m_elements); for (auto& element : elements) { auto* queue = element->reactionQueue(); ASSERT(queue); queue->invokeAll(element.get()); } ASSERT(m_elements.isEmpty()); } But I’ve added an assertion for m_invoking so that m_invoking is false when we’re calling add: inline void CustomElementReactionStack::ElementQueue::add(Element& element) { ASSERT(!m_invoking); // FIXME: Avoid inserting the same element multiple times. m_elements.append(element); } which isn't firing...