Bug 162029 – ASSERTION FAILED: m_items.isEmpty() in CustomElementReactionQueue destructor

• Component: New Bugs
• Status: RESOLVED FIXED

[Ryan Haddad]

Encountered on iOS 10 WK2 with LayoutTest fast/custom-elements/attribute-changed-callback.html

https://build.webkit.org/results/Apple%20iOS%2010%20Simulator%20Debug%20WK2%20(Tests)/r205989%20(50)/results.html

ASSERTION FAILED: m_items.isEmpty()
/Volumes/Data/slave/ios-simulator-10-debug/build/Source/WebCore/dom/CustomElementReactionQueue.cpp(114) : WebCore::CustomElementReactionQueue::~CustomElementReactionQueue()
1   0x10d74896d WTFCrash
2   0x10fb2eef9 WebCore::CustomElementReactionQueue::~CustomElementReactionQueue()
3   0x10fb2ef85 WebCore::CustomElementReactionQueue::~CustomElementReactionQueue()
4   0x10fb2fe05 WebCore::CustomElementReactionStack::processQueue()
5   0x1105a4985 WebCore::CustomElementReactionStack::~CustomElementReactionStack()
6   0x1105a3c55 WebCore::CustomElementReactionStack::~CustomElementReactionStack()
7   0x11071cb6e WebCore::jsElementPrototypeFunctionSetAttribute(JSC::ExecState*)
8   0x4bb236de028
9   0x10d44dbc5 llint_entry
10  0x10d44e00d llint_entry
11  0x10d44db4b llint_entry
12  0x10d44dbc5 llint_entry
13  0x10d446b9e vmEntryToJavaScript
14  0x10d254339 JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*)
15  0x10d1fc794 JSC::Interpreter::execute(JSC::ProgramExecutable*, JSC::ExecState*, JSC::JSObject*)
16  0x10cc9367d JSC::evaluate(JSC::ExecState*, JSC::SourceCode const&, JSC::JSValue, WTF::NakedPtr<JSC::Exception>&)
17  0x10cc9384e JSC::profiledEvaluate(JSC::ExecState*, JSC::ProfilingReason, JSC::SourceCode const&, JSC::JSValue, WTF::NakedPtr<JSC::Exception>&)
18  0x11152a32b WebCore::JSMainThreadExecState::profiledEvaluate(JSC::ExecState*, JSC::ProfilingReason, JSC::SourceCode const&, JSC::JSValue, WTF::NakedPtr<JSC::Exception>&)
19  0x11152a128 WebCore::ScriptController::evaluateInWorld(WebCore::ScriptSourceCode const&, WebCore::DOMWrapperWorld&, WebCore::ExceptionDetails*)
20  0x11152a40d WebCore::ScriptController::evaluate(WebCore::ScriptSourceCode const&, WebCore::ExceptionDetails*)
21  0x111538c4b WebCore::ScriptElement::executeScript(WebCore::ScriptSourceCode const&)
22  0x111537ae9 WebCore::ScriptElement::prepareScript(WTF::TextPosition const&, WebCore::ScriptElement::LegacyTypeSupport)
23  0x1101d78d1 WebCore::HTMLScriptRunner::runScript(WebCore::Element*, WTF::TextPosition const&)
24  0x1101d76ea WebCore::HTMLScriptRunner::execute(WTF::PassRefPtr<WebCore::Element>, WTF::TextPosition const&)
25  0x110102988 WebCore::HTMLDocumentParser::runScriptsForPausedTreeBuilder()
26  0x110102e03 WebCore::HTMLDocumentParser::pumpTokenizerLoop(WebCore::HTMLDocumentParser::SynchronousMode, bool, WebCore::PumpSession&)
27  0x110101ac8 WebCore::HTMLDocumentParser::pumpTokenizer(WebCore::HTMLDocumentParser::SynchronousMode)
28  0x11010161b WebCore::HTMLDocumentParser::pumpTokenizerIfPossible(WebCore::HTMLDocumentParser::SynchronousMode)
29  0x1101040d6 WebCore::HTMLDocumentParser::append(WTF::RefPtr<WTF::StringImpl>&&)
30  0x10fba3972 WebCore::DecodedDataDocumentParser::appendBytes(WebCore::DocumentWriter&, char const*, unsigned long)
31  0x10fcd92e9 WebCore::DocumentWriter::addData(char const*, unsigned long)
LEAK: 1 WebProcessPool

[Radar WebKit Bug Importer]

<rdar://problem/28332657>

[Ryosuke Niwa]

I haven't been able to reproduce this but this should be fixed in https://trac.webkit.org/changeset/207710 because we clear the queue for sure now.

[Ryosuke Niwa]

We’re still seeing this.

[Ryosuke Niwa]

Created attachment 296373 [details]
Fixes the bug

[Ryosuke Niwa]

Created attachment 296420 [details]
Fixed a regression

[Ryosuke Niwa]

<rdar://problem/28945851>

[Chris Dumez]

Comment on attachment 296420 [details]
Fixed a regression

View in context: https://bugs.webkit.org/attachment.cgi?id=296420&action=review

> Source/WebCore/dom/CustomElementReactionQueue.cpp:153
> +    if (element.document().refCount() <= 0)

I really don't like that we're checking the refCount. Could we instead check if the document's frame is null? A document's frame should always be null after calling prepareForDestruction() on it.

> LayoutTests/ChangeLog:18
> [phone]  Ryosuke Niwa  <[e-mail]>

Duplicate ChangeLog.

[Ryosuke Niwa]

Created attachment 296441 [details]
Removed a duplicate change log entry

[Ryosuke Niwa]

(In reply to comment #7)
> Comment on attachment 296420 [details]
> Fixed a regression
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=296420&action=review
> 
> > Source/WebCore/dom/CustomElementReactionQueue.cpp:153
> > +    if (element.document().refCount() <= 0)
> 
> I really don't like that we're checking the refCount. Could we instead check
> if the document's frame is null? A document's frame should always be null
> after calling prepareForDestruction() on it.

I did that (checking active DOM objects are stopped instead) in my previous patch, and that broke the test case I’m adding (disconnected-callback-in-detached-iframe.html). Because custom elements are coming from a different realm (iframe), we still need to enqueue disconnectedCallback in such a case.

An alternative approach is to add a flag on Document which indicates that we’re in the middle of Document tear down but I’m not sure that’s better than checking refcount.

[Ryosuke Niwa]

Committed r209582: <http://trac.webkit.org/changeset/209582>