[webkit-reviews] review granted: [Bug 130335] WKThumbnailView should support snapshots : [Attachment 226921] rebase

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted Tim Horton
<thorton at apple.com>'s request for review:
Bug 130335: WKThumbnailView should support snapshots
https://bugs.webkit.org/show_bug.cgi?id=130335

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

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=226921&action=review


> Source/WebKit2/ChangeLog:22
> +	   If we're using snapshots, and haven't already dispatched a async
snapshot request,

an async

> Source/WebKit2/UIProcess/API/Cocoa/WKThumbnailView.mm:107
> +    _pendingSnapshot = YES;

_pendingSnapshot is a confusing variable name. Is it the pending snapshot, that
a snapshot is pending, or that that this view is pending being snapshat?

> Source/WebKit2/UIProcess/API/Cocoa/WKThumbnailView.mm:116
> +	   [thumbnailView _setPendingSnapshot:NO];
> +	   [[thumbnailView layer] setSublayers:@[ ]];
> +	   [[thumbnailView layer] setContents:(id)cgImage.get()];

Why doesn't this just call a single method on the thumbnail view?

> Source/WebKit2/UIProcess/API/mac/WKView.mm:2503
> +    _data->_currentRootLayer = rootLayer;

What is the "current" supposed to imply?

> Source/WebKit2/WebProcess/WebPage/DrawingArea.h:110
> +    virtual WebCore::TransformationMatrix transform() const;

This name is rather generic. What does the transform do?