Make forward/backward navigation work even when redirection is involved.

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.

[yuzo, 2009-05-01 05:07:59]

Hi, Brett,

Can you take another look?

I've addressed your comments (for 67150) and added a ui test.
(If it looks good, please submit it.)

Yuzo

[brettw, 2009-05-04 08:27:54]

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 agree to use InsertOrReplace instead of Replace above, I think this
should also be changed to InsertOrReplace (false) to make the difference clear.

http://codereview.chromium.org/100245/diff/1/11
File chrome/browser/tab_contents/navigation_controller.h (right):

http://codereview.chromium.org/100245/diff/1/11#newcode433
Line 433: void ReplaceCurrentEntry(NavigationEntry* entry);
This function is private and only called once, so I think that the caller should
just use InsertOrReplace and you can delete this one.

[yuzo, 2009-05-07 03:43:32]

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 (right):

http://codereview.chromium.org/100245/diff/1/10#newcode614
Line 614: InsertEntry(new_entry);
On 2009/05/04 08:27:54, brettw wrote:
> If you agree to use InsertOrReplace instead of Replace above, I think this
> should also be changed to InsertOrReplace (false) to make the difference
clear.

Done.

http://codereview.chromium.org/100245/diff/1/11
File chrome/browser/tab_contents/navigation_controller.h (right):

http://codereview.chromium.org/100245/diff/1/11#newcode433
Line 433: void ReplaceCurrentEntry(NavigationEntry* entry);
On 2009/05/04 08:27:54, brettw wrote:
> This function is private and only called once, so I think that the caller
should
> just use InsertOrReplace and you can delete this one.

OK, I removed both InsertEntry and ReplaceCurrentEntry. Now only InsertOrReplace
is used.

[brettw, 2009-05-07 10:06:40]

LGTM

[yuzo, 2009-05-07 10:09:31]

Thank you.

I've uploaded the html files minutes ago, to make them
qualify as XHTML (xmlns, lower cased tags, etc.)

Please check if you have the latest patch.

Yuzo

On 2009/05/07 10:06:40, brettw wrote:
> LGTM

[M-A Ruel, 2009-05-07 17:42:59]

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.

[M-A Ruel, 2009-05-07 18:18:53]

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

[yuzo, 2009-05-08 04:02:31]

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

[yuzo, 2009-05-08 04:04:41]

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