Bug 69776 - [chromium] Check for lost context at beginning of compositor's execution
Summary: [chromium] Check for lost context at beginning of compositor's execution
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Kenneth Russell
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-10-10 12:58 PDT by Kenneth Russell
Modified: 2011-10-11 16:33 PDT (History)
6 users (show)

See Also:


Attachments
Patch (9.10 KB, patch)
2011-10-10 14:59 PDT, Kenneth Russell
no flags Details | Formatted Diff | Diff
Patch (9.41 KB, patch)
2011-10-10 16:55 PDT, Kenneth Russell
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kenneth Russell 2011-10-10 12:58:02 PDT
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.
Comment 1 Kenneth Russell 2011-10-10 14:59:48 PDT
Created attachment 110409 [details]
Patch
Comment 2 James Robinson 2011-10-10 16:10:12 PDT
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?
Comment 3 Kenneth Russell 2011-10-10 16:55:46 PDT
Created attachment 110438 [details]
Patch
Comment 4 Kenneth Russell 2011-10-10 16:59:27 PDT
(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.
Comment 5 Kenneth Russell 2011-10-11 12:14:55 PDT
Ping. This blocks the fix for http://crbug.com/88788 .
Comment 6 James Robinson 2011-10-11 12:27:47 PDT
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.
Comment 7 Kenneth Russell 2011-10-11 16:08:09 PDT
(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.
Comment 8 WebKit Review Bot 2011-10-11 16:33:45 PDT
Comment on attachment 110438 [details]
Patch

Clearing flags on attachment: 110438

Committed r97194: <http://trac.webkit.org/changeset/97194>
Comment 9 WebKit Review Bot 2011-10-11 16:33:49 PDT
All reviewed patches have been landed.  Closing bug.