Refactor ViewGestureController swipe snapshot removal to be more platform-independent
Created attachment 259106 [details] Patch
Mac is good; still have to test iOS.
Logging looks like this: Swipe Snapshot Removal (0.00 ms) - start Swipe Snapshot Removal (0.14 ms) - (re)started watchdog timer for 5 seconds Swipe Snapshot Removal (828.31 ms) - outstanding event occurred: VisuallyNonEmptyLayout Swipe Snapshot Removal (828.45 ms) - deferring removal; had oustanding events: RenderTreeSizeThreshold MainFrameLoad SubresourceLoads Swipe Snapshot Removal (828.51 ms) - (re)started watchdog timer for 3 seconds Swipe Snapshot Removal (844.20 ms) - outstanding event occurred: RenderTreeSizeThreshold Swipe Snapshot Removal (844.34 ms) - deferring removal; had oustanding events: MainFrameLoad SubresourceLoads Swipe Snapshot Removal (2046.35 ms) - outstanding event occurred: MainFrameLoad Swipe Snapshot Removal (2046.46 ms) - deferring removal; had oustanding events: SubresourceLoads Swipe Snapshot Removal (2046.52 ms) - outstanding event occurred: SubresourceLoads Swipe Snapshot Removal (2046.59 ms) - removed snapshot Swipe Snapshot Removal (0.00 ms) - start Swipe Snapshot Removal (0.12 ms) - (re)started watchdog timer for 5 seconds Swipe Snapshot Removal (86.57 ms) - outstanding event occurred: MainFrameLoad Swipe Snapshot Removal (86.68 ms) - deferring removal; had oustanding events: VisuallyNonEmptyLayout RenderTreeSizeThreshold SubresourceLoads Swipe Snapshot Removal (86.74 ms) - wait for event cancelled: VisuallyNonEmptyLayout Swipe Snapshot Removal (86.80 ms) - deferring removal; had oustanding events: RenderTreeSizeThreshold SubresourceLoads Swipe Snapshot Removal (86.86 ms) - outstanding event occurred: SubresourceLoads Swipe Snapshot Removal (86.91 ms) - deferring removal; had oustanding events: RenderTreeSizeThreshold Swipe Snapshot Removal (270.75 ms) - outstanding event occurred: RenderTreeSizeThreshold Swipe Snapshot Removal (270.91 ms) - removed snapshot
Created attachment 259108 [details] Patch
Created attachment 259109 [details] Patch
Comment on attachment 259109 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=259109&action=review > Source/WebKit2/UIProcess/ViewGestureController.cpp:51 > +static HashMap<uint64_t, WebKit::ViewGestureController*>& viewGestureControllersForAllPages() > +{ > + // The key in this map is the associated page ID. > + static NeverDestroyed<HashMap<uint64_t, WebKit::ViewGestureController*>> viewGestureControllers; > + return viewGestureControllers.get(); > +} Can this go inside the namespace WebKit block? > Source/WebKit2/UIProcess/ViewGestureController.cpp:151 > +String ViewGestureController::SnapshotRemovalTracker::eventsDescription(ViewGestureController::SnapshotRemovalTracker::Events event) This can be a static function, perhaps even a file-static function. > Source/WebKit2/UIProcess/ViewGestureController.cpp:153 > + String description; StringBuilder? > Source/WebKit2/UIProcess/ViewGestureController.cpp:238 > +bool ViewGestureController::SnapshotRemovalTracker::cancelOutstandingEvent(Events event) > +{ > + ASSERT(hasOneBitSet(event)); > + > + if (!(m_outstandingEvents & event)) > + return false; > + > + log("wait for event cancelled: " + eventsDescription(event)); > + > + m_outstandingEvents &= ~event; > + > + fireRemovalCallbackIfPossible(); > + return true; > +} Strange to have two functions that differ only in the message they log. > Source/WebKit2/UIProcess/ViewGestureController.cpp:256 > + log("removed snapshot"); “removing”? “will remove”? log after making the callback? > Source/WebKit2/UIProcess/mac/ViewGestureController.h:170 > + void log(String); const String&?
(In reply to comment #6) > Comment on attachment 259109 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=259109&action=review > > > Source/WebKit2/UIProcess/ViewGestureController.cpp:151 > > +String ViewGestureController::SnapshotRemovalTracker::eventsDescription(ViewGestureController::SnapshotRemovalTracker::Events event) > > This can be a static function, perhaps even a file-static function. It can't be file static because SnapshotRemovalTracker is private. But static, yes. And everything else, yes. Thanks!
(In reply to comment #6) > Comment on attachment 259109 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=259109&action=review > > Strange to have two functions that differ only in the message they log. I really want to keep separate function names for clarity at callsites, but I refactored it so the majority of the code is only in one place.
http://trac.webkit.org/changeset/188521