Bug 133885 – [iOS][wk2] Swiping back briefly shows the previous page before loading the new one

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Tim Horton]

Sometimes we will get a layer tree commit from the Web Process -> UI process with layers (and a render tree size) from the previous page AFTER the UI process has started the navigation to the new page (because layer tree flush is very asynchronous). This will cause ViewGestureControllerIOS to think that the render tree size limit has been hit, and remove the snapshot, revealing the old page.

Since we already synchronize layer tree commits between processes, we just need a way to identify commits by ID and know which commit current actions will be applied to.

[Tim Horton]

Created attachment 233086 [details]
patch

[Tim Horton]

Created attachment 233087 [details]
patch

[Tim Horton]

Transaction ID is reset upon crash because we tear down the drawing area (and make a new Web process). I've tested that this works!

[Simon Fraser (smfr)]

Comment on attachment 233087 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=233087&action=review

> Source/WebKit2/Shared/mac/RemoteLayerTreeTransaction.h:209
> +    void setTransactionID(uint64_t transactionID) { m_transactionID = transactionID; }

Should this be provided to the constructor?

> Source/WebKit2/UIProcess/DrawingAreaProxy.h:89
> +    virtual uint64_t nextTransactionID() const { ASSERT_NOT_REACHED(); return 0; }

Weird to have this one.

> Source/WebKit2/UIProcess/ios/ViewGestureControllerIOS.mm:231
> +    m_targetTransactionID = m_webPageProxy.drawingArea()->nextTransactionID();

"target" doesn't communicate what you're going to use this transaction ID for.

> Source/WebKit2/UIProcess/ios/ViewGestureControllerIOS.mm:253
> +    if (m_targetRenderTreeSize && renderTreeSize > m_targetRenderTreeSize && m_webPageProxy.drawingArea()->currentTransactionID() >= m_targetTransactionID)

I want to see a call here like "isCurrentTransaction()" or "isStaleTransaction" or something.

> Source/WebKit2/UIProcess/mac/RemoteLayerTreeDrawingAreaProxy.h:80
> +    virtual uint64_t nextTransactionID() const override { return m_currentTransactionID + 1; }

Remove?

> Source/WebKit2/UIProcess/mac/RemoteLayerTreeDrawingAreaProxy.h:94
> +    uint64_t m_currentTransactionID;

I feel like this should be lastVisibleTransactionID or something.

> Source/WebKit2/UIProcess/mac/RemoteLayerTreeDrawingAreaProxy.mm:312
> +    m_currentTransactionID++;

Rather than just increment this count, I feel like it should copy the ID obtained from the last transaction somehow.

> Source/WebKit2/WebProcess/WebPage/mac/RemoteLayerTreeDrawingArea.mm:282
> +    layerTransaction.setTransactionID(m_currentTransactionID++);

getNextTransactionID()?

[Tim Horton]

Created attachment 233199 [details]
patch

[Tim Horton]

http://trac.webkit.org/changeset/170035

[Radar WebKit Bug Importer]

<rdar://problem/17416499>