Bug 188750 - [WebGL] Contexts are not updated when display configuration changed.
Summary: [WebGL] Contexts are not updated when display configuration changed.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Per Arne Vollan
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-08-20 11:16 PDT by Per Arne Vollan
Modified: 2018-08-22 14:14 PDT (History)
10 users (show)

See Also:


Attachments
Patch (22.50 KB, patch)
2018-08-20 14:10 PDT, Per Arne Vollan
no flags Details | Formatted Diff | Diff
Patch (26.48 KB, patch)
2018-08-20 16:41 PDT, Per Arne Vollan
bfulgham: review+
ews-watchlist: commit-queue-
Details | Formatted Diff | Diff
Patch (27.52 KB, patch)
2018-08-20 17:46 PDT, Per Arne Vollan
ews-watchlist: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews102 for mac-sierra (2.48 MB, application/zip)
2018-08-20 18:12 PDT, EWS Watchlist
no flags Details
Archive of layout-test-results from ews106 for mac-sierra-wk2 (2.99 MB, application/zip)
2018-08-20 18:48 PDT, EWS Watchlist
no flags Details
Archive of layout-test-results from ews125 for ios-simulator-wk2 (23.34 MB, application/zip)
2018-08-20 18:50 PDT, EWS Watchlist
no flags Details
Archive of layout-test-results from ews105 for mac-sierra-wk2 (2.84 MB, application/zip)
2018-08-20 21:28 PDT, EWS Watchlist
no flags Details
Patch (27.45 KB, patch)
2018-08-21 08:36 PDT, Per Arne Vollan
no flags Details | Formatted Diff | Diff
Patch (27.99 KB, patch)
2018-08-21 09:10 PDT, Per Arne Vollan
no flags Details | Formatted Diff | Diff
Patch (28.04 KB, patch)
2018-08-21 09:37 PDT, Per Arne Vollan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Per Arne Vollan 2018-08-20 11:16:19 PDT
Calling CGDisplayRegisterReconfigurationCallback in GraphicsContext3DManager::addContext returns kCGErrorSuccess when WindowServer access is blocked in the WebContent process, but the callback function is never called. We should register the callback function in the UI process, and send a message to the WebContent process when the display configuration changed.
Comment 1 Per Arne Vollan 2018-08-20 14:10:53 PDT
Created attachment 347540 [details]
Patch
Comment 2 Per Arne Vollan 2018-08-20 16:41:48 PDT
Created attachment 347567 [details]
Patch
Comment 3 Brent Fulgham 2018-08-20 16:46:28 PDT
Comment on attachment 347567 [details]
Patch

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

Looks like a good change. r=me (but please confirm EWS is happy).

> Source/WebCore/platform/graphics/GraphicsContext3DManager.h:28
> +#include <WebCore/Timer.h>

Do we need to prefix this with WebCore when building in WebCore? Or is this header accessed outside WebCore?

> Source/WebCore/platform/graphics/GraphicsContext3DManager.h:71
> +

Please remove this whitespace-only change.
Comment 4 Brent Fulgham 2018-08-20 16:46:57 PDT
Comment on attachment 347567 [details]
Patch

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

> Source/WebCore/ChangeLog:4
> +        https://bugs.webkit.org/show_bug.cgi?id=188750

Is there a radar associated with this?
Comment 5 Radar WebKit Bug Importer 2018-08-20 16:49:18 PDT
<rdar://problem/43534354>
Comment 6 Per Arne Vollan 2018-08-20 16:52:08 PDT
(In reply to Brent Fulgham from comment #3)
> Comment on attachment 347567 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=347567&action=review
> 
> Looks like a good change. r=me (but please confirm EWS is happy).
> 
> > Source/WebCore/platform/graphics/GraphicsContext3DManager.h:28
> > +#include <WebCore/Timer.h>
> 
> Do we need to prefix this with WebCore when building in WebCore? Or is this
> header accessed outside WebCore?

Yes, this file is now accessed from WebKit.

> 
> > Source/WebCore/platform/graphics/GraphicsContext3DManager.h:71
> > +
> 
> Please remove this whitespace-only change.

Will do. I will also create a radar.

Thanks for reviewing!
Comment 7 Per Arne Vollan 2018-08-20 17:46:10 PDT
Created attachment 347587 [details]
Patch
Comment 8 Simon Fraser (smfr) 2018-08-20 17:57:31 PDT
Comment on attachment 347587 [details]
Patch

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

> Source/WebCore/platform/graphics/GraphicsContext3DManager.h:28
> +#include <WebCore/Timer.h>

I think this should just be "#include "Timer".

> Source/WebKit/UIProcess/API/C/mac/WKMockDisplay.h:35
> +    WK_EXPORT void WKSendDisplayConfigurationChangedMessageForTesting(WKContextRef);

No indenting here.
Comment 9 EWS Watchlist 2018-08-20 18:12:36 PDT
Comment on attachment 347567 [details]
Patch

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

New failing tests:
fast/canvas/webgl/context-update-on-display-configuration.html
Comment 10 EWS Watchlist 2018-08-20 18:12:38 PDT
Created attachment 347589 [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 11 EWS Watchlist 2018-08-20 18:48:06 PDT
Comment on attachment 347567 [details]
Patch

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

New failing tests:
fast/canvas/webgl/context-update-on-display-configuration.html
Comment 12 EWS Watchlist 2018-08-20 18:48:09 PDT
Created attachment 347594 [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
Comment 13 EWS Watchlist 2018-08-20 18:50:07 PDT
Comment on attachment 347567 [details]
Patch

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

New failing tests:
fast/canvas/webgl/context-update-on-display-configuration.html
Comment 14 EWS Watchlist 2018-08-20 18:50:09 PDT
Created attachment 347596 [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
Comment 15 Per Arne Vollan 2018-08-20 20:49:41 PDT
(In reply to Simon Fraser (smfr) from comment #8)
> Comment on attachment 347587 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=347587&action=review
> 
> > Source/WebCore/platform/graphics/GraphicsContext3DManager.h:28
> > +#include <WebCore/Timer.h>
> 
> I think this should just be "#include "Timer".
> 
> > Source/WebKit/UIProcess/API/C/mac/WKMockDisplay.h:35
> > +    WK_EXPORT void WKSendDisplayConfigurationChangedMessageForTesting(WKContextRef);
> 
> No indenting here.

Thanks for reviewing! I will update the patch.
Comment 16 EWS Watchlist 2018-08-20 21:28:21 PDT
Comment on attachment 347587 [details]
Patch

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

New failing tests:
fast/canvas/webgl/context-update-on-display-configuration.html
Comment 17 EWS Watchlist 2018-08-20 21:28:23 PDT
Created attachment 347608 [details]
Archive of layout-test-results from ews105 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6
Comment 18 Per Arne Vollan 2018-08-21 08:36:50 PDT
Created attachment 347642 [details]
Patch
Comment 19 Per Arne Vollan 2018-08-21 09:10:54 PDT
Created attachment 347646 [details]
Patch
Comment 20 Per Arne Vollan 2018-08-21 09:37:16 PDT
Created attachment 347650 [details]
Patch
Comment 21 WebKit Commit Bot 2018-08-21 11:04:55 PDT
Comment on attachment 347650 [details]
Patch

Clearing flags on attachment: 347650

Committed r235125: <https://trac.webkit.org/changeset/235125>
Comment 22 Ryosuke Niwa 2018-08-21 19:04:14 PDT
Landed a build fix in https://trac.webkit.org/changeset/235154.