The approach in r170035 is still not quite correct; there is a window where endSwipeGesture will grab the wrong ID, and I'm not sure it's trivially reconcilable. Instead, let's implement transaction callbacks (one of my alternate solutions to that problem); the UI process will register a callback in endSwipeGesture, just after goToBackForwardItem; it will send the callback ID to the Web process, which will stuff it in the next transaction (and schedule one if needed). Then, upon receipt of the transaction, the UI process will perform the callbacks included in that transaction. This ensures that a) we always get a commit some time soon after endSwipeGesture that contains our callback b) it's always in-order with the goToBackForwardItem: message. This seems to work great for swiping between very stable pages (which don't commit often), very unstable pages (which commit very frequently), and a mixture of the two, and should be generally extendable to solve other similar synchronization problems as they come up. <rdar://problem/17496803>
Created attachment 234192 [details] patch
Comment on attachment 234192 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=234192&action=review > Source/WebKit2/UIProcess/mac/RemoteLayerTreeDrawingAreaProxy.h:62 > + virtual void installTransactionCallback(PassRefPtr<VoidCallback>) override; This should take a std::function.
(In reply to comment #2) > (From update of attachment 234192 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=234192&action=review > > > Source/WebKit2/UIProcess/mac/RemoteLayerTreeDrawingAreaProxy.h:62 > > + virtual void installTransactionCallback(PassRefPtr<VoidCallback>) override; > > This should take a std::function. And construct a VoidCallback? Or not use that mechanism at all?
Created attachment 234201 [details] weinig's change
Created attachment 234202 [details] actually different patch
Attachment 234202 [details] did not pass style-queue: ERROR: Source/WebKit2/UIProcess/mac/RemoteLayerTreeDrawingAreaProxy.mm:343: Code inside a namespace should not be indented. [whitespace/indent] [4] ERROR: Source/WebKit2/UIProcess/mac/RemoteLayerTreeDrawingAreaProxy.mm:343: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit2/UIProcess/mac/RemoteLayerTreeDrawingAreaProxy.h:62: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit2/UIProcess/DrawingAreaProxy.h:91: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 4 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 234202 [details] actually different patch View in context: https://bugs.webkit.org/attachment.cgi?id=234202&action=review Too many confusing uint64_ts. > Source/WebKit2/Shared/mac/RemoteLayerTreeTransaction.h:226 > + Vector<uint64_t> m_callbackIDs; Typedef for the uint64_t? Is this a pageID or a transactionID or something else? If a pageID, this member should say m_pageCallbacks or something. > Source/WebKit2/UIProcess/ios/ViewGestureControllerIOS.mm:70 > + static NeverDestroyed<HashMap<uint64_t, WebKit::ViewGestureController*>> viewGestureControllers; Would be nice if we had a pageID typedef.
Created attachment 234293 [details] patch I'm not going to typedef PageID in this patch! I did add TransactionCallbackID typedef, and renamed some things.
Attachment 234293 [details] did not pass style-queue: ERROR: Source/WebKit2/UIProcess/mac/RemoteLayerTreeDrawingAreaProxy.mm:343: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit2/UIProcess/mac/RemoteLayerTreeDrawingAreaProxy.h:62: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit2/UIProcess/DrawingAreaProxy.h:91: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 3 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 234293 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=234293&action=review > Source/WebKit2/Shared/mac/RemoteLayerTreeTransaction.h:213 > + typedef uint64_t TransactionCallbackID; Very nice. > Source/WebKit2/UIProcess/ios/ViewGestureControllerIOS.mm:68 > +static HashMap<uint64_t, WebKit::ViewGestureController*>& allViewGestureControllers() Please add a comment saying that the uint64_t is a pageID or rename this to viewGestureControllersForAllPages() > Source/WebKit2/UIProcess/ios/ViewGestureControllerIOS.mm:263 > + if (gestureControllerIter == allViewGestureControllers().end()) > + return; > + gestureControllerIter->value->willCommitPostSwipeTransitionLayerTree(error == CallbackBase::Error::None); Would slightly prefer this as if (gestureControllerIter != allViewGestureControllers().end()) gestureControllerIter->value->willCommitPostSwipeTransitionLayerTree(error == CallbackBase::Error::None); > Source/WebKit2/UIProcess/mac/RemoteLayerTreeDrawingAreaProxy.mm:137 > + if (callback) if (auto callback = m_callbacks.take<VoidCallback>(callbackID)) ?
http://trac.webkit.org/changeset/170761