[webkit-reviews] review granted: [Bug 129508] Make UIViews for compositing layers in the UI process on iOS : [Attachment 225492] Patch

[bugzilla-daemon at webkit.org]

Sam Weinig <sam at webkit.org> has granted Simon Fraser (smfr)
<simon.fraser at apple.com>'s request for review:
Bug 129508: Make UIViews for compositing layers in the UI process on iOS
https://bugs.webkit.org/show_bug.cgi?id=129508

Attachment 225492: Patch
https://bugs.webkit.org/attachment.cgi?id=225492&action=review

------- Additional Comments from Sam Weinig <sam at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=225492&action=review


> Source/WebKit2/Shared/mac/RemoteLayerTreePropertyApplier.mm:41
> +- (void)setSubviews:(NSArray *)subviews;

This should be _web_setSubviews: (Says the council of 2).

> Source/WebKit2/Shared/mac/RemoteLayerTreePropertyApplier.mm:54
> + at implementation UIView (WKUIViewUtilities)
> +- (void)setSubviews:(NSArray *)subviews
> +{
> +    for (UIView* subview in self.subviews)
> +	   [subview removeFromSuperview];
> +
> +    for (UIView* view in subviews)
> +	   [self addSubview:view];
> +}
> + at end
> +#endif

Please add leading/trailing newlines.

> Source/WebKit2/UIProcess/ios/RemoteLayerTreeHostIOS.mm:109
> +    // FIXME: do through the view.
> +    [[layerOrView layer] web_disableAllActions];

Capital D in do!

> Source/WebKit2/UIProcess/ios/RemoteScrollingCoordinatorProxyIOS.mm:48
> +    for (auto& it : stateTree.nodeMap()) {
> +	   ScrollingStateNode* currNode = it.value;

You can do for (auto& currentNode : stateTree.nodeMap().values())

> Source/WebKit2/UIProcess/ios/WebVideoFullscreenManagerProxy.mm:117
> +}
> +
> +    
> +} // namespace WebKit

Too many lines!!!!!!!