Bug 132417

Summary: [iOS WebKit2] Swipe snapshots should be taken asynchronously
Product: WebKit Reporter: Tim Horton <thorton>
Component: WebKit2Assignee: Tim Horton <thorton>
Status: RESOLVED FIXED    
Severity: Normal CC: aestes, andersca, commit-queue, psolanki, sam, simon.fraser
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
patch andersca: review+

Description Tim Horton 2014-04-30 17:57:57 PDT
<rdar://problem/16535921>
Comment 1 Tim Horton 2014-04-30 18:31:23 PDT
Created attachment 230552 [details]
patch
Comment 2 WebKit Commit Bot 2014-04-30 18:32:58 PDT
Attachment 230552 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/mac/ViewSnapshotStore.mm:73:  Missing space before {  [whitespace/braces] [5]
Total errors found: 1 in 20 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Anders Carlsson 2014-05-02 12:16:45 PDT
Comment on attachment 230552 [details]
patch

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

> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:495
> +- (void)_takeViewSnapshot:(WebKit::ViewSnapshot&)snapshot

I think this should return a ViewSnapshot instead.

> Source/WebKit2/UIProcess/API/mac/WKView.mm:3040
> +- (void)_takeViewSnapshot:(ViewSnapshot&)snapshot

I think this should return a ViewSnapshot.

> Source/WebKit2/UIProcess/API/mac/WKView.mm:3074
> +    RetainPtr<CGImageRef> croppedSnapshotImage = adoptCF(CGImageCreateWithImageInRect(windowSnapshotImage.get(), NSRectToCGRect([window convertRectToBacking:croppedImageRect])));

auto?

> Source/WebKit2/UIProcess/API/mac/WKViewInternal.h:41
> +struct KeypressCommand;

I think structs should come after the classes.

> Source/WebKit2/UIProcess/WebMemoryPressureHandler.cpp:40
> +    NeverDestroyed<WebMemoryPressureHandler> memoryPressureHandler;

This should be static.

> Source/WebKit2/UIProcess/WebMemoryPressureHandler.cpp:48
> +WebMemoryPressureHandler::WebMemoryPressureHandler()
> +{
> +    memoryPressureHandler().setLowMemoryHandler(handleLowMemory);
> +    memoryPressureHandler().install();
> +}

How does this interact with WebKit1 and coexistence on iOS?

> Source/WebKit2/UIProcess/WebMemoryPressureHandler.h:33
> +    static WebMemoryPressureHandler& ensureSharedHandler();

I think this should just be called shared().

> Source/WebKit2/UIProcess/WebMemoryPressureHandler.h:37
> +    static void handleLowMemory(bool);

This can probably be a lambda.

> Source/WebKit2/UIProcess/mac/ViewSnapshotStore.h:48
> +    uint32_t slot;

I think this should be called slotID. If you want you can just assign = 0 here and not bother with the constructor.
Comment 4 Tim Horton 2014-05-05 00:38:43 PDT
http://trac.webkit.org/changeset/168260
Comment 5 Andy Estes 2014-05-05 09:31:36 PDT
Build fixes landed in r168276 and r168289.