Bug 181091 - REGRESSION: [iOS] ASSERTION FAILED: !node.isConnected() in WebCore::notifyNodeInsertedIntoDocument
Summary: REGRESSION: [iOS] ASSERTION FAILED: !node.isConnected() in WebCore::notifyNod...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jer Noble
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-12-21 10:41 PST by Ryan Haddad
Modified: 2018-01-08 08:46 PST (History)
6 users (show)

See Also:


Attachments
Patch (1.32 KB, patch)
2017-12-21 13:43 PST, Jer Noble
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews100 for mac-elcapitan (2.31 MB, application/zip)
2017-12-21 14:37 PST, EWS Watchlist
no flags Details
Archive of layout-test-results from ews107 for mac-elcapitan-wk2 (2.99 MB, application/zip)
2017-12-21 14:54 PST, EWS Watchlist
no flags Details
Archive of layout-test-results from ews116 for mac-elcapitan (2.91 MB, application/zip)
2017-12-21 15:14 PST, EWS Watchlist
no flags Details
Patch (2.20 KB, patch)
2018-01-02 00:33 PST, Jer Noble
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryan Haddad 2017-12-21 10:41:33 PST
The following assertion failure is seen on iOS debug bots with LayoutTests fast/events/event-handler-detached-document-dispatchEvent.html and fast/events/event-handler-detached-document.html:

ASSERTION FAILED: !node.isConnected()
/Volumes/Data/slave/ios-simulator-11-debug/build/Source/WebCore/dom/ContainerNodeAlgorithms.cpp(47) : void WebCore::notifyNodeInsertedIntoDocument(WebCore::ContainerNode &, WebCore::Node &, WebCore::TreeScopeChange, NodeVector &)
1   0x115eefc9d WTFCrash
2   0x11a0f60c3 WebCore::notifyNodeInsertedIntoDocument(WebCore::ContainerNode&, WebCore::Node&, WebCore::TreeScopeChange, WTF::Vector<WTF::Ref<WebCore::Node, WTF::DumbPtrTraits<WebCore::Node> >, 11ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>&)
3   0x11a0f62c9 WebCore::notifyNodeInsertedIntoDocument(WebCore::ContainerNode&, WebCore::Node&, WebCore::TreeScopeChange, WTF::Vector<WTF::Ref<WebCore::Node, WTF::DumbPtrTraits<WebCore::Node> >, 11ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>&)
4   0x11a0f5f82 WebCore::notifyChildNodeInserted(WebCore::ContainerNode&, WebCore::Node&)
5   0x11a0f3bc5 void WebCore::executeNodeInsertionWithScriptAssertion<WebCore::ContainerNode::parserAppendChild(WebCore::Node&)::$_5>(WebCore::ContainerNode&, WebCore::Node&, WebCore::ContainerNode::ChildChangeSource, WebCore::ReplacedAllChildren, WebCore::ContainerNode::parserAppendChild(WebCore::Node&)::$_5)
6   0x11a0effed WebCore::ContainerNode::parserAppendChild(WebCore::Node&)
7   0x11a5c9d88 WebCore::insert(WebCore::HTMLConstructionSiteTask&)
8   0x11a5c97eb WebCore::executeInsertTask(WebCore::HTMLConstructionSiteTask&)
9   0x11a5b58e6 WebCore::executeTask(WebCore::HTMLConstructionSiteTask&)
10  0x11a5b57a2 WebCore::HTMLConstructionSite::executeQueuedTasks()
11  0x11a5f3310 WebCore::HTMLTreeBuilder::constructTree(WebCore::AtomicHTMLToken&&)
12  0x11a5beb17 WebCore::HTMLDocumentParser::constructTreeFromHTMLToken(WebCore::HTMLTokenizer::TokenPtr&)
13  0x11a5be7e8 WebCore::HTMLDocumentParser::pumpTokenizerLoop(WebCore::HTMLDocumentParser::SynchronousMode, bool, WebCore::PumpSession&)
14  0x11a5bd0c8 WebCore::HTMLDocumentParser::pumpTokenizer(WebCore::HTMLDocumentParser::SynchronousMode)
15  0x11a5bcc3b WebCore::HTMLDocumentParser::pumpTokenizerIfPossible(WebCore::HTMLDocumentParser::SynchronousMode)
16  0x11a5bf68a WebCore::HTMLDocumentParser::append(WTF::RefPtr<WTF::StringImpl, WTF::DumbPtrTraits<WTF::StringImpl> >&&)
17  0x11a13d89c WebCore::Document::setContent(WTF::String const&)
18  0x11b6ebedb WebCore::DOMParser::parseFromString(WTF::String const&, WTF::String const&)
19  0x118a81e69 WebCore::jsDOMParserPrototypeFunctionParseFromStringBody(JSC::ExecState*, WebCore::JSDOMParser*, JSC::ThrowScope&)
20  0x118a705ee long long WebCore::IDLOperation<WebCore::JSDOMParser>::call<&(WebCore::jsDOMParserPrototypeFunctionParseFromStringBody(JSC::ExecState*, WebCore::JSDOMParser*, JSC::ThrowScope&)), (WebCore::CastedThisErrorBehavior)0>(JSC::ExecState&, char const*)
21  0x118a7037c WebCore::jsDOMParserPrototypeFunctionParseFromString(JSC::ExecState*)
22  0x5596e08a178
23  0x114f3fd36 llint_entry
24  0x114f37e12 vmEntryToJavaScript
25  0x1158b550e JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*)
26  0x11585ad0d JSC::Interpreter::executeProgram(JSC::SourceCode const&, JSC::ExecState*, JSC::JSObject*)
27  0x115adbbe7 JSC::evaluate(JSC::ExecState*, JSC::SourceCode const&, JSC::JSValue, WTF::NakedPtr<JSC::Exception>&)
28  0x115adbd70 JSC::profiledEvaluate(JSC::ExecState*, JSC::ProfilingReason, JSC::SourceCode const&, JSC::JSValue, WTF::NakedPtr<JSC::Exception>&)
29  0x119cf76fb WebCore::JSMainThreadExecState::profiledEvaluate(JSC::ExecState*, JSC::ProfilingReason, JSC::SourceCode const&, JSC::JSValue, WTF::NakedPtr<JSC::Exception>&)
30  0x119cf74e8 WebCore::ScriptController::evaluateInWorld(WebCore::ScriptSourceCode const&, WebCore::DOMWrapperWorld&, WebCore::ExceptionDetails*)
31  0x119cf77dd WebCore::ScriptController::evaluate(WebCore::ScriptSourceCode const&, WebCore::ExceptionDetails*)
LEAK: 1 WebPageProxy

https://build.webkit.org/results/Apple%20iOS%2011%20Simulator%20Debug%20WK2%20(Tests)/r226223%20(1751)/results.html
Comment 1 Ryan Haddad 2017-12-21 10:55:03 PST
Flakiness dashboard blames https://trac.webkit.org/log/webkit/?verbose=on&rev=226154&stop_rev=226150

https://trac.webkit.org/changeset/226150/webkit is the only one out of those changes that seems like it could be responsible.
Comment 2 Ryan Haddad 2017-12-21 11:13:55 PST
The crash is consistent and only on iOS.
Comment 3 Jer Noble 2017-12-21 12:43:03 PST
(In reply to Ryan Haddad from comment #1)
> Flakiness dashboard blames
> https://trac.webkit.org/log/webkit/?verbose=on&rev=226154&stop_rev=226150
> 
> https://trac.webkit.org/changeset/226150/webkit is the only one out of those
> changes that seems like it could be responsible.

Here's the diagnosis:

In HTMLMediaElement::insertedIntoAncestor(), we call prepareForLoad(), which calls configureMediaControls(), which calls ensureMediaControlsShadowRoot(), which adds the ShadowRoot to the media element.  But this is happening half-way through notifyNodeInsertedIntoDocument(), the end of which tries to notify all the node's children that they've been inserted. However, due to ensureMediaControlsShadowRoot(), the root has already been notified, and has it's flag set, hence the ASSERTion.

This doesn't happen on Mac because prepareForLoad() and selectMediaResource() are called when the src= attribute is parsed. On iOS, loading isn't permitted without a user gesture, and selectMediaResource() isn't called, which doesn't set m_networkState to NETWORK_NO_SOURCE. So when the insertion into the document happens, m_networkState is still NETWORK_EMPTY, and we call prepareForLoad() again.

The short-term solution is to not try to create the media controls until the element is fully in the document. Luckily, that code already exists in HTMLMediaElement::didFinishInsertingNode().
Comment 4 Jer Noble 2017-12-21 13:43:52 PST
Created attachment 330059 [details]
Patch
Comment 5 EWS Watchlist 2017-12-21 14:37:55 PST
Comment on attachment 330059 [details]
Patch

Attachment 330059 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/5792940

New failing tests:
media/video-fullscreeen-only-controls.html
Comment 6 EWS Watchlist 2017-12-21 14:37:56 PST
Created attachment 330070 [details]
Archive of layout-test-results from ews100 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 7 EWS Watchlist 2017-12-21 14:54:20 PST
Comment on attachment 330059 [details]
Patch

Attachment 330059 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/5793172

New failing tests:
media/video-fullscreeen-only-controls.html
Comment 8 EWS Watchlist 2017-12-21 14:54:21 PST
Created attachment 330071 [details]
Archive of layout-test-results from ews107 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6
Comment 9 EWS Watchlist 2017-12-21 15:14:16 PST
Comment on attachment 330059 [details]
Patch

Attachment 330059 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/5793174

New failing tests:
svg/custom/object-sizing-explicit-height.xhtml
media/video-fullscreeen-only-controls.html
Comment 10 EWS Watchlist 2017-12-21 15:14:17 PST
Created attachment 330073 [details]
Archive of layout-test-results from ews116 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 11 Radar WebKit Bug Importer 2017-12-21 16:45:33 PST
<rdar://problem/36188093>
Comment 12 Jer Noble 2018-01-02 00:33:36 PST
Created attachment 330310 [details]
Patch

Despite Ryosuke's review, uploading a new patch for re-review, since the implementation has changed significantly.
Comment 13 WebKit Commit Bot 2018-01-08 08:46:19 PST
Comment on attachment 330310 [details]
Patch

Clearing flags on attachment: 330310

Committed r226514: <https://trac.webkit.org/changeset/226514>
Comment 14 WebKit Commit Bot 2018-01-08 08:46:21 PST
All reviewed patches have been landed.  Closing bug.