When a page is reloaded using the original request URL, ensure that the RVH picked to render it is …

When a page is reloaded using the original request URL, ensure that the RVH picked to render it is based on that URL and WebContentsObserver::NavigateToPendingEntry sees that new URL.

BUG=306569, 112923
R=creis@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=228418

[Charlie Reis, 2013-10-11 21:22:26]

[CC'ing dfalcantara as FYI.]

Great catch.  I was thinking about how to test that we use the correct
RenderViewHost for this case, but I'm not sure it's critical given the limited
ways it could happen on a reload.

One suggestion below.

https://codereview.chromium.org/27045005/diff/1/content/browser/web_contents/...
File content/browser/web_contents/web_contents_impl.cc (right):

https://codereview.chromium.org/27045005/diff/1/content/browser/web_contents/...
content/browser/web_contents/web_contents_impl.cc:1816: NavigationEntryImpl
entry(entry_in);
After taking a closer look, I wonder if we just want to update the entry before
passing it in here.  That way we don't end up with a difference between the
pending entry in the controller and the one we used here.

To do that, I think we can just move this check to
NavigationControllerImpl::ReloadInternal (right after the if (!entry) return
check on line 303).

[jam, 2013-10-11 22:24:48]

https://codereview.chromium.org/27045005/diff/1/content/browser/web_contents/...
File content/browser/web_contents/web_contents_impl.cc (right):

https://codereview.chromium.org/27045005/diff/1/content/browser/web_contents/...
content/browser/web_contents/web_contents_impl.cc:1816: NavigationEntryImpl
entry(entry_in);
On 2013/10/11 21:22:27, creis wrote:
> After taking a closer look, I wonder if we just want to update the entry
before
> passing it in here.  That way we don't end up with a difference between the
> pending entry in the controller and the one we used here.
> 
> To do that, I think we can just move this check to
> NavigationControllerImpl::ReloadInternal (right after the if (!entry) return
> check on line 303).

Done.

[Charlie Reis, 2013-10-11 22:29:36]

Thanks!  LGTM.