Bug 47113 - Move SharedGraphicsContext3D from ChromeClient to Page
Summary: Move SharedGraphicsContext3D from ChromeClient to Page
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Chris Marrin
Depends on:
Reported: 2010-10-04 14:12 PDT by Chris Marrin
Modified: 2010-10-04 17:10 PDT (History)
4 users (show)

See Also:

Patch (7.97 KB, patch)
2010-10-04 14:30 PDT, Chris Marrin
no flags Details | Formatted Diff | Diff
Patch (9.45 KB, patch)
2010-10-04 15:07 PDT, Chris Marrin
jamesr: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Marrin 2010-10-04 14:12:22 PDT
In talking to James Robinson, we've agreed that SharedGraphicsContext3D doesn't need to be in ChromeClient. Page has the same lifetime and there are no platform dependencies in this class.
Comment 1 Chris Marrin 2010-10-04 14:30:45 PDT
Created attachment 69681 [details]
Comment 2 Simon Fraser (smfr) 2010-10-04 14:39:03 PDT
Comment on attachment 69681 [details]

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

> WebCore/page/Page.cpp:783
> +        if (!context)
> +            return 0;
> +        m_sharedGraphicsContext3D = SharedGraphicsContext3D::create(context.release());
> +    }

I think the GraphicsContext3D::create() stuff should be hidden inside of 
static bool SharedGraphicsContext3D::canCreateContext() or something, or maybe SharedGraphicsContext3D::create() should just return 0 if the context can't be created.
Comment 3 Chris Marrin 2010-10-04 15:07:41 PDT
Created attachment 69695 [details]
Comment 4 James Robinson 2010-10-04 17:03:42 PDT
Comment on attachment 69695 [details]

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

Looks fine to me.

> WebCore/page/Page.cpp:69
> +#include "SharedGraphicsContext3D.h"
> +#endif

Normally conditionally included headers are put after the main list of headers, not in the middle.
Comment 5 Chris Marrin 2010-10-04 17:10:23 PDT
Landed in http://trac.webkit.org/changeset/69053