Bug 132417 – [iOS WebKit2] Swipe snapshots should be taken asynchronously

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

[Tim Horton]

<rdar://problem/16535921>

[Tim Horton]

Created attachment 230552 [details]
patch

[WebKit Commit Bot]

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.

[Anders Carlsson]

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.

[Tim Horton]

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

[Andy Estes]

Build fixes landed in r168276 and r168289.