Avoid corrupting the pending_entry when an unexpected navigation commits.

Avoid corrupting the pending_entry when an unexpected navigation commits.

BUG=68350
TEST=TabContentsTest.CrossSiteNavigationBackPreempted
TEST=NavigationControllerTest.RestoreNavigate

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

[Charlie Reis, 2011-03-21 15:49:24]

This CL clears the pending entry after a commit of an existing entry, even if it
wasn't for the pending entry.  It also avoids corrupting the SiteInstance-- once
a NavigationEntry has a SiteInstance, it should never change.

This fix revealed that SafeBrowsingBlockingPageTest was using the wrong RVH in
one of its back navigations, so I fixed that as well.

[brettw, 2011-03-22 05:39:18]

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

http://codereview.chromium.org/6709056/diff/2001/content/browser/tab_contents...
content/browser/tab_contents/navigation_controller.cc:509: if
(pending_entry_index_ >= 0 && !pending_entry_->site_instance()) {
Is there a unit test covering this case?

http://codereview.chromium.org/6709056/diff/2001/content/browser/tab_contents...
content/browser/tab_contents/navigation_controller.cc:755: // is now likely
canceled.
Can you add here that it may not be cancelled, but then when it commits it will
just look like a new navigation, which will be OK.

[Charlie Reis, 2011-03-22 17:04:31]

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

http://codereview.chromium.org/6709056/diff/2001/content/browser/tab_contents...
content/browser/tab_contents/navigation_controller.cc:509: if
(pending_entry_index_ >= 0 && !pending_entry_->site_instance()) {
On 2011/03/22 05:39:18, brettw wrote:
> Is there a unit test covering this case?

Yes: NavigationControllerTest.RestoreNavigate checks that the site instance and
restore type are set properly.  It fails if this block is removed.

http://codereview.chromium.org/6709056/diff/2001/content/browser/tab_contents...
content/browser/tab_contents/navigation_controller.cc:755: // is now likely
canceled.
On 2011/03/22 05:39:18, brettw wrote:
> Can you add here that it may not be cancelled, but then when it commits it
will
> just look like a new navigation, which will be OK.

Done.

[brettw, 2011-03-22 17:42:42]

LGTM