Bug 47113

Summary: Move SharedGraphicsContext3D from ChromeClient to Page
Product: WebKit Reporter: Chris Marrin <cmarrin>
Component: Layout and RenderingAssignee: Chris Marrin <cmarrin>
Status: RESOLVED FIXED    
Severity: Normal CC: jamesr, kbr, senorblanco, simon.fraser
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
Patch
none
Patch jamesr: review+

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]
Patch
Comment 2 Simon Fraser (smfr) 2010-10-04 14:39:03 PDT
Comment on attachment 69681 [details]
Patch

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]
Patch
Comment 4 James Robinson 2010-10-04 17:03:42 PDT
Comment on attachment 69695 [details]
Patch

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

Looks fine to me.

> WebCore/page/Page.cpp:69
> +#if ENABLE(ACCELERATED_2D_CANVAS)
> +#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