Bug 164814 – Add more assertions to ElementQueue diagnose a bug

• Component: DOM
• Status: RESOLVED FIXED

[Ryosuke Niwa]

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...

[Ryosuke Niwa]

Created attachment 294918 [details]
Tightens assertions

[Yusuke Suzuki]

Comment on attachment 294918 [details]
Tightens assertions

r=me

[Ryosuke Niwa]

Created attachment 294923 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 294923 [details]
Patch for landing

Clearing flags on attachment: 294923

Committed r208785: <http://trac.webkit.org/changeset/208785>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Ryosuke Niwa]

Reopening to attach new patch.

[Ryosuke Niwa]

Created attachment 296267 [details]
Set m_invoking in release builds

[WebKit Commit Bot]

Comment on attachment 296267 [details]
Set m_invoking in release builds

Clearing flags on attachment: 296267

Committed r209426: <http://trac.webkit.org/changeset/209426>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.