[webkit-reviews] review granted: [Bug 124325] Consolidate and expose Frame/Node/Selection screenshot capabilities : [Attachment 217138] v2.0 (appease style bot)

[bugzilla-daemon at webkit.org]

Sam Weinig <sam at webkit.org> has granted Brian Burg <burg at cs.washington.edu>'s
request for review:
Bug 124325: Consolidate and expose Frame/Node/Selection screenshot capabilities
https://bugs.webkit.org/show_bug.cgi?id=124325

Attachment 217138: v2.0 (appease style bot)
https://bugs.webkit.org/attachment.cgi?id=217138&action=review

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


Overall, looks great. r=me with the changes.

> Source/WebCore/page/FrameSnapshotting.h:44
> +enum {
> +    SnapshotOptionsExcludeSelectionHighlighting = 1 << 1,
> +    SnapshotOptionsInViewCoordinates = 1 << 2,
> +    SnapshotOptionsForceBlackText = 1 << 3,
> +};
> +typedef uint32_t SnapshotOptions;

This is C++, so this should probably use enum SnapshotOptions { ... } instead
of the typedef.

> Source/WebCore/page/FrameSnapshotting.h:47
> +OwnPtr<ImageBuffer> snapshotNode(Frame&, Node*);

This should take the Node by reference.

> Source/WebCore/page/FrameSnapshotting.h:48
> +OwnPtr<ImageBuffer> snapshotFrameRect(Frame&, const IntRect&,
SnapshotOptions = 0);
> +OwnPtr<ImageBuffer> snapshotNode(Frame&, Node*);
> +OwnPtr<ImageBuffer> snapshotSelection(Frame&, SnapshotOptions = 0);

These should return std::unique_ptr<ImageBuffer>