Bug 186226 – Fullscreen requires active document.

• Component: Media
• Status: RESOLVED CONFIGURATION CHANGED

[Jeremy Jones]

Fullscreen requires active document.

[Jeremy Jones]

rdar://problem/36187413

[Jeremy Jones]

Created attachment 341832 [details]
wip

[EWS Watchlist]

Comment on attachment 341832 [details]
wip

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

New failing tests:
media/no-fullscreen-from-background.html

[EWS Watchlist]

Created attachment 341834 [details]
Archive of layout-test-results from ews103 for mac-sierra

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

[EWS Watchlist]

Comment on attachment 341832 [details]
wip

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

New failing tests:
media/no-fullscreen-from-background.html

[EWS Watchlist]

Created attachment 341835 [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 341832 [details]
wip

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

New failing tests:
media/no-fullscreen-from-background.html

[EWS Watchlist]

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

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

[EWS Watchlist]

Comment on attachment 341832 [details]
wip

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

New failing tests:
http/tests/security/video-poster-cross-origin-crash2.html

[EWS Watchlist]

Created attachment 341840 [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

[EWS Watchlist]

Comment on attachment 341832 [details]
wip

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

New failing tests:
media/no-fullscreen-from-background.html

[EWS Watchlist]

Created attachment 341868 [details]
Archive of layout-test-results from ews116 for mac-sierra

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

[Jeremy Jones]

Created attachment 343833 [details]
Patch

[Jer Noble]

Comment on attachment 343833 [details]
Patch

I don't think this will do what you want; we should be using the document's visibility state rather than whether active DOM objects are suspended.

[Jeremy Jones]

Created attachment 343859 [details]
Patch

[Jer Noble]

Better, but I think it could use a test. TAL at internals.setPageVisibility(false) to simulate a page going into the background.

[Jeremy Jones]

Created attachment 343873 [details]
Patch

[EWS Watchlist]

Attachment 343873 [details] did not pass style-queue:


ERROR: Source/WebCore/page/ChromeClient.h:490:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
Total errors found: 1 in 10 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Jeremy Jones]

Created attachment 343945 [details]
Patch

[EWS Watchlist]

Attachment 343945 [details] did not pass style-queue:


ERROR: Source/WebCore/page/ChromeClient.h:490:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
Total errors found: 1 in 20 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[EWS Watchlist]

Comment on attachment 343945 [details]
Patch

Attachment 343945 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/8387773

New failing tests:
media/no-fullscreen-when-hidden.html

[EWS Watchlist]

Created attachment 343956 [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 343945 [details]
Patch

Attachment 343945 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/8387640

New failing tests:
http/tests/security/canvas-remote-read-remote-video-localhost.html

[EWS Watchlist]

Created attachment 343957 [details]
Archive of layout-test-results from ews202 for win-future

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

[EWS Watchlist]

Comment on attachment 343945 [details]
Patch

Attachment 343945 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/8387879

New failing tests:
media/no-fullscreen-when-hidden.html

[EWS Watchlist]

Created attachment 343965 [details]
Archive of layout-test-results from ews116 for mac-sierra

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

[Jeremy Jones]

Created attachment 343972 [details]
Patch

[EWS Watchlist]

Attachment 343972 [details] did not pass style-queue:


ERROR: Source/WebCore/page/ChromeClient.h:490:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
Total errors found: 1 in 23 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Jeremy Jones]

Created attachment 343985 [details]
Patch for landing

[EWS Watchlist]

Attachment 343985 [details] did not pass style-queue:


ERROR: Source/WebCore/page/ChromeClient.h:490:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
Total errors found: 1 in 25 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[EWS Watchlist]

Comment on attachment 343985 [details]
Patch for landing

Attachment 343985 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/8391532

New failing tests:
media/no-fullscreen-when-hidden.html

[EWS Watchlist]

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

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

[Maciej Stachowiak]

Probably shouldn't land as-is due the iOS-sim failure.

[Jeremy Jones]

Created attachment 344478 [details]
Patch for landing.

[EWS Watchlist]

Attachment 344478 [details] did not pass style-queue:


ERROR: Source/WebCore/page/ChromeClient.h:490:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
Total errors found: 1 in 25 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[EWS Watchlist]

Comment on attachment 344478 [details]
Patch for landing.

Attachment 344478 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/8462095

New failing tests:
media/no-fullscreen-when-hidden.html

[EWS Watchlist]

Created attachment 344494 [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 344478 [details]
Patch for landing.

Attachment 344478 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/8462587

New failing tests:
media/no-fullscreen-when-hidden.html

[EWS Watchlist]

Created attachment 344500 [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.13.4

[Jeremy Jones]

Created attachment 344727 [details]
Patch for landing.

[EWS Watchlist]

Attachment 344727 [details] did not pass style-queue:


ERROR: Source/WebCore/page/ChromeClient.h:490:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
Total errors found: 1 in 26 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[EWS Watchlist]

Comment on attachment 344727 [details]
Patch for landing.

Attachment 344727 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/8499310

New failing tests:
media/media-source/only-bcp47-language-tags-accepted-as-valid.html

[EWS Watchlist]

Created attachment 344733 [details]
Archive of layout-test-results from ews103 for mac-sierra

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

[EWS Watchlist]

Comment on attachment 344727 [details]
Patch for landing.

Attachment 344727 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/8499368

New failing tests:
media/no-fullscreen-when-hidden.html
media/media-source/only-bcp47-language-tags-accepted-as-valid.html

[EWS Watchlist]

Created attachment 344735 [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 344727 [details]
Patch for landing.

Attachment 344727 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/8499388

New failing tests:
media/media-source/only-bcp47-language-tags-accepted-as-valid.html

[EWS Watchlist]

Created attachment 344737 [details]
Archive of layout-test-results from ews115 for mac-sierra

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

[EWS Watchlist]

Comment on attachment 344727 [details]
Patch for landing.

Attachment 344727 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/8499399

New failing tests:
media/no-fullscreen-when-hidden.html

[EWS Watchlist]

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

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

[Jeremy Jones]

Created attachment 344908 [details]
Patch for landing.

[Jeremy Jones]

Created attachment 344910 [details]
Patch for landing.

[EWS Watchlist]

Attachment 344910 [details] did not pass style-queue:


ERROR: Source/WebCore/page/ChromeClient.h:490:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
Total errors found: 1 in 27 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[EWS Watchlist]

Comment on attachment 344910 [details]
Patch for landing.

Attachment 344910 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/8522319

New failing tests:
media/media-source/only-bcp47-language-tags-accepted-as-valid.html

[EWS Watchlist]

Created attachment 344916 [details]
Archive of layout-test-results from ews103 for mac-sierra

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

[EWS Watchlist]

Comment on attachment 344910 [details]
Patch for landing.

Attachment 344910 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/8522283

New failing tests:
media/media-source/only-bcp47-language-tags-accepted-as-valid.html

[EWS Watchlist]

Created attachment 344919 [details]
Archive of layout-test-results from ews112 for mac-sierra

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

[Jeremy Jones]

Created attachment 344921 [details]
Patch for landing.

[EWS Watchlist]

Attachment 344921 [details] did not pass style-queue:


ERROR: Source/WebCore/page/ChromeClient.h:490:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
Total errors found: 1 in 27 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[EWS Watchlist]

Comment on attachment 344921 [details]
Patch for landing.

Attachment 344921 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/8523057

New failing tests:
media/no-fullscreen-when-hidden.html

[EWS Watchlist]

Created attachment 344923 [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

[EWS Watchlist]

Comment on attachment 344921 [details]
Patch for landing.

Attachment 344921 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/8523331

New failing tests:
http/tests/security/contentSecurityPolicy/userAgentShadowDOM/allow-video.html

[EWS Watchlist]

Created attachment 344925 [details]
Archive of layout-test-results from ews200 for win-future

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

[Jeremy Jones]

Created attachment 345028 [details]
Patch for landing.

[EWS Watchlist]

Attachment 345028 [details] did not pass style-queue:


ERROR: Source/WebCore/page/ChromeClient.h:490:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
Total errors found: 1 in 16 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[EWS Watchlist]

Comment on attachment 345028 [details]
Patch for landing.

Attachment 345028 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/8534427

New failing tests:
media/no-fullscreen-when-hidden.html

[EWS Watchlist]

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

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

[EWS Watchlist]

Comment on attachment 345028 [details]
Patch for landing.

Attachment 345028 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/8534523

New failing tests:
http/tests/security/video-poster-cross-origin-crash2.html

[EWS Watchlist]

Created attachment 345031 [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

[Jeremy Jones]

Created attachment 345111 [details]
Patch for landing.

[EWS Watchlist]

Attachment 345111 [details] did not pass style-queue:


ERROR: Source/WebCore/page/ChromeClient.h:490:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
Total errors found: 1 in 16 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Commit Bot]

Comment on attachment 345111 [details]
Patch for landing.

Clearing flags on attachment: 345111

Committed r233865: <https://trac.webkit.org/changeset/233865>

[Truitt Savell]

It looks like r233865 <https://trac.webkit.org/changeset/233865/webkit> is causing Timeouts for:

    TestWebKitAPI.Fullscreen.TopContentInset
    TestWebKitAPI.Fullscreen.Delegate

Output link:
https://build.webkit.org/builders/Apple%20Sierra%20Release%20WK1%20%28Tests%29/builds/11562/steps/run-api-tests/logs/stdio

[Jeremy Jones]

(In reply to Truitt Savell from comment #72)
> It looks like r233865 <https://trac.webkit.org/changeset/233865/webkit> is
> causing Timeouts for:
> 
>     TestWebKitAPI.Fullscreen.TopContentInset
>     TestWebKitAPI.Fullscreen.Delegate
> 
> Output link:
> https://build.webkit.org/builders/
> Apple%20Sierra%20Release%20WK1%20%28Tests%29/builds/11562/steps/run-api-
> tests/logs/stdio

I'm looking into updating the tests.

[Jeremy Jones]

(In reply to Jeremy Jones from comment #73)
> (In reply to Truitt Savell from comment #72)
> > It looks like r233865 <https://trac.webkit.org/changeset/233865/webkit> is
> > causing Timeouts for:
> > 
> >     TestWebKitAPI.Fullscreen.TopContentInset
> >     TestWebKitAPI.Fullscreen.Delegate
> > 
> > Output link:
> > https://build.webkit.org/builders/
> > Apple%20Sierra%20Release%20WK1%20%28Tests%29/builds/11562/steps/run-api-
> > tests/logs/stdio
> 
> I'm looking into updating the tests.

The tests need to order front the window.

[Jeremy Jones]

(In reply to Jeremy Jones from comment #74)
> (In reply to Jeremy Jones from comment #73)
> > (In reply to Truitt Savell from comment #72)
> > > It looks like r233865 <https://trac.webkit.org/changeset/233865/webkit> is
> > > causing Timeouts for:
> > > 
> > >     TestWebKitAPI.Fullscreen.TopContentInset
> > >     TestWebKitAPI.Fullscreen.Delegate
> > > 
> > > Output link:
> > > https://build.webkit.org/builders/
> > > Apple%20Sierra%20Release%20WK1%20%28Tests%29/builds/11562/steps/run-api-
> > > tests/logs/stdio
> > 
> > I'm looking into updating the tests.
> 
> The tests need to order front the window.

Fix for these failures is in https://bugs.webkit.org/show_bug.cgi?id=187720

[Chris Dumez]

Comment on attachment 345111 [details]
Patch for landing.

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

> Source/WebCore/html/HTMLMediaElement.cpp:5913
> +    if (!document().page() || !document().page()->chrome().client().isViewVisible()) {

This is triggering Synchronous IPC with the UIProcess in the middle of layout (rdar://problem/42807306)... As far as I can tell, this is also completely unnecessary since we know if the view is visible or not on WebProcess side.

[Chris Dumez]

Comment on attachment 345111 [details]
Patch for landing.

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

> Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp:1317
> +    WebProcess::singleton().parentProcessConnection()->sendSync(Messages::WebPageProxy::GetIsViewVisible(), Messages::WebPageProxy::GetIsViewVisible::Reply(isVisible), m_page.pageID());

Also note that this kind of code in WebKit2 still requires WK2 OWNER approval AFAIK. And if you're introducing synchronous IPC you're likely doing it wrong.

[Tim Nguyen (:ntim)]

I think this is fixed in the current code.