Bug 188750 – [WebGL] Contexts are not updated when display configuration changed.

• Component: WebGL
• Status: RESOLVED FIXED

[Per Arne Vollan]

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.

[Per Arne Vollan]

Created attachment 347540 [details]
Patch

[Per Arne Vollan]

Created attachment 347567 [details]
Patch

[Brent Fulgham]

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.

[Brent Fulgham]

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?

[Radar WebKit Bug Importer]

<rdar://problem/43534354>

[Per Arne Vollan]

(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!

[Per Arne Vollan]

Created attachment 347587 [details]
Patch

[Simon Fraser (smfr)]

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.

[EWS Watchlist]

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

[EWS Watchlist]

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

[EWS Watchlist]

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

[EWS Watchlist]

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

[EWS Watchlist]

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

[EWS Watchlist]

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

[Per Arne Vollan]

(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.

[EWS Watchlist]

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

[EWS Watchlist]

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

[Per Arne Vollan]

Created attachment 347642 [details]
Patch

[Per Arne Vollan]

Created attachment 347646 [details]
Patch

[Per Arne Vollan]

Created attachment 347650 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 347650 [details]
Patch

Clearing flags on attachment: 347650

Committed r235125: <https://trac.webkit.org/changeset/235125>

[Ryosuke Niwa]

Landed a build fix in https://trac.webkit.org/changeset/235154.