Release ASSERT when reloading Vimeo page @ WebCore: WebCore::Document::updateLayout
<rdar://problem/36186214>
Created attachment 331712 [details] Patch
Comment on attachment 331712 [details] Patch Attachment 331712 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/6131807 New failing tests: media/video-fullscreen-reload-crash.html
Created attachment 331714 [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
Comment on attachment 331712 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=331712&action=review > Source/WebKit/WebProcess/cocoa/VideoFullscreenManager.mm:62 > + if (document.activeDOMObjectsAreSuspended() || document.activeDOMObjectsAreStopped()) It's not obvious why active DOM objects being suspended means that you can't do layout. What does it mean for an "active" DOM object to be suspended? Is that reflecting some higher-level state (being in the page cache?) that we should be checking for?
(In reply to Simon Fraser (smfr) from comment #5) > Comment on attachment 331712 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=331712&action=review > > > Source/WebKit/WebProcess/cocoa/VideoFullscreenManager.mm:62 > > + if (document.activeDOMObjectsAreSuspended() || document.activeDOMObjectsAreStopped()) > > It's not obvious why active DOM objects being suspended means that you can't > do layout. What does it mean for an "active" DOM object to be suspended? Is > that reflecting some higher-level state (being in the page cache?) that we > should be checking for? It hits " RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(isSafeToUpdateStyleOrLayout())" in Document::updateLayout(). isSafeToUpdateStyleOrLayout() itself calls ScriptDisallowedScope::InMainThread::isScriptAllowed(), which is set by ScriptExecutionContext() while stopping ActiveDOMObjects. So I suppose we could check Document:: isSafeToUpdateStyleOrLayout() instead.
That's what (Z)Alan suggests.
Created attachment 331792 [details] Patch for landing
Comment on attachment 331792 [details] Patch for landing Attachment 331792 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/6142286 New failing tests: media/video-fullscreen-reload-crash.html
Created attachment 331818 [details] Archive of layout-test-results from ews126 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews126 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.6
Created attachment 331834 [details] Patch for landing
Comment on attachment 331834 [details] Patch for landing Clearing flags on attachment: 331834 Committed r227272: <https://trac.webkit.org/changeset/227272>
All reviewed patches have been landed. Closing bug.
Comment on attachment 331712 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=331712&action=review >>> Source/WebKit/WebProcess/cocoa/VideoFullscreenManager.mm:62 >>> + if (document.activeDOMObjectsAreSuspended() || document.activeDOMObjectsAreStopped()) >> >> It's not obvious why active DOM objects being suspended means that you can't do layout. What does it mean for an "active" DOM object to be suspended? Is that reflecting some higher-level state (being in the page cache?) that we should be checking for? > > It hits " RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(isSafeToUpdateStyleOrLayout())" in Document::updateLayout(). isSafeToUpdateStyleOrLayout() itself calls ScriptDisallowedScope::InMainThread::isScriptAllowed(), which is set by ScriptExecutionContext() while stopping ActiveDOMObjects. > > So I suppose we could check Document:: isSafeToUpdateStyleOrLayout() instead. I think this check would have been better.
Comment on attachment 331712 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=331712&action=review >>>> Source/WebKit/WebProcess/cocoa/VideoFullscreenManager.mm:62 >>>> + if (document.activeDOMObjectsAreSuspended() || document.activeDOMObjectsAreStopped()) >>> >>> It's not obvious why active DOM objects being suspended means that you can't do layout. What does it mean for an "active" DOM object to be suspended? Is that reflecting some higher-level state (being in the page cache?) that we should be checking for? >> >> It hits " RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(isSafeToUpdateStyleOrLayout())" in Document::updateLayout(). isSafeToUpdateStyleOrLayout() itself calls ScriptDisallowedScope::InMainThread::isScriptAllowed(), which is set by ScriptExecutionContext() while stopping ActiveDOMObjects. >> >> So I suppose we could check Document:: isSafeToUpdateStyleOrLayout() instead. > > I think this check would have been better. Another check would have been to check the presence of RenderView. At this point, the render view had been destroyed.