Bug 79321 - [chromium] Clean up GraphicsContext3D initialization paths
Summary: [chromium] Clean up GraphicsContext3D initialization paths
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: James Robinson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-02-22 20:22 PST by James Robinson
Modified: 2012-02-23 17:38 PST (History)
7 users (show)

See Also:

Attachments
Patch (29.58 KB, patch)
2012-02-22 20:30 PST, James Robinson 		no flags Details | Formatted Diff | Diff
delta from last patch (4.07 KB, patch)
2012-02-23 15:31 PST, James Robinson 		no flags Details | Formatted Diff | Diff
Patch (31.82 KB, patch)
2012-02-23 15:39 PST, James Robinson 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description James Robinson 2012-02-22 20:22:15 PST 
[chromium] Clean up GraphicsContext3D initialization paths
Comment 1 James Robinson 2012-02-22 20:30:01 PST 
Created attachment 128378 [details]
Patch
Comment 2 Kenneth Russell 2012-02-23 13:03:54 PST 
Comment on attachment 128378 [details]
Patch

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

Looks good. r=me

> Source/WebKit/chromium/ChangeLog:21
> +        2) GraphicsContext3DPrivate::createGraphicsContextFromWebContext() wraps the WebGraphicsContext3D in a

Missing end of comment?

> Source/WebKit/chromium/src/GraphicsContext3DPrivate.h:58
> +    // via ContextLost before using the context in any way.. Once made current on a thread, the context cannot

Two periods after "way".
Comment 3 James Robinson 2012-02-23 13:08:57 PST 
Committed r108666: <http://trac.webkit.org/changeset/108666>
Comment 4 Adrienne Walker 2012-02-23 15:11:56 PST 
Committed r108679: <http://trac.webkit.org/changeset/108679>
Comment 5 Adrienne Walker 2012-02-23 15:12:46 PST 
Breaks tests.  See commit above.
Comment 6 James Robinson 2012-02-23 15:30:43 PST 
This broke some plugin tests because I wasn't properly retaining the temporary graphics context across WebViewImpl::graphicsContext3D() calls before the compositor was fully initialized.  I've made an attempt at making this a bit clearer, because it sure confused the heck out of me.
Comment 7 James Robinson 2012-02-23 15:31:33 PST 
Created attachment 128568 [details]
delta from last patch
Comment 8 James Robinson 2012-02-23 15:39:39 PST 
Created attachment 128571 [details]
Patch
Comment 9 James Robinson 2012-02-23 15:40:21 PST 
Ken - would you mind taking another look?  The differences here are in WebViewImpl::graphicsContext3D() and WebViewImpl::createLayerTreeHost3D().  I've added some inline comments to try to explain what the code is trying to do a bit more explicitly.
Comment 10 Kenneth Russell 2012-02-23 17:29:16 PST 
Comment on attachment 128571 [details]
Patch

The delta looks fine to me.
Comment 11 James Robinson 2012-02-23 17:38:19 PST 
Comment on attachment 128571 [details]
Patch

Clearing flags on attachment: 128571

Committed r108706: <http://trac.webkit.org/changeset/108706>
Comment 12 James Robinson 2012-02-23 17:38:23 PST 
All reviewed patches have been landed.  Closing bug.