Ensure address bar is reset after a redirect turns into a download.

Ensure address bar is reset after a redirect turns into a download.

BUG=83031
TEST=NavigationControllerTest.LoadURL_RedirectAbortCancelsPending

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=88886

[Charlie Reis, 2011-06-04 00:40:12]

Here's an attempt to fix the issue in 83031.  It means we'll cancel the pending
entry if the user starts a new navigation during a slow one, but that seems
safer than leaving a stale URL showing if a redirect is aborted.  I'm open to
suggestions if there are better ways to tell whether the failed URL is a
redirect vs a new navigation.

[Charlie Reis, 2011-06-07 23:52:33]

I just got around to fixing a failure on the Windows try bot, so this is
plausible now if you agree with the approach.  Please let me know when you get
back from vacation.

Thanks!
Charlie

[brettw, 2011-06-12 12:44:56]

LGTM, I think this approach sounds fine.

http://codereview.chromium.org/6993037/diff/2001/content/browser/tab_contents...
File content/browser/tab_contents/navigation_controller_unittest.cc (right):

http://codereview.chromium.org/6993037/diff/2001/content/browser/tab_contents...
content/browser/tab_contents/navigation_controller_unittest.cc:485:
ViewHostMsg_DidRedirectProvisionalLoad(0, // routing_id
Style nit: be sure you have two spaces before end-of-line comments.

http://codereview.chromium.org/6993037/diff/2001/content/browser/tab_contents...
File content/browser/tab_contents/tab_contents.cc (right):

http://codereview.chromium.org/6993037/diff/2001/content/browser/tab_contents...
content/browser/tab_contents/tab_contents.cc:861: // navigation.  It's safer to
clear the pending entry in both cases than
Can you make more clear how this is wrong in the case of a new navigation? If
somebody tries to fix that bug, I want it to be clear we know that it's
happening in this case and why we need to do it.

[Charlie Reis, 2011-06-13 17:46:48]

http://codereview.chromium.org/6993037/diff/2001/content/browser/tab_contents...
File content/browser/tab_contents/navigation_controller_unittest.cc (right):

http://codereview.chromium.org/6993037/diff/2001/content/browser/tab_contents...
content/browser/tab_contents/navigation_controller_unittest.cc:485:
ViewHostMsg_DidRedirectProvisionalLoad(0, // routing_id
On 2011/06/12 12:44:56, brettw wrote:
> Style nit: be sure you have two spaces before end-of-line comments.

Done.

http://codereview.chromium.org/6993037/diff/2001/content/browser/tab_contents...
File content/browser/tab_contents/tab_contents.cc (right):

http://codereview.chromium.org/6993037/diff/2001/content/browser/tab_contents...
content/browser/tab_contents/tab_contents.cc:861: // navigation.  It's safer to
clear the pending entry in both cases than
On 2011/06/12 12:44:56, brettw wrote:
> Can you make more clear how this is wrong in the case of a new navigation? If
> somebody tries to fix that bug, I want it to be clear we know that it's
> happening in this case and why we need to do it.

Good idea.  I've tried to clarify it-- does it look better now?  Are there other
implications to clearing the pending entry I should mention?

[brettw, 2011-06-13 17:48:42]

New comment sounds fine. I can't think of any other problems, though there are
probably many. LGTM.