Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(8)

Issue 100245: Make forward/backward navigation work even when redirection is involved. (Closed)

Created:
9 years, 8 months ago by yuzo
Modified:
7 years, 8 months ago
Reviewers:
brettw, M-A Ruel
CC:
chromium-reviews_googlegroups.com
Visibility:
Public.

Description

This is the successor to http://codereview.chromium.org/67150 Make forward/backward navigation work even when redirection is involved. Currently, Chrome tries to go back to the page immediately before the current one. This doesn't work if the current page was visited by redirection; redirection just occurs again. With this change, Chrome first tries to find the redirection source of the current page and then to go back to the page before the source. BUG=9663, 10531 Tested: unit_tests, ui_tests, manually. Committed as revision 15950.

Patch Set 1 #

Total comments: 4

Patch Set 2 : '' #

Patch Set 3 : '' #

Patch Set 4 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+244 lines, -115 lines) Patch
M chrome/browser/tab_contents/navigation_controller.h View 1 2 3 1 chunk +3 lines, -3 lines 0 comments Download
M chrome/browser/tab_contents/navigation_controller.cc View 1 2 3 6 chunks +18 lines, -10 lines 0 comments Download
M chrome/test/data/History/HistoryHelper.js View 1 2 3 1 chunk +3 lines, -0 lines 0 comments Download
D chrome/test/data/History/history_length_test1.html View 1 2 3 1 chunk +0 lines, -66 lines 0 comments Download
D chrome/test/data/History/history_length_test2.html View 1 2 3 1 chunk +0 lines, -32 lines 0 comments Download
A chrome/test/data/History/history_length_test_page_1.html View 1 2 1 chunk +46 lines, -0 lines 0 comments Download
A chrome/test/data/History/history_length_test_page_2.html View 1 2 1 chunk +37 lines, -0 lines 0 comments Download
A chrome/test/data/History/history_length_test_page_3.html View 1 2 1 chunk +58 lines, -0 lines 0 comments Download
A chrome/test/data/History/history_length_test_page_4.html View 1 2 1 chunk +48 lines, -0 lines 0 comments Download
M chrome/test/ui/history_uitest.cc View 1 2 3 1 chunk +31 lines, -4 lines 0 comments Download

Messages

Total messages: 9 (0 generated)
yuzo
Hi, Brett, Can you take another look? I've addressed your comments (for 67150) and added ...
9 years, 8 months ago (2009-05-01 05:07:59 UTC) #1
brettw
LGTM with these two changes: http://codereview.chromium.org/100245/diff/1/10 File chrome/browser/tab_contents/navigation_controller.cc (right): http://codereview.chromium.org/100245/diff/1/10#newcode614 Line 614: InsertEntry(new_entry); If you ...
9 years, 8 months ago (2009-05-04 08:27:54 UTC) #2
yuzo
Hi, Thank you for the review. Can you take yet another look? http://codereview.chromium.org/100245/diff/1/10 File chrome/browser/tab_contents/navigation_controller.cc ...
9 years, 8 months ago (2009-05-07 03:43:32 UTC) #3
brettw
LGTM
9 years, 8 months ago (2009-05-07 10:06:40 UTC) #4
yuzo
Thank you. I've uploaded the html files minutes ago, to make them qualify as XHTML ...
9 years, 8 months ago (2009-05-07 10:09:31 UTC) #5
M-A Ruel
Compilation failure. See http://build.chromium.org/buildbot/try-server/builders/linux/builds/1775 http://build.chromium.org/buildbot/try-server/builders/mac/builds/1796 Visit http://go/ChromeTryServer for instructions.
9 years, 8 months ago (2009-05-07 17:42:59 UTC) #6
M-A Ruel
On 2009/05/07 17:42:59, M-A wrote: > Compilation failure. See > http://build.chromium.org/buildbot/try-server/builders/linux/builds/1775 > http://build.chromium.org/buildbot/try-server/builders/mac/builds/1796 > > ...
9 years, 8 months ago (2009-05-07 18:18:53 UTC) #7
yuzo
Thank you very much for taking care of this, and sorry for the patch failure. ...
9 years, 8 months ago (2009-05-08 04:02:31 UTC) #8
yuzo
9 years, 8 months ago (2009-05-08 04:04:41 UTC) #9
I've also gcl upload'ed the change.

Yuzo

On 2009/05/08 04:02:31, yuzo wrote:
> Thank you very much for taking care of this,
> and sorry for the patch failure.
> 
> I've synced to the HEAD and confirmed that
> gcl try succeeds for all the three platforms.
> 
> Can you give it another try?
> 
> Yuzo
> 
> On 2009/05/07 18:18:53, M-A wrote:
> > On 2009/05/07 17:42:59, M-A wrote:
> > > Compilation failure. See
> > > http://build.chromium.org/buildbot/try-server/builders/linux/builds/1775
> > > http://build.chromium.org/buildbot/try-server/builders/mac/builds/1796
> > > 
> > > Visit http://go/ChromeTryServer for instructions.
> > 
> > 
> > In fact the patch hadn't applied cleanly, hence the compile failure. Please
> sync
> > and upload again. e.g.:
> > 
> > Hunk #4 FAILED at 747.
> > 1 out of 6 hunks FAILED -- saving rejects to file
> > chrome/browser/tab_contents/navigation_controller.cc.rej

Powered by Google App Engine
This is Rietveld 408576698