Summary: | [iOS WebKit2] Swipe snapshots should be taken asynchronously | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Tim Horton <thorton> | ||||
Component: | WebKit2 | Assignee: | 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
Tim Horton
2014-04-30 17:57:57 PDT
Created attachment 230552 [details]
patch
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 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. |