Bug 181204 – [WK1] Layout Test fast/events/beforeunload-dom-manipulation-crash.html is crashing

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore JavaScript

[Arunprasad]

Refer https://build.webkit.org/builders/Apple%20Win%207%20Release%20%28Tests%29/builds/3613/steps/layout-test/logs/stdio

To reproduce the issue with nightly build, download latest build from https://webkit.org/build-archives/ and execute the DRT binary like below,

MallocCheckHeapStart=1000 MallocCheckHeapEach=100 DYLD_LIBRARY_PATH=./Release DYLD_FRAMEWORK_PATH=./Release ./Release/DumpRenderTree <WebKit-Source>/LayoutTests/fast/events/beforeunload-dom-manipulation-crash.html

It crashes with following dump,

DumpRenderTree(15907,0x7fff96307340) malloc: checks heap after 1000th operation and each 100 operations
DumpRenderTree(15907,0x7fff96307340) malloc: will sleep for 100 seconds on heap corruption
2018-01-01 12:57:51.938 DumpRenderTree[15907:232832] NetworkStorageDB:_openDBReadConnections: failed to open read connection to DB @ (null)/Cache.db.  Error=14. Cause=unable to open database file
2018-01-01 12:57:51.938 DumpRenderTree[15907:232832] CacheRead: unable to open cache files in (null)
DumpRenderTree(15907,0x7fff96307340) malloc: at szone_check counter=10000
CONSOLE MESSAGE: line 19: TypeError: testRunner.forceImmediateCompletion is not a function. (In 'testRunner.forceImmediateCompletion()', 'testRunner.forceImmediateCompletion' is undefined)
#CRASHED
Segmentation fault: 11

[Arunprasad]

https://bugs.webkit.org/show_bug.cgi?id=177071 fixes partially

[Alexey Proskuryakov]

Thank you for following up! I now filed internal rdar://problem/36256274 for a problem that is related and may or may not have the same root cause. So we'll be looking into what else is going on with this test.

[Per Arne Vollan]

Created attachment 332274 [details]
Patch

[Per Arne Vollan]

<rdar://problem/36256274>

[Brent Fulgham]

Comment on attachment 332274 [details]
Patch

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

I think this looks good, but I'd like Ryosuke to take a quick look before approving.

> Source/WebCore/loader/FrameLoader.cpp:1675
> +    if (!isStopLoadingAllowed())

It seems reasonable to treat a "stopAllLoaders" differently than real navigations. The whole 'beforeunload' should probably be removed entirely.

[EWS Watchlist]

Comment on attachment 332274 [details]
Patch

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

New failing tests:
fast/events/beforeunload-dom-manipulation-crash.html

[EWS Watchlist]

Created attachment 332281 [details]
Archive of layout-test-results from ews101 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 332274 [details]
Patch

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

New failing tests:
fast/events/beforeunload-dom-manipulation-crash.html

[EWS Watchlist]

Created attachment 332284 [details]
Archive of layout-test-results from ews104 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6

[Per Arne Vollan]

Created attachment 332285 [details]
Patch

[EWS Watchlist]

Comment on attachment 332285 [details]
Patch

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

New failing tests:
fast/events/beforeunload-dom-manipulation-crash.html

[EWS Watchlist]

Created attachment 332290 [details]
Archive of layout-test-results from ews102 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-sierra  Platform: Mac OS X 10.12.6

[Per Arne Vollan]

Created attachment 332293 [details]
Patch

[Per Arne Vollan]

Created attachment 332298 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 332298 [details]
Patch

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

r=me with the new release assertions.

> Source/WebCore/loader/FrameLoader.cpp:1674
>      ASSERT(!m_frame.document() || m_frame.document()->pageCacheState() != Document::InPageCache);

Add RELEASE_ASSERT(ScriptDisallowedScope::InMainThread::isScriptAllowed()) here and FrameLoader::frameDetached().

[Per Arne Vollan]

(In reply to Ryosuke Niwa from comment #15)
> Comment on attachment 332298 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=332298&action=review
> 
> r=me with the new release assertions.
> 
> > Source/WebCore/loader/FrameLoader.cpp:1674
> >      ASSERT(!m_frame.document() || m_frame.document()->pageCacheState() != Document::InPageCache);
> 
> Add RELEASE_ASSERT(ScriptDisallowedScope::InMainThread::isScriptAllowed())
> here and FrameLoader::frameDetached().

Thanks for reviewing! I will update the patch.

[Per Arne Vollan]

Created attachment 332375 [details]
Patch

[EWS Watchlist]

Comment on attachment 332375 [details]
Patch

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

New failing tests:
css3/shapes/shape-outside/shape-image/shape-image-006.html
css3/shapes/shape-outside/shape-image/shape-image-020.html

[EWS Watchlist]

Created attachment 332380 [details]
Archive of layout-test-results from ews101 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-sierra  Platform: Mac OS X 10.12.6

[Per Arne Vollan]

Created attachment 332381 [details]
Patch

[EWS Watchlist]

Comment on attachment 332375 [details]
Patch

Attachment 332375 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/6222471

New failing tests:
svg/custom/global-constructors.html

[EWS Watchlist]

Created attachment 332385 [details]
Archive of layout-test-results from ews122 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews122  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 332375 [details]
Patch

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

New failing tests:
imported/w3c/web-platform-tests/css/css-shapes/shape-outside/shape-image/shape-image-014.html

[EWS Watchlist]

Created attachment 332386 [details]
Archive of layout-test-results from ews106 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 332381 [details]
Patch

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

New failing tests:
css3/shapes/shape-outside/shape-image/shape-image-020.html

[EWS Watchlist]

Created attachment 332390 [details]
Archive of layout-test-results from ews100 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 332381 [details]
Patch

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

New failing tests:
css3/shapes/shape-outside/shape-image/shape-image-021.html
css3/shapes/shape-outside/shape-image/shape-image-014.html
imported/w3c/web-platform-tests/css/css-shapes/shape-outside/shape-image/shape-image-006.html
css3/shapes/shape-outside/shape-image/shape-image-006.html
http/tests/security/text-track-crossorigin.html

[EWS Watchlist]

Created attachment 332395 [details]
Archive of layout-test-results from ews114 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 332375 [details]
Patch

Attachment 332375 [details] did not pass win-ews (win):
Output: http://webkit-queues.webkit.org/results/6223102

New failing tests:
css3/shapes/shape-outside/shape-image/shape-image-011.html
fast/shapes/shape-outside-floats/shape-outside-floats-image-margin-003.html
fast/shapes/shape-outside-floats/shape-outside-image-set.html
fast/shapes/shape-outside-floats/shape-outside-image-fit-005.html
fast/shapes/shape-outside-floats/shape-outside-image-fit-001.html
css3/shapes/shape-outside/shape-image/shape-image-007.html
fast/shapes/shape-outside-floats/shape-outside-image-fit-003.html
fast/shapes/shape-outside-floats/shape-outside-floats-margin-crash.html
svg/custom/empty-className-baseVal-crash.html
css3/shapes/shape-outside/shape-image/shape-image-014.html
css3/shapes/shape-outside/shape-image/shape-image-005.html
css3/shapes/shape-outside/shape-image/shape-image-002.html
fast/shapes/shape-outside-floats/shape-outside-image-fit-004.html
css3/shapes/shape-outside/shape-image/shape-image-003.html
fast/shapes/shape-outside-floats/shape-outside-image-fit-006.html
css3/shapes/shape-outside/shape-image/shape-image-020.html
imported/blink/fast/shapes/shape-outside-floats/shape-outside-image-too-big.html
http/tests/security/svg-image-with-css-cross-domain.html
fast/shapes/shape-outside-floats/shape-outside-floats-image-threshold-002.html

[EWS Watchlist]

Created attachment 332398 [details]
Archive of layout-test-results from ews206 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews206  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

[EWS Watchlist]

Comment on attachment 332381 [details]
Patch

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

New failing tests:
imported/w3c/web-platform-tests/css/css-shapes/shape-outside/shape-image/shape-image-021.html

[EWS Watchlist]

Created attachment 332399 [details]
Archive of layout-test-results from ews104 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6

[Per Arne Vollan]

(In reply to Ryosuke Niwa from comment #15)
> Comment on attachment 332298 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=332298&action=review
> 
> r=me with the new release assertions.
> 
> > Source/WebCore/loader/FrameLoader.cpp:1674
> >      ASSERT(!m_frame.document() || m_frame.document()->pageCacheState() != Document::InPageCache);
> 
> Add RELEASE_ASSERT(ScriptDisallowedScope::InMainThread::isScriptAllowed())
> here and FrameLoader::frameDetached().

It seems we cannot currently add these asserts, since some tests are crashing. We should look into this, but I think it can be done independently of this bug. I will file a new bug about this.

[Per Arne Vollan]

(In reply to Per Arne Vollan from comment #33)
> (In reply to Ryosuke Niwa from comment #15)
> > Comment on attachment 332298 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=332298&action=review
> > 
> > r=me with the new release assertions.
> > 
> > > Source/WebCore/loader/FrameLoader.cpp:1674
> > >      ASSERT(!m_frame.document() || m_frame.document()->pageCacheState() != Document::InPageCache);
> > 
> > Add RELEASE_ASSERT(ScriptDisallowedScope::InMainThread::isScriptAllowed())
> > here and FrameLoader::frameDetached().
> 
> It seems we cannot currently add these asserts, since some tests are
> crashing. We should look into this, but I think it can be done independently
> of this bug. I will file a new bug about this.

https://bugs.webkit.org/show_bug.cgi?id=182186

[Per Arne Vollan]

(In reply to Per Arne Vollan from comment #33)
> (In reply to Ryosuke Niwa from comment #15)
> > Comment on attachment 332298 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=332298&action=review
> > 
> > r=me with the new release assertions.
> > 
> > > Source/WebCore/loader/FrameLoader.cpp:1674
> > >      ASSERT(!m_frame.document() || m_frame.document()->pageCacheState() != Document::InPageCache);
> > 
> > Add RELEASE_ASSERT(ScriptDisallowedScope::InMainThread::isScriptAllowed())
> > here and FrameLoader::frameDetached().
> 
> It seems we cannot currently add these asserts, since some tests are
> crashing. We should look into this, but I think it can be done independently
> of this bug. I will file a new bug about this.

Ryosuke, do we still have a 'r+' without the release asserts?

[Per Arne Vollan]

(In reply to Per Arne Vollan from comment #35)
> (In reply to Per Arne Vollan from comment #33)
> > (In reply to Ryosuke Niwa from comment #15)
> > > Comment on attachment 332298 [details]
> > > Patch
> > > 
> > > View in context:
> > > https://bugs.webkit.org/attachment.cgi?id=332298&action=review
> > > 
> > > r=me with the new release assertions.
> > > 
> > > > Source/WebCore/loader/FrameLoader.cpp:1674
> > > >      ASSERT(!m_frame.document() || m_frame.document()->pageCacheState() != Document::InPageCache);
> > > 
> > > Add RELEASE_ASSERT(ScriptDisallowedScope::InMainThread::isScriptAllowed())
> > > here and FrameLoader::frameDetached().
> > 
> > It seems we cannot currently add these asserts, since some tests are
> > crashing. We should look into this, but I think it can be done independently
> > of this bug. I will file a new bug about this.
> 
> Ryosuke, do we still have a 'r+' without the release asserts?

I believe this change is safe, since FrameLoader::stopAllLoaders does not seem to dispatch any events (please correct me if I am wrong).

[Ryosuke Niwa]

We need to look at those failing tests (backtraces).

It's possible that there are some call sites at which it's not safe to execute scripts in stopAllLoaders. Given my analysis, stopAllLoaders can definitely execute scripts in some cases.

[Per Arne Vollan]

(In reply to Ryosuke Niwa from comment #37)
> We need to look at those failing tests (backtraces).
> 
> It's possible that there are some call sites at which it's not safe to
> execute scripts in stopAllLoaders. Given my analysis, stopAllLoaders can
> definitely execute scripts in some cases.

It seems most or all of the crashes have the following backtrace:

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   com.apple.WebCore             	0x000000010fda5f2f 0x10ef76000 + 14876463
1   com.apple.WebCore             	0x000000010fdb8a5f WebCore::FrameLoader::frameDetached() + 47 (FrameLoader.cpp:2560)
2   com.apple.WebCore             	0x00000001103ab5b8 WebCore::SVGImage::~SVGImage() + 56 (memory:2537)
3   com.apple.WebCore             	0x00000001103ab62e WebCore::SVGImage::~SVGImage() + 14 (RefCounted.h:140)
4   com.apple.WebCore             	0x000000010fe09f93 WebCore::CachedImage::clearImage() + 291 (CachedImage.cpp:422)
5   com.apple.WebCore             	0x000000010fe09d4d WebCore::CachedImage::~CachedImage() + 29 (memory:2733)
6   com.apple.WebCore             	0x000000010fe09fee WebCore::CachedImage::~CachedImage() + 14 (CachedResource.h:59)
7   com.apple.WebCore             	0x000000010fe0f6f2 WebCore::CachedResource::deleteIfPossible() + 130 (CachedResource.cpp:608)
8   com.apple.WebCore             	0x000000010fe0ff7c WebCore::CachedResource::unregisterHandle(WebCore::CachedResourceHandleBase*) + 188 (CachedResource.cpp:787)
9   com.apple.WebCore             	0x000000010fc1cbe9 WebCore::HTMLImageElement::~HTMLImageElement() + 201 (HTMLElement.h:38)
10  com.apple.WebCore             	0x000000010fc1cd3e WebCore::HTMLImageElement::~HTMLImageElement() + 14 (Node.h:81)
11  com.apple.WebCore             	0x000000010fa6dd2f WebCore::addChildNodesToDeletionQueue(WebCore::Node*&, WebCore::Node*&, WebCore::ContainerNode&) + 287 (Node.h:725)
12  com.apple.WebCore             	0x000000010fa678b7 WebCore::ContainerNode::removeDetachedChildren() + 103 (ContainerNodeAlgorithms.cpp:213)
13  com.apple.WebCore             	0x000000010fa68188 WebCore::ContainerNode::~ContainerNode() + 56 (ContainerNode.cpp:267)
14  com.apple.WebCore             	0x000000010fc0099e WebCore::HTMLBodyElement::~HTMLBodyElement() + 14 (Node.h:81)
15  com.apple.WebCore             	0x000000010fa6dd2f WebCore::addChildNodesToDeletionQueue(WebCore::Node*&, WebCore::Node*&, WebCore::ContainerNode&) + 287 (Node.h:725)
16  com.apple.WebCore             	0x000000010fa678b7 WebCore::ContainerNode::removeDetachedChildren() + 103 (ContainerNodeAlgorithms.cpp:213)
17  com.apple.WebCore             	0x000000010fa68188 WebCore::ContainerNode::~ContainerNode() + 56 (ContainerNode.cpp:267)
18  com.apple.WebCore             	0x000000010fc26cde WebCore::HTMLHtmlElement::~HTMLHtmlElement() + 14 (Node.h:81)
19  com.apple.WebCore             	0x000000010fa6dd2f WebCore::addChildNodesToDeletionQueue(WebCore::Node*&, WebCore::Node*&, WebCore::ContainerNode&) + 287 (Node.h:725)
20  com.apple.WebCore             	0x000000010fa678b7 WebCore::ContainerNode::removeDetachedChildren() + 103 (ContainerNodeAlgorithms.cpp:213)
21  com.apple.WebCore             	0x000000010fa81460 WebCore::Document::removedLastRef() + 656 (memory:2733)
22  com.apple.JavaScriptCore      	0x0000000113cddba9 void JSC::MarkedBlock::Handle::specializedSweep<true, (JSC::MarkedBlock::Handle::EmptyMode)1, (JSC::MarkedBlock::Handle::SweepMode)1, (JSC::MarkedBlock::Handle::SweepDestructionMode)1, (JSC::MarkedBlock::Handle::ScribbleMode)0, (JSC::MarkedBlock::Handle::NewlyAllocatedMode)1, (JSC::MarkedBlock::Handle::MarksMode)1, JSC::JSDestructibleObjectDestroyFunc>(JSC::FreeList*, JSC::MarkedBlock::Handle::EmptyMode, JSC::MarkedBlock::Handle::SweepMode, JSC::MarkedBlock::Handle::SweepDestructionMode, JSC::MarkedBlock::Handle::ScribbleMode, JSC::MarkedBlock::Handle::NewlyAllocatedMode, JSC::MarkedBlock::Handle::MarksMode, JSC::JSDestructibleObjectDestroyFunc const&) + 201 (JSDestructibleObjectHeapCellType.cpp:37)
23  com.apple.JavaScriptCore      	0x0000000113cdc585 void JSC::MarkedBlock::Handle::finishSweepKnowingHeapCellType<JSC::JSDestructibleObjectDestroyFunc>(JSC::FreeList*, JSC::JSDestructibleObjectDestroyFunc const&)::'lambda'()::operator()() const + 357 (MarkedBlockInlines.h:413)
24  com.apple.JavaScriptCore      	0x0000000113cdc159 void JSC::MarkedBlock::Handle::finishSweepKnowingHeapCellType<JSC::JSDestructibleObjectDestroyFunc>(JSC::FreeList*, JSC::JSDestructibleObjectDestroyFunc const&) + 313 (MarkedBlockInlines.h:425)
25  com.apple.JavaScriptCore      	0x0000000113cdc01a JSC::JSDestructibleObjectHeapCellType::finishSweep(JSC::MarkedBlock::Handle&, JSC::FreeList*) + 26 (JSDestructibleObjectHeapCellType.cpp:53)
26  com.apple.JavaScriptCore      	0x00000001139c7d20 JSC::MarkedBlock::Handle::sweep(JSC::FreeList*) + 320 (MarkedBlock.cpp:418)
27  com.apple.JavaScriptCore      	0x00000001139c58e3 JSC::LocalAllocator::tryAllocateIn(JSC::MarkedBlock::Handle*) + 35 (FreeList.h:91)
28  com.apple.JavaScriptCore      	0x00000001139c5829 JSC::LocalAllocator::tryAllocateWithoutCollecting() + 41 (LocalAllocator.cpp:208)
29  com.apple.JavaScriptCore      	0x00000001139c5738 JSC::LocalAllocator::allocateSlowCase(JSC::GCDeferralContext*, JSC::AllocationFailureMode) + 296 (LocalAllocator.cpp:157)
30  com.apple.JavaScriptCore      	0x00000001139a694a JSC::CompleteSubspace::allocateNonVirtual(JSC::VM&, unsigned long, JSC::GCDeferralContext*, JSC::AllocationFailureMode) + 170 (LocalAllocatorInlines.h:37)
31  com.apple.WebCore             	0x000000010f874256 std::__1::enable_if<std::is_same<WebCore::HTMLDocument, WebCore::HTMLDocument>::value, WebCore::JSDOMWrapperConverterTraits<WebCore::HTMLDocument>::WrapperClass*>::type WebCore::createWrapper<WebCore::HTMLDocument, WebCore::HTMLDocument>(WebCore::JSDOMGlobalObject*, WTF::Ref<WebCore::HTMLDocument, WTF::DumbPtrTraits<WebCore::HTMLDocument> >&&) + 214 (JSCellInlines.h:151)
32  com.apple.WebCore             	0x000000010f8702a1 WebCore::toJSNewlyCreated(JSC::ExecState*, WebCore::JSDOMGlobalObject*, WTF::Ref<WebCore::Document, WTF::DumbPtrTraits<WebCore::Document> >&&) + 81 (JSDOMWrapperCache.h:195)
33  com.apple.WebCore             	0x000000010f8703eb WebCore::toJS(JSC::ExecState*, WebCore::JSDOMGlobalObject*, WebCore::Document&) + 75 (JSDocumentCustom.cpp:88)
34  com.apple.WebCore             	0x000000010f86b534 WebCore::JSDOMWindowBase::updateDocument() + 132 (JSNodeCustom.h:46)
35  com.apple.WebCore             	0x000000010f890575 WebCore::ScriptController::updateDocument() + 197 (ScriptController.cpp:512)
36  com.apple.WebCore             	0x000000010fa879c0 WebCore::Document::didBecomeCurrentDocumentInFrame() + 32 (Document.h:603)
37  com.apple.WebCore             	0x000000010fe6850b WebCore::Frame::setDocument(WTF::RefPtr<WebCore::Document, WTF::DumbPtrTraits<WebCore::Document> >&&) + 331 (RefPtr.h:88)
38  com.apple.WebCore             	0x000000010fda61ea WebCore::DocumentWriter::begin(WebCore::URL const&, bool, WebCore::Document*) + 698 (utility:753)
39  com.apple.WebCore             	0x000000010fd94aba WebCore::DocumentLoader::commitData(char const*, unsigned long) + 186 (utility:753)
40  com.apple.WebKit              	0x000000010de635cc WebKit::WebFrameLoaderClient::committedLoad(WebCore::DocumentLoader*, char const*, int) + 50 (WebFrameLoaderClient.cpp:1009)
41  com.apple.WebCore             	0x000000010fd976c4 WebCore::DocumentLoader::commitLoad(char const*, int) + 148 (DocumentLoader.h:244)
42  com.apple.WebCore             	0x000000010fe0cbcb WebCore::CachedRawResource::notifyClientsDataWasReceived(char const*, unsigned int) + 123 (CachedRawResource.cpp:116)
43  com.apple.WebCore             	0x000000010fe0ca7a WebCore::CachedRawResource::updateBuffer(WebCore::SharedBuffer&) + 186 (CachedRawResource.cpp:65)
44  com.apple.WebCore             	0x000000010fde476a WebCore::SubresourceLoader::didReceiveDataOrBuffer(char const*, int, WTF::RefPtr<WebCore::SharedBuffer, WTF::DumbPtrTraits<WebCore::SharedBuffer> >&&, long long, WebCore::DataPayloadType) + 186 (SubresourceLoader.cpp:430)
45  com.apple.WebCore             	0x000000010fde46a2 WebCore::SubresourceLoader::didReceiveData(char const*, unsigned int, long long, WebCore::DataPayloadType) + 34 (SubresourceLoader.cpp:399)
46  com.apple.WebKit              	0x000000010df5e9f5 WebKit::WebResourceLoader::didReceiveData(IPC::DataReference const&, long long) + 85 (WebResourceLoader.cpp:134)
47  com.apple.WebKit              	0x000000010df5f44d WebKit::WebResourceLoader::didReceiveWebResourceLoaderMessage(IPC::Connection&, IPC::Decoder&) + 277 (HandleMessage.h:40)
48  com.apple.WebKit              	0x000000010dd31673 WebKit::NetworkProcessConnection::didReceiveMessage(IPC::Connection&, IPC::Decoder&) + 453 (NetworkProcessConnection.cpp:98)
49  com.apple.WebKit              	0x000000010dc9f99f IPC::Connection::dispatchMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >) + 119 (memory:2714)
50  com.apple.WebKit              	0x000000010dca2504 IPC::Connection::dispatchOneMessage() + 176 (Connection.cpp:965)
51  com.apple.JavaScriptCore      	0x0000000113f475e6 WTF::RunLoop::performWork() + 214 (Function.h:56)
52  com.apple.JavaScriptCore      	0x0000000113f47882 WTF::RunLoop::performWork(void*) + 34 (RunLoopCF.cpp:39)
53  com.apple.CoreFoundation      	0x00007fffc2f7f3e1 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 17
54  com.apple.CoreFoundation      	0x00007fffc2f6065c __CFRunLoopDoSources0 + 556
55  com.apple.CoreFoundation      	0x00007fffc2f5fb46 __CFRunLoopRun + 934
56  com.apple.CoreFoundation      	0x00007fffc2f5f544 CFRunLoopRunSpecific + 420
57  com.apple.HIToolbox           	0x00007fffc24beebc RunCurrentEventLoopInMode + 240
58  com.apple.HIToolbox           	0x00007fffc24becf1 ReceiveNextEventCommon + 432
59  com.apple.HIToolbox           	0x00007fffc24beb26 _BlockUntilNextEventMatchingListInModeWithFilter + 71
60  com.apple.AppKit              	0x00007fffc0a55a54 _DPSNextEvent + 1120
61  com.apple.AppKit              	0x00007fffc11d17ee -[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:] + 2796
62  com.apple.AppKit              	0x00007fffc0a4a3db -[NSApplication run] + 926
63  com.apple.AppKit              	0x00007fffc0a14e0e NSApplicationMain + 1237
64  libxpc.dylib                  	0x00007fffd8d828c7 _xpc_objc_main + 775
65  libxpc.dylib                  	0x00007fffd8d812e4 xpc_main + 494
66  com.apple.WebKit.WebContent   	0x000000010dc4569a main + 490 (XPCServiceMain.mm:122)
67  libdyld.dylib                 	0x00007fffd8b29235 start + 1

[Ryosuke Niwa]

Ah, okay. I think we need to add a variant of frameDetached to called in SVGImage::~SVGImage() which doesn't assert.

It's okay for scripts to execute there because SVGImage has its own page, frame, document, etc... and it doesn't have access to a document in which the SVG image appears.

[Per Arne Vollan]

Created attachment 332450 [details]
Patch

[Per Arne Vollan]

Created attachment 332472 [details]
Patch for landing

[Per Arne Vollan]

(In reply to Ryosuke Niwa from comment #39)
> Ah, okay. I think we need to add a variant of frameDetached to called in
> SVGImage::~SVGImage() which doesn't assert.
> 
> It's okay for scripts to execute there because SVGImage has its own page,
> frame, document, etc... and it doesn't have access to a document in which
> the SVG image appears.

Thanks! Since FrameLoader::frameDetached() is only called from two sites, I added the assert only to the site not related to SVG.

[WebKit Commit Bot]

Comment on attachment 332472 [details]
Patch for landing

Clearing flags on attachment: 332472

Committed r227731: <https://trac.webkit.org/changeset/227731>

[Matt Lewis]

This commit caused an assertion failure for the API test WebKit.DidRemoveFrameFromHiearchyInPageCache

TIMEOUT WebKit.DidRemoveFrameFromHiearchyInPageCache
ASSERTION FAILED: ScriptDisallowedScope::InMainThread::isScriptAllowed()
/Volumes/Data/slave/highsierra-debug/build/Source/WebCore/html/HTMLFrameOwnerElement.cpp(84) : void WebCore::HTMLFrameOwnerElement::disconnectContentFrame()
1   0x7886ee1ad WTFCrash
2   0x77a10ff0d WebCore::HTMLFrameOwnerElement::disconnectContentFrame()
3   0x779d5f222 WebCore::disconnectSubframes(WebCore::ContainerNode&, WebCore::SubframeDisconnectPolicy)
4   0x779d5ac02 WebCore::disconnectSubframesIfNeeded(WebCore::ContainerNode&, WebCore::SubframeDisconnectPolicy)
5   0x779d5abc7 WebCore::ContainerNode::disconnectDescendantFrames()
6   0x779dadad8 WebCore::Document::prepareForDestruction()
7   0x77a084663 WebCore::CachedFrame::destroy()
8   0x77a0863b9 WebCore::CachedPage::~CachedPage()
9   0x77a0864a5 WebCore::CachedPage::~CachedPage()
10  0x77a089f1c WebCore::PageCache::prune(WebCore::PruningReason)
11  0x77a08ac13 WebCore::PageCache::addIfCacheable(WebCore::HistoryItem&, WebCore::Page*)
12  0x77a4b8f63 WebCore::FrameLoader::commitProvisionalLoad()
13  0x77a46480c WebCore::DocumentLoader::commitIfReady()
14  0x77a468f0c WebCore::DocumentLoader::commitLoad(char const*, int)
15  0x77a468eaf WebCore::DocumentLoader::dataReceived(char const*, int)
16  0x77a4697b4 WebCore::DocumentLoader::dataReceived(WebCore::CachedResource&, char const*, int)
17  0x77a4697fa non-virtual thunk to WebCore::DocumentLoader::dataReceived(WebCore::CachedResource&, char const*, int)
18  0x77a5847f8 WebCore::CachedRawResource::notifyClientsDataWasReceived(char const*, unsigned int)
19  0x77a58468d WebCore::CachedRawResource::updateBuffer(WebCore::SharedBuffer&)
20  0x77a52349a WebCore::SubresourceLoader::didReceiveDataOrBuffer(char const*, int, WTF::RefPtr<WebCore::SharedBuffer, WTF::DumbPtrTraits<WebCore::SharedBuffer> >&&, long long, WebCore::DataPayloadType)
21  0x77a523262 WebCore::SubresourceLoader::didReceiveData(char const*, unsigned int, long long, WebCore::DataPayloadType)
22  0x10dc117a4 WebKit::WebResourceLoader::didReceiveData(IPC::DataReference const&, long long)
23  0x10dc15140 void IPC::callMemberFunctionImpl<WebKit::WebResourceLoader, void (WebKit::WebResourceLoader::*)(IPC::DataReference const&, long long), std::__1::tuple<IPC::DataReference, long long>, 0ul, 1ul>(WebKit::WebResourceLoader*, void (WebKit::WebResourceLoader::*)(IPC::DataReference const&, long long), std::__1::tuple<IPC::DataReference, long long>&&, std::__1::integer_sequence<unsigned long, 0ul, 1ul>)
24  0x10dc15070 void IPC::callMemberFunction<WebKit::WebResourceLoader, void (WebKit::WebResourceLoader::*)(IPC::DataReference const&, long long), std::__1::tuple<IPC::DataReference, long long>, std::__1::integer_sequence<unsigned long, 0ul, 1ul> >(std::__1::tuple<IPC::DataReference, long long>&&, WebKit::WebResourceLoader*, void (WebKit::WebResourceLoader::*)(IPC::DataReference const&, long long))
25  0x10dc144d1 void IPC::handleMessage<Messages::WebResourceLoader::DidReceiveData, WebKit::WebResourceLoader, void (WebKit::WebResourceLoader::*)(IPC::DataReference const&, long long)>(IPC::Decoder&, WebKit::WebResourceLoader*, void (WebKit::WebResourceLoader::*)(IPC::DataReference const&, long long))
26  0x10dc13c96 WebKit::WebResourceLoader::didReceiveWebResourceLoaderMessage(IPC::Connection&, IPC::Decoder&)
27  0x10d26fd89 WebKit::NetworkProcessConnection::didReceiveMessage(IPC::Connection&, IPC::Decoder&)
28  0x10cffb333 IPC::Connection::dispatchMessage(IPC::Decoder&)
29  0x10cff0918 IPC::Connection::dispatchMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >)
30  0x10cffb93a IPC::Connection::dispatchOneMessage()
31  0x10d013dfd IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >)::$_14::operator()()

The assertion is happening on all Debug testers.

https://build.webkit.org/builders/Apple%20High%20Sierra%20Debug%20WK1%20%28Tests%29/builds/2104/steps/run-api-tests/logs/stdio
https://build.webkit.org/builders/Apple%20High%20Sierra%20Debug%20WK1%20%28Tests%29/builds/2104

[Matt Lewis]

Reverted r227731 for reason:

This caused and assertion failure in API tests.

Committed r227743: <https://trac.webkit.org/changeset/227743>

[Per Arne Vollan]

Created attachment 332578 [details]
Patch

[Per Arne Vollan]

Created attachment 332599 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 332599 [details]
Patch

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

> Source/WebCore/loader/FrameLoader.cpp:2559
> +        // FrameLoader::stopAllLoaders() might dispatch events.
> +        RELEASE_ASSERT(ScriptDisallowedScope::InMainThread::isScriptAllowed());
>          stopAllLoaders();

Why don't we add this to stopAllLoaders() itself?

[EWS Watchlist]

Comment on attachment 332599 [details]
Patch

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

New failing tests:
http/tests/security/text-track-crossorigin.html

[EWS Watchlist]

Created attachment 332611 [details]
Archive of layout-test-results from ews106 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 332599 [details]
Patch

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

New failing tests:
css3/shapes/shape-outside/shape-image/shape-image-021.html
imported/w3c/web-platform-tests/css/css-shapes/shape-outside/shape-image/shape-image-021.html

[EWS Watchlist]

Created attachment 332615 [details]
Archive of layout-test-results from ews114 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 332599 [details]
Patch

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

New failing tests:
css3/shapes/shape-outside/shape-image/shape-image-007.html
css3/shapes/shape-outside/shape-image/shape-image-016.html
css3/shapes/shape-outside/shape-image/shape-image-003.html
css3/shapes/shape-outside/shape-image/shape-image-012.html

[EWS Watchlist]

Created attachment 332617 [details]
Archive of layout-test-results from ews102 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-sierra  Platform: Mac OS X 10.12.6

[Per Arne Vollan]

Created attachment 332618 [details]
Patch

[EWS Watchlist]

Comment on attachment 332599 [details]
Patch

Attachment 332599 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/6255738

New failing tests:
fast/shapes/shape-outside-floats/shape-outside-image-fit-005.html

[EWS Watchlist]

Created attachment 332619 [details]
Archive of layout-test-results from ews122 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews122  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 332599 [details]
Patch

Attachment 332599 [details] did not pass win-ews (win):
Output: http://webkit-queues.webkit.org/results/6257157

New failing tests:
css3/shapes/shape-outside/shape-image/shape-image-011.html
fast/shapes/shape-outside-floats/shape-outside-floats-image-threshold-002.html
fast/shapes/shape-outside-floats/shape-outside-floats-image-margin-003.html
fast/shapes/shape-outside-floats/shape-outside-image-set.html
fast/shapes/shape-outside-floats/shape-outside-image-fit-005.html
fast/shapes/shape-outside-floats/shape-outside-image-fit-001.html
css3/shapes/shape-outside/shape-image/shape-image-007.html
fast/shapes/shape-outside-floats/shape-outside-image-fit-003.html
fast/shapes/shape-outside-floats/shape-outside-floats-margin-crash.html
css3/shapes/shape-outside/shape-image/shape-image-014.html
css3/shapes/shape-outside/shape-image/shape-image-005.html
css3/shapes/shape-outside/shape-image/shape-image-002.html
fast/shapes/shape-outside-floats/shape-outside-image-fit-004.html
css3/shapes/shape-outside/shape-image/shape-image-003.html
fast/shapes/shape-outside-floats/shape-outside-image-fit-006.html
css3/shapes/shape-outside/shape-image/shape-image-020.html
imported/blink/fast/shapes/shape-outside-floats/shape-outside-image-too-big.html
http/tests/security/svg-image-with-css-cross-domain.html
svg/custom/empty-className-baseVal-crash.html

[EWS Watchlist]

Created attachment 332629 [details]
Archive of layout-test-results from ews205 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews205  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

[Per Arne Vollan]

Created attachment 332660 [details]
Patch

[Per Arne Vollan]

Created attachment 332677 [details]
Patch

[EWS Watchlist]

Comment on attachment 332677 [details]
Patch

Attachment 332677 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/6264888

New failing tests:
imported/w3c/web-platform-tests/service-workers/service-worker/navigation-redirect.https.html

[EWS Watchlist]

Created attachment 332705 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.6

[Per Arne Vollan]

Created attachment 332716 [details]
Patch

[Per Arne Vollan]

Created attachment 332725 [details]
Patch

[Per Arne Vollan]

Created attachment 332818 [details]
Patch

[Per Arne Vollan]

(In reply to Per Arne Vollan from comment #66)
> Created attachment 332818 [details]
> Patch

Uploaded a variant of the patch for EWS testing.

[Ryosuke Niwa]

Comment on attachment 332725 [details]
Patch

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

> Source/WebCore/loader/FrameLoader.cpp:1677
>  

We need to assert there that scripts are enabled. Just disable the assertion in SVGImage's destructor using ScriptDisallowedScope::DisableAssertionsInScope.

[Per Arne Vollan]

Created attachment 332829 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 332829 [details]
Patch

Clearing flags on attachment: 332829

Committed r227948: <https://trac.webkit.org/changeset/227948>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.