Bug 48513 - [Chromium] History related tests REGRESSED after r70723
Summary: [Chromium] History related tests REGRESSED after r70723
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: History (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P1 Normal
Assignee: Mihai Parparita
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-10-28 04:57 PDT by Mikhail Naganov
Modified: 2010-10-28 16:21 PDT (History)
5 users (show)

See Also:


Attachments
Patch (14.50 KB, patch)
2010-10-28 15:40 PDT, Mihai Parparita
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mikhail Naganov 2010-10-28 04:57:15 PDT
A number of tests (see below) fail on Chromium, having the same problem: instead of reporting "http://127.0.0.1:8000/history" in Back Forward List, they report "chrome-back-forward://go/0"

E.g. for http/tests/history/redirect-301, expected is:


============== Back Forward List ==============
        http://127.0.0.1:8000/history/redirect-301.html  **nav target**
curr->  http://127.0.0.1:8000/history/resources/redirect-target.html#2  **nav target**
===============================================


Actual is:


============== Back Forward List ==============
        http://127.0.0.1:8000/history/redirect-301.html  **nav target**
curr->  chrome-back-forward://go/0
===============================================


I searched for "chrome-back-forward", and it originates from WebKit/chromium/src/BackForwardListClientImpl.cpp, having a FIXME. Probably, it's time to do it.

The full list of tests affected (see http://build.chromium.org/p/chromium.webkit/builders/Webkit%20Mac/builds/312/steps/webkit_tests/logs/stdio):

  fast/history/form-submit-in-frame-via-onclick.html = TEXT
  fast/history/form-submit-in-frame.html = TEXT
  fast/history/gesture-before-onload-form-submit.html = TEXT
  fast/history/gesture-before-onload-location-href.html = TEXT
  fast/history/redirect-via-iframe.html = TEXT
  fast/history/same-document-iframes-changing-fragment.html = TEXT
  fast/history/same-document-iframes-changing-pushstate.html = TEXT
  fast/loader/fragment-after-redirect-gets-back-entry.html = TEXT
  fast/loader/frame-location-change-not-added-to-history.html = TEXT
  fast/loader/frame-src-change-added-to-history.html = TEXT
  fast/loader/frame-src-change-not-added-to-history.html = TEXT
  fast/loader/stateobjects/pushstate-clears-forward-history.html = TEXT
  fast/loader/subframe-navigate-during-main-frame-load.html = TEXT
  http/tests/history/redirect-200-refresh-0-seconds.pl = TEXT
  http/tests/history/redirect-200-refresh-2-seconds.pl = TEXT
  http/tests/history/redirect-301.html = TEXT
  http/tests/history/redirect-302.html = TEXT
  http/tests/history/redirect-303.html = TEXT
  http/tests/history/redirect-307.html = TEXT
  http/tests/history/redirect-js-document-location-0-seconds.html = TEXT
  http/tests/history/redirect-js-document-location-2-seconds.html = TEXT
  http/tests/history/redirect-js-document-location-before-load.html = TEXT
  http/tests/history/redirect-js-form-submit-0-seconds.html = TEXT
  http/tests/history/redirect-js-form-submit-2-seconds.html = TEXT
  http/tests/history/redirect-js-form-submit-before-load.html = TEXT
  http/tests/history/redirect-js-location-0-seconds.html = TEXT
  http/tests/history/redirect-js-location-2-seconds.html = TEXT
  http/tests/history/redirect-js-location-assign-0-seconds.html = TEXT
  http/tests/history/redirect-js-location-assign-2-seconds.html = TEXT
  http/tests/history/redirect-js-location-assign-before-load.html = TEXT
  http/tests/history/redirect-js-location-before-load.html = TEXT
  http/tests/history/redirect-js-location-href-0-seconds.html = TEXT
  http/tests/history/redirect-js-location-href-2-seconds.html = TEXT
  http/tests/history/redirect-js-location-href-before-load.html = TEXT
  http/tests/history/redirect-js-location-replace-0-seconds.html = TEXT
  http/tests/history/redirect-js-location-replace-2-seconds.html = TEXT
  http/tests/history/redirect-js-location-replace-before-load.html = TEXT
  http/tests/history/redirect-meta-refresh-0-seconds.html = TEXT
  http/tests/history/redirect-meta-refresh-2-seconds.html = TEXT
  http/tests/navigation/anchor-basic.html = TEXT
  http/tests/navigation/anchor-goback.html = TEXT
  http/tests/navigation/anchor-subframeload.html = TEXT
  http/tests/navigation/back-to-slow-frame.html = TEXT
  http/tests/navigation/document-location-click-timeout.html = TEXT
  http/tests/navigation/document-location-click.html = TEXT
  http/tests/navigation/document-location-mouseover.html = TEXT
  http/tests/navigation/document-location-onload.html = TEXT
  http/tests/navigation/document-location-script.html = TEXT
  http/tests/navigation/dynamic-iframe-dynamic-form-back-entry.html = TEXT
  http/tests/navigation/error404-basic.html = TEXT
  http/tests/navigation/error404-frames.html = TEXT
  http/tests/navigation/error404-goback.html = TEXT
  http/tests/navigation/error404-subframeload.html = TEXT
  http/tests/navigation/javascriptlink-basic.html = TEXT
  http/tests/navigation/javascriptlink-frames.html = TEXT
  http/tests/navigation/javascriptlink-goback.html = TEXT
  http/tests/navigation/javascriptlink-subframeload.html = TEXT
  http/tests/navigation/location-assign-adds-history-item.html = TEXT
  http/tests/navigation/location-href-set-adds-history-item.html = TEXT
  http/tests/navigation/location-replace-adds-history-item.html = TEXT
  http/tests/navigation/location-set-adds-history-item.html = TEXT
  http/tests/navigation/lockedhistory-iframe.html = TEXT
  http/tests/navigation/metaredirect-basic.html = TEXT
  http/tests/navigation/metaredirect-frames.html = TEXT
  http/tests/navigation/metaredirect-goback.html = TEXT
  http/tests/navigation/metaredirect-subframeload.html = TEXT
  http/tests/navigation/multiple-back-forward-entries.html = TEXT
  http/tests/navigation/new-window-redirect-history.html = TEXT
  http/tests/navigation/onload-navigation-iframe-2.html = TEXT
  http/tests/navigation/onload-navigation-iframe-timeout.html = TEXT
  http/tests/navigation/onload-navigation-iframe.html = TEXT
  http/tests/navigation/parsed-iframe-dynamic-form-back-entry.html = TEXT
  http/tests/navigation/post-basic.html = TEXT
  http/tests/navigation/post-frames.html = TEXT
  http/tests/navigation/post-goback-same-url.html = TEXT
  http/tests/navigation/post-goback1.html = TEXT
  http/tests/navigation/post-goback2.html = TEXT
  http/tests/navigation/postredirect-basic.html = TEXT
  http/tests/navigation/postredirect-frames.html = TEXT
  http/tests/navigation/postredirect-goback1.html = TEXT
  http/tests/navigation/postredirect-goback2.html = TEXT
  http/tests/navigation/postredirect-reload.html = TEXT
  http/tests/navigation/redirect-cycle.html = TEXT
  http/tests/navigation/redirect-load-no-form-restoration.html = TEXT
  http/tests/navigation/redirect302-basic.html = TEXT
  http/tests/navigation/redirect302-frames.html = TEXT
  http/tests/navigation/redirect302-goback.html = TEXT
  http/tests/navigation/redirect302-metaredirect.html = TEXT
  http/tests/navigation/redirect302-subframeload.html = TEXT
  http/tests/navigation/relativeanchor-basic.html = TEXT
  http/tests/navigation/relativeanchor-frames.html = TEXT
  http/tests/navigation/relativeanchor-goback.html = TEXT
  http/tests/navigation/restore-form-state-https.html = TEXT
  http/tests/navigation/slowmetaredirect-basic.html = TEXT
  http/tests/navigation/slowtimerredirect-basic.html = TEXT
  http/tests/navigation/success200-basic.html = TEXT
  http/tests/navigation/success200-frames-loadsame.html = TEXT
  http/tests/navigation/success200-frames.html = TEXT
  http/tests/navigation/success200-goback.html = TEXT
  http/tests/navigation/success200-loadsame.html = TEXT
  http/tests/navigation/success200-reload.html = TEXT
  http/tests/navigation/success200-subframeload.html = TEXT
  http/tests/navigation/timerredirect-basic.html = TEXT
  http/tests/navigation/timerredirect-frames.html = TEXT
  http/tests/navigation/timerredirect-goback.html = TEXT
  http/tests/navigation/timerredirect-subframeload.html = TEXT
  http/tests/navigation/window-open-adds-history-item.html = TEXT
  http/tests/navigation/window-open-adds-history-item2.html = TEXT
Comment 1 Darin Adler 2010-10-28 08:13:26 PDT
I’ll try to find some time to look at this today, but if someone else can fix it before I get to it, that would be great. I have limited experience with Chrome’s back/forward implementation.
Comment 2 Mihai Parparita 2010-10-28 10:46:07 PDT
I'll take a stab at this.
Comment 3 Darin Adler 2010-10-28 12:06:59 PDT
On guess is that this has something to do with the backItem, currentItem, and forwardItem functions. Perhaps itemAtIndex doesn’t give the same result or there was some side effect from the old version of the functions.

Very little else changed in WebCore.
Comment 4 Mihai Parparita 2010-10-28 15:08:53 PDT
(In reply to comment #3)
> On guess is that this has something to do with the backItem, currentItem, and forwardItem functions. Perhaps itemAtIndex doesn’t give the same result or there was some side effect from the old version of the functions.

Yes, BackForwardListChromium has a custom implementation of currentItem(). Either changing it back to virtual in BackForwardList.h or making itemAtIndex(0) in the Chromium version be the same as currentItem should fix things.
Comment 5 Darin Adler 2010-10-28 15:11:06 PDT
(In reply to comment #4)
> making itemAtIndex(0) in the Chromium version be the same as currentItem should fix things.

That’s what we should do. In the future we probably don’t want a separate currentItem function. Unless we get rid of itemAtIndex altogether, we want to make sure that itemAtIndex(0) works properly.
Comment 6 Mihai Parparita 2010-10-28 15:40:14 PDT
Created attachment 72249 [details]
Patch
Comment 7 Mihai Parparita 2010-10-28 15:40:49 PDT
(In reply to comment #5)
> That’s what we should do. In the future we probably don’t want a separate currentItem function. Unless we get rid of itemAtIndex altogether, we want to make sure that itemAtIndex(0) works properly.

Makes sense. Uploaded patch does that.
Comment 8 Darin Adler 2010-10-28 15:43:19 PDT
Comment on attachment 72249 [details]
Patch

Looks good. Thanks!
Comment 9 WebKit Commit Bot 2010-10-28 16:21:03 PDT
Comment on attachment 72249 [details]
Patch

Clearing flags on attachment: 72249

Committed r70823: <http://trac.webkit.org/changeset/70823>
Comment 10 WebKit Commit Bot 2010-10-28 16:21:09 PDT
All reviewed patches have been landed.  Closing bug.