Bug 123818 - Remote Layer Tree: Apply layer changes to LayerTypeCustom layers to the custom layer in the Web process, not to the CALayerHost
Summary: Remote Layer Tree: Apply layer changes to LayerTypeCustom layers to the custo...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tim Horton
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-11-05 12:59 PST by Tim Horton
Modified: 2013-11-05 14:02 PST (History)
4 users (show)

See Also:


Attachments
patch (27.38 KB, patch)
2013-11-05 13:08 PST, Tim Horton
andersca: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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]
patch
Comment 2 Anders Carlsson 2013-11-05 13:54:05 PST
Comment on attachment 216062 [details]
patch

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
http://trac.webkit.org/changeset/158691