Bug 123130

Summary: Remote Layer Tree: Implement superlayer, removeFromSuperlayer, replaceSublayer, and adoptSublayers
Product: WebKit Reporter: Tim Horton <thorton>
Component: WebKit2Assignee: Tim Horton <thorton>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, simon.fraser
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
patch
none
address sam's comments
none
we'll assert that they're PlatformCALayerRemotes in just one place andersca: review+

Description Tim Horton 2013-10-21 18:58:39 PDT
There are a few remaining unimplemented layer hierarchy properties that were left that way because they're more complicated than the others. But, we need them!
Comment 1 Tim Horton 2013-10-21 19:17:19 PDT
Created attachment 214807 [details]
patch
Comment 2 Sam Weinig 2013-10-22 07:26:44 PDT
Comment on attachment 214807 [details]
patch

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

> Source/WebKit2/WebProcess/WebPage/mac/PlatformCALayerRemote.cpp:74
> +        static_cast<PlatformCALayerRemote*>(layer.get())->m_superlayer = nullptr;

Can we add a checked cast function for this (e.g. toPlatformCALayerRemote() that asserts the type is PlatformCALayerRemote)?

> Source/WebKit2/WebProcess/WebPage/mac/PlatformCALayerRemote.h:157
> +    PlatformCALayer* m_superlayer;

Seems like m_superlayer is always a PlatformCALayerRemote. Can we make the type more specific here?
Comment 3 Tim Horton 2013-10-22 11:37:36 PDT
Created attachment 214872 [details]
address sam's comments
Comment 4 Tim Horton 2013-10-22 12:29:48 PDT
Created attachment 214876 [details]
we'll assert that they're PlatformCALayerRemotes in just one place
Comment 5 Anders Carlsson 2013-10-22 17:54:34 PDT
Comment on attachment 214876 [details]
we'll assert that they're PlatformCALayerRemotes in just one place

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

> Source/WebKit2/WebProcess/WebPage/mac/PlatformCALayerRemote.cpp:154
> +    PlatformCALayerList& siblings = m_children;

I don't think you need to use this local variable. Also, it is misnamed.
Comment 6 Tim Horton 2013-10-23 01:23:52 PDT
http://trac.webkit.org/changeset/157852