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
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.
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.
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
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
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
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().
(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.
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
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
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
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
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
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
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
(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.
(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
(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?
(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).
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.
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.
(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.
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
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
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
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
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
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
2018-01-25 09:26 PST, Per Arne Vollan
2018-01-25 10:32 PST, EWS Watchlist
2018-01-25 10:33 PST, EWS Watchlist
2018-01-25 10:39 PST, Per Arne Vollan
2018-01-25 11:24 PST, EWS Watchlist
2018-01-25 11:35 PST, Per Arne Vollan
2018-01-25 12:17 PST, Per Arne Vollan
2018-01-26 09:06 PST, Per Arne Vollan
2018-01-26 10:11 PST, EWS Watchlist
2018-01-26 10:20 PST, Per Arne Vollan
2018-01-26 10:39 PST, EWS Watchlist
2018-01-26 10:49 PST, EWS Watchlist
2018-01-26 11:25 PST, EWS Watchlist
2018-01-26 11:50 PST, EWS Watchlist
2018-01-26 12:00 PST, EWS Watchlist
2018-01-26 12:16 PST, EWS Watchlist
2018-01-26 19:46 PST, Per Arne Vollan
2018-01-27 09:16 PST, Per Arne Vollan
2018-01-29 14:13 PST, Per Arne Vollan
2018-01-29 16:33 PST, Per Arne Vollan
2018-01-29 17:57 PST, EWS Watchlist
2018-01-29 18:40 PST, EWS Watchlist
2018-01-29 18:45 PST, EWS Watchlist
2018-01-29 18:58 PST, Per Arne Vollan
2018-01-29 19:02 PST, EWS Watchlist
2018-01-29 21:54 PST, EWS Watchlist
2018-01-30 09:20 PST, Per Arne Vollan
2018-01-30 11:58 PST, Per Arne Vollan
2018-01-30 14:19 PST, EWS Watchlist
2018-01-30 15:44 PST, Per Arne Vollan
2018-01-30 16:54 PST, Per Arne Vollan
2018-01-31 16:21 PST, Per Arne Vollan
2018-01-31 18:29 PST, Per Arne Vollan