Bug 162029

Summary: ASSERTION FAILED: m_items.isEmpty() in CustomElementReactionQueue destructor
Product: WebKit Reporter: Ryan Haddad <ryanhaddad>
Component: New BugsAssignee: Ryosuke Niwa <rniwa>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, cdumez, commit-queue, dbates, esprehn+autocc, kangil.han, rniwa, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 164814    
Bug Blocks: 154907    
Attachments:
Description Flags
Fixes the bug
none
Fixed a regression
none
Removed a duplicate change log entry cdumez: review+

Description Ryan Haddad 2016-09-15 12:51:32 PDT
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
Comment 1 Radar WebKit Bug Importer 2016-09-15 21:28:26 PDT
<rdar://problem/28332657>
Comment 2 Ryosuke Niwa 2016-10-23 18:59:36 PDT
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.
Comment 3 Ryosuke Niwa 2016-12-06 20:56:17 PST
We’re still seeing this.
Comment 4 Ryosuke Niwa 2016-12-06 21:46:11 PST
Created attachment 296373 [details]
Fixes the bug
Comment 5 Ryosuke Niwa 2016-12-07 14:10:36 PST
Created attachment 296420 [details]
Fixed a regression
Comment 6 Ryosuke Niwa 2016-12-07 14:40:37 PST
<rdar://problem/28945851>
Comment 7 Chris Dumez 2016-12-07 16:14:41 PST
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
> +2016-12-07  Ryosuke Niwa  <rniwa@webkit.org>

Duplicate ChangeLog.
Comment 8 Ryosuke Niwa 2016-12-07 16:56:57 PST
Created attachment 296441 [details]
Removed a duplicate change log entry
Comment 9 Ryosuke Niwa 2016-12-07 17:23:22 PST
(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.
Comment 10 Ryosuke Niwa 2016-12-08 16:54:01 PST
Committed r209582: <http://trac.webkit.org/changeset/209582>