Bug 130335 - WKThumbnailView should support snapshots
Summary: WKThumbnailView should support snapshots
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: InRadar
Depends on:
Blocks:
 
Reported: 2014-03-17 07:33 PDT by Tim Horton
Modified: 2014-03-17 13:09 PDT (History)
3 users (show)

See Also:

Attachments
patch (18.81 KB, patch)
2014-03-17 07:44 PDT, Tim Horton 		no flags Details | Formatted Diff | Diff
rebase (17.10 KB, patch)
2014-03-17 08:16 PDT, Tim Horton 		simon.fraser: review+
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Tim Horton 2014-03-17 07:33:26 PDT 
In some cases, clients of WKThumbnailView might want to use snapshots of the web content for performance reasons.

<rdar://problem/16255139>
Comment 1 Tim Horton 2014-03-17 07:44:16 PDT 
Created attachment 226916 [details]
patch
Comment 2 Tim Horton 2014-03-17 07:54:50 PDT 
Comment on attachment 226916 [details]
patch

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

> Source/WebCore/WebCore.exp.in:791
> +__ZN7WebCore19MediaSessionManager9addClientEPNS_25MediaSessionManagerClientE

hmm, sort-export-file seems to have made a mistake
Comment 3 Tim Horton 2014-03-17 08:03:37 PDT 
(In reply to comment #2)
> (From update of attachment 226916 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=226916&action=review
> 
> > Source/WebCore/WebCore.exp.in:791
> > +__ZN7WebCore19MediaSessionManager9addClientEPNS_25MediaSessionManagerClientE
> 
> hmm, sort-export-file seems to have made a mistake

I guess it just sorts alphabetically :|
Comment 4 Tim Horton 2014-03-17 08:16:11 PDT 
Created attachment 226921 [details]
rebase
Comment 5 Simon Fraser (smfr) 2014-03-17 10:27:55 PDT 
Comment on attachment 226921 [details]
rebase

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?
Comment 6 mitz 2014-03-17 10:41:07 PDT 
Comment on attachment 226921 [details]
rebase

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

> Source/WebKit2/UIProcess/API/Cocoa/WKThumbnailView.h:42
> +@property (nonatomic) BOOL useSnapshot;

Why not usesSnapshot?
Comment 7 Tim Horton 2014-03-17 13:09:08 PDT 
https://trac.webkit.org/r165748