Bug 98051 – Would like a way to customize the type of GraphicsLayers created on a per page basis

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Anders Carlsson]

Would like a way to customize the type of GraphicsLayers created on a per page basis

[Anders Carlsson]

Created attachment 166511 [details]
Patch

[Simon Fraser (smfr)]

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.

[Anders Carlsson]

(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?

[Anders Carlsson]

Created attachment 166524 [details]
Patch

[Anders Carlsson]

Committed r130072: <http://trac.webkit.org/changeset/130072>