Bug 123818

Summary: Remote Layer Tree: Apply layer changes to LayerTypeCustom layers to the custom layer in the Web process, not to the CALayerHost
Product: WebKit Reporter: Tim Horton <thorton>
Component: WebKit2Assignee: Tim Horton <thorton>
Severity: Normal CC: andersca, eric.carlson, jer.noble, simon.fraser
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Description Flags
patch andersca: review+

Description Tim Horton 2013-11-05 12:59:28 PST
Given the implementation of the remote layer tree at the moment, changes to properties of a LayerTypeCustom PlatformCALayerRemote will get bundled up, shipped to the UI process, and applied to the CALayerHost that is hosting the Web-process-owned custom layer tree.

This is not the intent of code that sets properties on this layer.

It instead intends to affect the platformLayer owned by the aforementioned PlatformCALayerRemote, which in our case happens to live in the Web process.

So, we have to apply these property changes to the correct layer.

I'm going to do this by sharing the code to apply a LayerProperties struct to a CALayer with both processes, and using it when doing a Web process side commit to push the LayerTypeCustom's layer changes onto its PlatformLayer instead of including them in the transaction.

This means that changes to LayerTypeCustom layers will happen before (and unpredictably out of sync with) the UI process commit, but it's the best solution I can come up with for now.

This is the last patch required to get <video> working.
Comment 1 Tim Horton 2013-11-05 13:08:55 PST
Created attachment 216062 [details]
Comment 2 Anders Carlsson 2013-11-05 13:54:05 PST
Comment on attachment 216062 [details]

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

> Source/WebKit2/Shared/mac/RemoteLayerTreePropertyApplier.h:39
> +    typedef HashMap<RemoteLayerTreeTransaction::LayerID, PlatformLayer *> RelatedLayerMap;
> +    static void applyPropertiesToLayer(PlatformLayer *, RemoteLayerTreeTransaction::LayerProperties, RelatedLayerMap);
> +    static void disableActionsForLayer(PlatformLayer *);

Please use CALayer instead of PlatformLayer.

> Source/WebKit2/Shared/mac/RemoteLayerTreePropertyApplier.mm:43
> +    RetainPtr<CGColorRef> cgColor = adoptCF(CGColorCreate(colorSpace.get(), components));
> +    return cgColor;

Can just return adoptCF

> Source/WebKit2/Shared/mac/RemoteLayerTreePropertyApplier.mm:158
> +void RemoteLayerTreePropertyApplier::disableActionsForLayer(PlatformLayer *layer)

Again, CALayer!

> Source/WebKit2/Shared/mac/RemoteLayerTreePropertyApplier.mm:162
> +    [layer setStyle:@{ @"actions" : @{
> +        @"anchorPoint" : nullValue,

Please put @“actions” on a new line. Can also use layr.style = here I think.

> Source/WebKit2/Shared/mac/RemoteLayerTreePropertyApplier.mm:173
> +        @"zPosition" : nullValue } }];

} should be on a separate line.
Comment 3 Tim Horton 2013-11-05 14:02:48 PST