RESOLVED FIXED 133364
[iOS][WK2] When a page does not finish rotation before the end of the animation, synchronize explicitly
https://bugs.webkit.org/show_bug.cgi?id=133364
Summary [iOS][WK2] When a page does not finish rotation before the end of the animati...
Benjamin Poulain
Reported 2014-05-28 16:43:39 PDT
[iOS][WK2] When a page does not finish rotation before the end of the animation, synchronize explicitely
Attachments
Patch (12.65 KB, patch)
2014-05-28 16:50 PDT, Benjamin Poulain
no flags
Patch (11.61 KB, patch)
2014-05-29 15:35 PDT, Benjamin Poulain
sam: review+
Benjamin Poulain
Comment 1 2014-05-28 16:50:54 PDT
Benjamin Poulain
Comment 2 2014-05-28 16:54:11 PDT
Tim Horton
Comment 3 2014-05-28 17:09:44 PDT
Comment on attachment 232221 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=232221&action=review > Source/WebKit2/ChangeLog:9 > + When a page that relayout on rotation does not respond before the end of the animation, we end up with maybe "does layout" instead of "relayout"? there's a mismatch here. > Source/WebKit2/ChangeLog:10 > + a completely inconistent state on the UIProcess (because it is unware of the new states). typo inconsistent > Source/WebKit2/ChangeLog:15 > + This patch force a synchronous synchronization whenever we finish the animation before we heard back redundant "synchronous synchronization" :D > Source/WebKit2/UIProcess/ios/WebPageProxyIOS.mm:249 > + WebCore::FloatPoint newScrollPosition; no WebCore:: > Source/WebKit2/UIProcess/ios/WebPageProxyIOS.mm:250 > + if (m_process->sendSync(Messages::WebPage::SynchronizeDynamicViewportUpdate(), Messages::WebPage::SynchronizeDynamicViewportUpdate::Reply(newScale, newScrollPosition), m_pageID)) please check this with Sam > Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:2149 > +void WebPage::synchronizeDynamicViewportUpdate(double& newTargetScale, WebCore::FloatPoint& newScrollPosition) no WebCore here either
Benjamin Poulain
Comment 4 2014-05-28 23:03:30 PDT
Comment on attachment 232221 [details] Patch I'll fix from the comments and see how ugly it would be to sync the update with the incoming frame.
Benjamin Poulain
Comment 5 2014-05-29 15:35:35 PDT
Sam Weinig
Comment 6 2014-05-30 15:16:18 PDT
Comment on attachment 232268 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=232268&action=review > Source/WebKit2/UIProcess/ios/WebPageProxyIOS.mm:257 > + if (m_dynamicViewportSizeUpdateInProgress) { > + m_dynamicViewportSizeUpdateInProgress = false; > + double newScale; > + FloatPoint newScrollPosition; > + if (m_process->sendSync(Messages::WebPage::SynchronizeDynamicViewportUpdate(), Messages::WebPage::SynchronizeDynamicViewportUpdate::Reply(newScale, newScrollPosition), m_pageID, std::chrono::seconds(2))) { > + m_pageClient.dynamicViewportUpdateChangedTarget(newScale, newScrollPosition); > + > + m_process->connection()->waitForAndDispatchImmediately<Messages::RemoteLayerTreeDrawingAreaProxy::CommitLayerTree>(m_pageID, std::chrono::seconds(1)); > + } > + } > +} I would add a comment (and bugzilla link?) about how we could do this without a sync message in the future using transactions.
Benjamin Poulain
Comment 7 2014-05-30 17:57:48 PDT
Note You need to log in before you can comment on or make changes to this bug.