Bug 127438 – WebKit2 View Gestures (Swipe): Add a simple cache of view snapshots

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Tim Horton]

This is a cache of bitmap snapshots of the WKView, for use in the forthcoming fluid swipe implementation. For now, it just stores all snapshots forever.

https://bugs.webkit.org/show_bug.cgi?id=127389 and https://bugs.webkit.org/show_bug.cgi?id=127390 track improvements.

[Tim Horton]

Created attachment 221887 [details]
patch

[Tim Horton]

Errm, I need a way to make us not take snapshots if swiping isn't enabled.

[Tim Horton]

Created attachment 221897 [details]
patch disabled by default

[Sam Weinig]

Comment on attachment 221897 [details]
patch disabled by default

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

> Source/WebKit2/UIProcess/mac/ViewSnapshotStore.h:48
> +    CGImageRef getSnapshotAndRenderTreeSize(WebBackForwardListItem*, uint64_t& renderTreeSize);

This should return a std::pair<RetainPtr<CGImageRef>, uint64_t>

> Source/WebKit2/UIProcess/mac/ViewSnapshotStore.mm:76
> +    WKView *wkView = webPageProxy.wkView();

This class should not know about WKView! Instead, takeSnapshot() should really do all its work through the PageClient interface.  Is there a WebCore image type we could use for this kind of thing instead of CGImageRef directly?

[Tim Horton]

(In reply to comment #4)
> (From update of attachment 221897 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=221897&action=review
> 
> > Source/WebKit2/UIProcess/mac/ViewSnapshotStore.h:48
> > +    CGImageRef getSnapshotAndRenderTreeSize(WebBackForwardListItem*, uint64_t& renderTreeSize);
> 
> This should return a std::pair<RetainPtr<CGImageRef>, uint64_t>

So much better.

> > Source/WebKit2/UIProcess/mac/ViewSnapshotStore.mm:76
> > +    WKView *wkView = webPageProxy.wkView();
> 
> This class should not know about WKView! Instead, takeSnapshot() should really do all its work through the PageClient interface.

OK.

> Is there a WebCore image type we could use for this kind of thing instead of CGImageRef directly?

I didn't want to get too locked into something like that until we figure out the purgeability story.

[Sam Weinig]

> > Is there a WebCore image type we could use for this kind of thing instead of CGImageRef directly?
> 
> I didn't want to get too locked into something like that until we figure out the purgeability story.

Ok.

[Anders Carlsson]

(In reply to comment #5)
> > 
> > This class should not know about WKView! Instead, takeSnapshot() should really do all its work through the PageClient interface.
> 

For classes that are Mac only I see no problem exposing the view directly. Maybe this is not a Mac only class though.

[Sam Weinig]

(In reply to comment #7)
> (In reply to comment #5)
> > > 
> > > This class should not know about WKView! Instead, takeSnapshot() should really do all its work through the PageClient interface.
> > 
> 
> For classes that are Mac only I see no problem exposing the view directly. Maybe this is not a Mac only class though.

I have been trying to keep a division still.  If something wants to know about the WKView, it should be in UIProcess/API/Cocoa or UIProcess/API/Mac.

[Anders Carlsson]

(In reply to comment #8)
> (In reply to comment #7)
> > (In reply to comment #5)
> > > > 
> > > > This class should not know about WKView! Instead, takeSnapshot() should really do all its work through the PageClient interface.
> > > 
> > 
> > For classes that are Mac only I see no problem exposing the view directly. Maybe this is not a Mac only class though.
> 
> I have been trying to keep a division still.  If something wants to know about the WKView, it should be in UIProcess/API/Cocoa or UIProcess/API/Mac.

And that's why PageClient is a clusterfuck of random unrelated functions...

[Tim Horton]

Created attachment 222054 [details]
patch

[Tim Horton]

http://trac.webkit.org/changeset/162710