r233865 is causing synchronous IPC in the middle of layout: Exception Type: EXC_BREAKPOINT (SIGTRAP) Exception Codes: 0x0000000000000001, 0x00000001983c5c20 Termination Signal: Trace/BPT trap: 5 Termination Reason: Namespace SIGNAL, Code 0x5 Terminating Process: exc handler [721] Triggered by Thread: 0 Thread 0 name: Dispatch queue: com.apple.main-thread Thread 0 Crashed: 0 WebKit 0x00000001983c5c20 bool IPC::Connection::sendSync<Messages::WebPageProxy::GetIsViewVisible>(Messages::WebPageProxy::GetIsViewVisible&&, Messages::WebPageProxy::GetIsViewVisible::Reply&&, unsigned long long, WTF::Seconds, WTF::OptionSet<IPC::SendSyncOption>) + 296 (Connection.h:430) 1 WebKit 0x00000001983c5b2c bool IPC::Connection::sendSync<Messages::WebPageProxy::GetIsViewVisible>(Messages::WebPageProxy::GetIsViewVisible&&, Messages::WebPageProxy::GetIsViewVisible::Reply&&, unsigned long long, WTF::Seconds, WTF::OptionSet<IPC::SendSyncOption>) + 52 (ScriptDisallowedScope.h:56) 2 WebKit 0x00000001983c5ae4 WebKit::WebChromeClient::isViewVisible() + 76 (WebChromeClient.cpp:1317) 3 WebCore 0x0000000192132ec4 WebCore::HTMLMediaElement::enterFullscreen(unsigned int) + 176 (HTMLMediaElement.cpp:5901) 4 WebCore 0x0000000192120a5c WebCore::HTMLMediaElement::updatePlayState(WebCore::HTMLMediaElement::UpdateState) + 264 (HTMLMediaElement.cpp:5306) 5 WebCore 0x000000019212b618 WebCore::HTMLMediaElement::playInternal() + 972 (HTMLMediaElement.cpp:3551) 6 WebCore 0x000000019212b6cc WebCore::HTMLMediaElement::play() + 176 (HTMLMediaElement.cpp:3479) 7 WebCore 0x0000000192136a04 non-virtual thunk to WebCore::HTMLMediaElement::resumeAutoplaying() + 204 (HTMLMediaElement.cpp:7472) 8 WebCore 0x00000001924880f8 WebCore::PlatformMediaSession::endInterruption(WebCore::PlatformMediaSession::EndInterruptionFlags) + 512 (PlatformMediaSession.cpp:166) 9 WebCore 0x000000019280c320 WebCore::RenderTreeUpdater::tearDownRenderers(WebCore::Element&, WebCore::RenderTreeUpdater::TeardownType, WebCore::RenderTreeBuilder&)::$_5::operator()(unsigned int) const + 48 (RenderTreeUpdater.cpp:583) 10 WebCore 0x000000019280b46c WebCore::RenderTreeUpdater::tearDownRenderers(WebCore::Element&, WebCore::RenderTreeUpdater::TeardownType, WebCore::RenderTreeBuilder&) + 572 (RenderTreeUpdater.cpp:591) 11 WebCore 0x000000019280aca4 WebCore::RenderTreeUpdater::updateElementRenderer(WebCore::Element&, WebCore::Style::ElementUpdate const&) + 408 (RenderTreeUpdater.cpp:321) 12 WebCore 0x000000019280a290 WebCore::RenderTreeUpdater::updateRenderTree(WebCore::ContainerNode&) + 1188 (RenderTreeUpdater.cpp:200) 13 WebCore 0x0000000192809d58 WebCore::RenderTreeUpdater::commit(std::__1::unique_ptr<WebCore::Style::Update const, std::__1::default_delete<WebCore::Style::Update const> >) + 592 (RenderTreeUpdater.cpp:132) 14 WebCore 0x0000000191f353c0 WebCore::Document::resolveStyle(WebCore::Document::ResolveStyleType) + 1036 (Document.cpp:1864) 15 WebCore 0x0000000191f35d7c WebCore::Document::updateStyleIfNeeded() + 344 (Document.cpp:1968) 16 WebCore 0x0000000191de910c WebCore::updateStyleIfNeededForProperty(WebCore::Element&, WebCore::CSSPropertyID) + 108 (CSSComputedStyleDeclaration.cpp:2470) 17 WebCore 0x0000000191de8598 WebCore::ComputedStyleExtractor::propertyValue(WebCore::CSSPropertyID, WebCore::EUpdateLayout) + 144 (CSSComputedStyleDeclaration.cpp:2681) 18 WebCore 0x0000000191de7f7c WebCore::CSSComputedStyleDeclaration::getPropertyValue(WebCore::CSSPropertyID) const + 76 (CSSComputedStyleDeclaration.cpp:2419) 19 WebCore 0x0000000191494144 WebCore::jsCSSStyleDeclarationPrototypeFunctionGetPropertyValue(JSC::ExecState*) + 316 (JSCSSStyleDeclaration.cpp:431)
<rdar://problem/42807306>
Created attachment 346489 [details] Patch
@Jer / Jeremy: Note that there is likely another bug here. Seems very odd that destroying a Media element's renderer would cause it to start autoplaying in the first place.
(In reply to Chris Dumez from comment #3) > @Jer / Jeremy: Note that there is likely another bug here. Seems very odd > that destroying a Media element's renderer would cause it to start > autoplaying in the first place. Filed https://bugs.webkit.org/show_bug.cgi?id=188310 for this.
Comment on attachment 346489 [details] Patch iOS simulator failure looks like a real failure? media/no-fullscreen-when-hidden.html [ Failure ]
(In reply to Geoffrey Garen from comment #5) > Comment on attachment 346489 [details] > Patch > > iOS simulator failure looks like a real failure? > > media/no-fullscreen-when-hidden.html [ Failure ] Yes, look related indeed. I verified it was passing on Mac.
Comment on attachment 346489 [details] Patch Attachment 346489 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: https://webkit-queues.webkit.org/results/8752011 New failing tests: media/no-fullscreen-when-hidden.html
Created attachment 346503 [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.13.4
To Jer as there is apparently a security aspect to this and will requires a little bit more work.
(In reply to Build Bot from comment #7) > New failing tests: > media/no-fullscreen-when-hidden.html Iām guessing we have introduced a new race.
Comment on attachment 346489 [details] Patch Attachment 346489 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/8755092 New failing tests: http/tests/security/canvas-remote-read-remote-video-blocked-no-crossorigin.html
Created attachment 346556 [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 355082 [details] Patch
Comment on attachment 355082 [details] Patch Attachment 355082 [details] did not pass mac-ews (mac): Output: https://webkit-queues.webkit.org/results/10020628 Number of test failures exceeded the failure limit.
Created attachment 355093 [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 355082 [details] Patch Attachment 355082 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/10021010 Number of test failures exceeded the failure limit.
Created attachment 355098 [details] Archive of layout-test-results from ews107 for mac-sierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews107 Port: mac-sierra-wk2 Platform: Mac OS X 10.12.6
Comment on attachment 355082 [details] Patch Attachment 355082 [details] did not pass mac-debug-ews (mac): Output: https://webkit-queues.webkit.org/results/10020766 Number of test failures exceeded the failure limit.
Created attachment 355101 [details] Archive of layout-test-results from ews117 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews117 Port: mac-sierra Platform: Mac OS X 10.12.6
Created attachment 355123 [details] Patch
Comment on attachment 355123 [details] Patch Clearing flags on attachment: 355123 Committed r238322: <https://trac.webkit.org/changeset/238322>
All reviewed patches have been landed. Closing bug.
I think this may have caused the following API test time outs: Timeout TestWebKitAPI.FullscreenZoomInitialFrame.WebKit TestWebKitAPI.ExitFullscreenOnEnterPiP.ElementFullscreen
Looks like this changed the order of IPC messages slightly, and the WKFullscreenWindowController doesn't get a chance to register as a VideoFullscreenModelContext client before the context tells everyone we entered PiP mode, breaking the test.
Both of these tests are incorrect in different ways: In TestWebKitAPI.ExitFullscreenOnEnterPiP.ElementFullscreen, the video element is not the main content; it's too small and it's not playing. We should only cancel fullscreen mode when PiP is entered if the video which was taken into PiP was main content. This worked before by accident; the video element entering PiP makes it the main content, but it wasn't when it was in fullscreen. The new behavior is actually correct; the test needs to be updated to make the video element explicitly the main content, in this case by playing the video during a user gesture. In TestWebKitAPI.FullscreenZoomInitialFrame.WebKit, the WK1 WebView isn't visible at all; it shouldn't be allowed to go into fullscreen. The reason this worked previously is because the "GetIsViewVisible" UIClient method was not implemented in WK1, and it's default return value is true.
Reopening to attach new patch.
Created attachment 355186 [details] Follow-up test fix
Comment on attachment 355186 [details] Follow-up test fix Clearing flags on attachment: 355186 Committed r238348: <https://trac.webkit.org/changeset/238348>