Bug 138753

Summary: Wrong (off-by-1) navigation snapshots shown after a mix of gesture and button back/forward navigation
Product: WebKit Reporter: mitz
Component: WebKit2Assignee: mitz
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, japhet, thorton
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Record a a navigation snapshot before the current item changes in WebBackForwardList thorton: review+

Description mitz 2014-11-14 12:52:18 PST
<rdar://problem/18978497>

* STEPS TO REPRODUCE
1. In a new tab, navigate to <data:text/html,1>
2. In the same tab, navigate to <data:text/html,2>
3. In the same tab, navigate to <data:text/html,3>
4. Swipe to navigate back to 2
5. Swipe to navigate back to 1
6. Click or tap the Forward button to navigate forward to 2
7. Click or tap the Forward button to navigate forward to 3
8. Swipe back to 2

* RESULTS
In step 8, the 1 snapshot is revealed, instead of the 2 snapshot.
Comment 1 mitz 2014-11-14 12:54:25 PST
It looks like when using the button to navigate, by the time WebPageProxy::willChangeCurrentHistoryItemForMainFrame() is called, the current item has already changed. In particular, WebPageProxy::backForwardGoToItem() is called before WebPageProxy::willChangeCurrentHistoryItemForMainFrame().

This call ordering happens not only in the button navigation case, but also in the navigation gesture case, but in the latter we are protected by the early return in ViewSnapshotStore::recordSnapshot() as we are still showing a snapshot.

I think we can simply make it WebBackForwardList’s job to record a snapshot right before it changes what currentItem() would return.
Comment 2 mitz 2014-11-14 13:08:47 PST
Created attachment 241616 [details]
Record a a navigation snapshot before the current item changes in WebBackForwardList
Comment 3 mitz 2014-11-14 13:29:05 PST
Fixed in <http://trac.webkit.org/r176136>.