Bug 69776 – [chromium] Check for lost context at beginning of compositor's execution

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Kenneth Russell]

Currently the compositor checks for the loss of OpenGL context at the end of its execution. In order to implement dynamic GPU switching we need to handle the case where the creation of a WebGL context provokes the loss of the compositor's context. This means that the compositor needs to check for lost context at the beginning of its run as well.

[Kenneth Russell]

Created attachment 110409 [details]
Patch

[James Robinson]

Comment on attachment 110409 [details]
Patch

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

Will this test actually fail without the .cpp changes?  I think in general it won't - we don't run the layout tests on any bots that will trigger GPU switching (since we're using osmesa) so I don't think this is providing coverage really - although I may be misunderstanding the testcase.

Can you use the existing layoutTestController.loseCompositorContext() functionality to emulate this failure mode?

> LayoutTests/platform/chromium/compositing/webgl-loses-compositor-context.html:11
> +      layoutTestController.overridePreference("WebKitWebGLEnabled", "1");

does this do anything in Chromium?

[Kenneth Russell]

Created attachment 110438 [details]
Patch

[Kenneth Russell]

(In reply to comment #2)
> (From update of attachment 110409 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=110409&action=review
> 
> Will this test actually fail without the .cpp changes?  I think in general it won't - we don't run the layout tests on any bots that will trigger GPU switching (since we're using osmesa) so I don't think this is providing coverage really - although I may be misunderstanding the testcase.

It asserts when run in Chrome. I was planning to add it as a UI layout test.

> Can you use the existing layoutTestController.loseCompositorContext() functionality to emulate this failure mode?

I've added this to the test case but unfortunately it doesn't provoke the assertion failure. In order to do so we'll need to force the loss of context at a lower level -- at the GraphicsContext3D level, I think.

> > LayoutTests/platform/chromium/compositing/webgl-loses-compositor-context.html:11
> > +      layoutTestController.overridePreference("WebKitWebGLEnabled", "1");
> 
> does this do anything in Chromium?

It was necessary in order to get the WebGL context to be created.

[Kenneth Russell]

Ping. This blocks the fix for http://crbug.com/88788 .

[James Robinson]

Comment on attachment 110438 [details]
Patch

Looks good.  If you have time, it'd be great if you could plumb loseCompositorContext() through deeper to make the test more useful.

[Kenneth Russell]

(In reply to comment #6)
> (From update of attachment 110438 [details])
> Looks good.  If you have time, it'd be great if you could plumb loseCompositorContext() through deeper to make the test more useful.

Yes, I'd like to do that. Filed https://bugs.webkit.org/show_bug.cgi?id=69878 to track it.

[WebKit Review Bot]

Comment on attachment 110438 [details]
Patch

Clearing flags on attachment: 110438

Committed r97194: <http://trac.webkit.org/changeset/97194>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.