Bug 133364 – [iOS][WK2] When a page does not finish rotation before the end of the animation, synchronize explicitly

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Benjamin Poulain]

[iOS][WK2] When a page does not finish rotation before the end of the animation, synchronize explicitely

[Benjamin Poulain]

Created attachment 232221 [details]
Patch

[Benjamin Poulain]

<rdar://problem/17026333>

[Tim Horton]

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 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]

Created attachment 232268 [details]
Patch

[Sam Weinig]

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]

Committed r169501: <http://trac.webkit.org/changeset/169501>