Bug 98051 - Would like a way to customize the type of GraphicsLayers created on a per page basis
Summary: Would like a way to customize the type of GraphicsLayers created on a per pag...
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: Anders Carlsson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-10-01 11:27 PDT by Anders Carlsson
Modified: 2012-10-01 13:43 PDT (History)
0 users

See Also:


Attachments
Patch (4.18 KB, patch)
2012-10-01 11:31 PDT, Anders Carlsson
no flags Details | Formatted Diff | Diff
Patch (8.87 KB, patch)
2012-10-01 12:23 PDT, Anders Carlsson
simon.fraser: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Anders Carlsson 2012-10-01 11:27:07 PDT
Would like a way to customize the type of GraphicsLayers created on a per page basis
Comment 1 Anders Carlsson 2012-10-01 11:31:39 PDT
Created attachment 166511 [details]
Patch
Comment 2 Simon Fraser (smfr) 2012-10-01 11:43:18 PDT
Comment on attachment 166511 [details]
Patch

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

> Source/WebCore/platform/graphics/GraphicsLayerClient.h:63
> +    // Allows for a GraphicsLayerClient to customize the kind of GraphicsLayer being created on a per client basis.
> +    virtual PassOwnPtr<GraphicsLayer> createCustomGraphicsLayer() { return nullptr; }

It seems a bit odd to have a create method on the client. Client methods are usually callbacks that involve a specific graphics layer; this one is called before you've even created one. And since the client is calling GraphicsLayer::create() already, why doesn't it just make the custom type? This level of indirection also makes it hard for the client to pass any context that's needed to decide what kind of GraphicsLayer to create.

I think a factory pattern actually makes more sense here.
Comment 3 Anders Carlsson 2012-10-01 11:52:31 PDT
(In reply to comment #2)
> (From update of attachment 166511 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=166511&action=review
> 
> > Source/WebCore/platform/graphics/GraphicsLayerClient.h:63
> > +    // Allows for a GraphicsLayerClient to customize the kind of GraphicsLayer being created on a per client basis.
> > +    virtual PassOwnPtr<GraphicsLayer> createCustomGraphicsLayer() { return nullptr; }
> 
> It seems a bit odd to have a create method on the client. Client methods are usually callbacks that involve a specific graphics layer; this one is called before you've even created one. And since the client is calling GraphicsLayer::create() already, why doesn't it just make the custom type? This level of indirection also makes it hard for the client to pass any context that's needed to decide what kind of GraphicsLayer to create.

Yeah I agree that it's weird. I didn't want to have to add a separate parameter to GraphicsLayer::create, but I can do that if you think it's better.

> 
> I think a factory pattern actually makes more sense here.

Cool. Do you think it should be a parameter passed to GraphicsLayer::create?
Comment 4 Anders Carlsson 2012-10-01 12:23:07 PDT
Created attachment 166524 [details]
Patch
Comment 5 Anders Carlsson 2012-10-01 13:43:51 PDT
Committed r130072: <http://trac.webkit.org/changeset/130072>