Do not reset NavigationHandle when navigating same-page

content/browser/frame_host/navigation_controller_impl.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 /*
  * Copyright (C) 2006, 2007, 2008, 2009 Apple Inc. All rights reserved.
  * Copyright (C) 2008 Nokia Corporation and/or its subsidiary(-ies)
  * Copyright (C) 2008, 2009 Torch Mobile Inc. All rights reserved.
  *     (http://www.torchmobile.com/)
  *
@@ skipping 763 matching lines @@
 bool NavigationControllerImpl::PendingEntryMatchesHandle(
     NavigationHandleImpl* handle) const {
   return pending_entry_ &&
          pending_entry_->GetUniqueID() == handle->pending_nav_entry_id();
 }
 
 bool NavigationControllerImpl::RendererDidNavigate(
     RenderFrameHostImpl* rfh,
     const FrameHostMsg_DidCommitProvisionalLoad_Params& params,
     LoadCommittedDetails* details,
-    bool is_navigation_within_page) {
+    bool is_navigation_within_page,
+    NavigationHandleImpl* navigation_handle) {
   is_initial_navigation_ = false;
 
   // Save the previous state before we clobber it.
   if (GetLastCommittedEntry()) {
     details->previous_url = GetLastCommittedEntry()->GetURL();
     details->previous_entry_index = GetLastCommittedEntryIndex();
   } else {
     details->previous_url = GURL();
     details->previous_entry_index = -1;
   }
@@ skipping 10 matching lines @@
   details->did_replace_entry = params.should_replace_current_entry;
 
   // Do navigation-type specific actions. These will make and commit an entry.
   details->type = ClassifyNavigation(rfh, params);
 
   // is_in_page must be computed before the entry gets committed.
   details->is_in_page = is_navigation_within_page;
 
   // Save reload type and timestamp for a reload navigation to detect
   // consecutive reloads when the next reload is requested.
-  if (PendingEntryMatchesHandle(rfh->navigation_handle())) {
+  if (PendingEntryMatchesHandle(navigation_handle)) {
     if (pending_entry_->reload_type() != ReloadType::NONE) {
       last_committed_reload_type_ = pending_entry_->reload_type();
       last_committed_reload_time_ =
           time_smoother_.GetSmoothedTime(get_timestamp_callback_.Run());
     } else if (!pending_entry_->is_renderer_initiated() ||
                params.gesture == NavigationGestureUser) {
       last_committed_reload_type_ = ReloadType::NONE;
       last_committed_reload_time_ = base::Time();
     }
   }
 
   switch (details->type) {
     case NAVIGATION_TYPE_NEW_PAGE:
       RendererDidNavigateToNewPage(rfh, params, details->is_in_page,
-                                   details->did_replace_entry);
+                                   details->did_replace_entry,
+                                   navigation_handle);
       break;
     case NAVIGATION_TYPE_EXISTING_PAGE:
       details->did_replace_entry = details->is_in_page;
-      RendererDidNavigateToExistingPage(rfh, params, details->is_in_page);
+      RendererDidNavigateToExistingPage(rfh, params, details->is_in_page,
+                                        navigation_handle);
       break;
     case NAVIGATION_TYPE_SAME_PAGE:
-      RendererDidNavigateToSamePage(rfh, params);
+      RendererDidNavigateToSamePage(rfh, params, navigation_handle);
       break;
     case NAVIGATION_TYPE_NEW_SUBFRAME:
       RendererDidNavigateNewSubframe(rfh, params, details->is_in_page,
                                      details->did_replace_entry);
       break;
     case NAVIGATION_TYPE_AUTO_SUBFRAME:
       if (!RendererDidNavigateAutoSubframe(rfh, params)) {
         // In UseSubframeNavigationEntries mode, we won't send a notification
         // about auto-subframe PageState during UpdateStateForFrame, since it
         // looks like nothing has changed.  Send it here at commit time instead.
@@ skipping 208 matching lines @@
 
   // Since we weeded out "new" navigations above, we know this is an existing
   // (back/forward) navigation.
   return NAVIGATION_TYPE_EXISTING_PAGE;
 }
 
 void NavigationControllerImpl::RendererDidNavigateToNewPage(
     RenderFrameHostImpl* rfh,
     const FrameHostMsg_DidCommitProvisionalLoad_Params& params,
     bool is_in_page,
-    bool replace_entry) {
+    bool replace_entry,
+    NavigationHandleImpl* handle) {
   std::unique_ptr<NavigationEntryImpl> new_entry;
   bool update_virtual_url = false;
 
   // First check if this is an in-page navigation.  If so, clone the current
   // entry instead of looking at the pending entry, because the pending entry
   // does not have any subframe history items.
   if (is_in_page && GetLastCommittedEntry()) {
     FrameNavigationEntry* frame_entry = new FrameNavigationEntry(
         params.frame_unique_name, params.item_sequence_number,
         params.document_sequence_number, rfh->GetSiteInstance(), nullptr,
         params.url, params.referrer, params.method, params.post_id);
     new_entry = GetLastCommittedEntry()->CloneAndReplace(
         frame_entry, true, rfh->frame_tree_node(),
         delegate_->GetFrameTree()->root());
 
     // We expect |frame_entry| to be owned by |new_entry|.  This should never
     // fail, because it's the main frame.
     CHECK(frame_entry->HasOneRef());
 
     update_virtual_url = new_entry->update_virtual_url_with_url();
   }
 
   // Only make a copy of the pending entry if it is appropriate for the new page
   // that was just loaded. Verify this by checking if the entry corresponds
-  // to the current navigation handle. Note that in some tests the render frame
-  // host does not have a valid handle. Additionally, coarsely check that:
+  // to the current navigation handle. Additionally, coarsely check that:

[Charlie Reis, 2016/11/04 17:30:53]

nit: s/current/given/

[clamy, 2016/11/07 14:10:51]

Done.

   // 1. The SiteInstance hasn't been assigned to something else.
   // 2. The pending entry was intended as a new entry, rather than being a
   // history navigation that was interrupted by an unrelated,
   // renderer-initiated navigation.
   // TODO(csharrison): Investigate whether we can remove some of the coarser
   // checks.
-  NavigationHandleImpl* handle = rfh->navigation_handle();
-  DCHECK(handle);
-
   if (!new_entry &&
       PendingEntryMatchesHandle(handle) && pending_entry_index_ == -1 &&
       (!pending_entry_->site_instance() ||
        pending_entry_->site_instance() == rfh->GetSiteInstance())) {
     new_entry = pending_entry_->Clone();
 
     update_virtual_url = new_entry->update_virtual_url_with_url();
     new_entry->GetSSL() = handle->ssl_status();
   }
 
@@ skipping 59 matching lines @@
     entries_.clear();
     last_committed_entry_index_ = -1;
   }
 
   InsertOrReplaceEntry(std::move(new_entry), replace_entry);
 }
 
 void NavigationControllerImpl::RendererDidNavigateToExistingPage(
     RenderFrameHostImpl* rfh,
     const FrameHostMsg_DidCommitProvisionalLoad_Params& params,
-    bool is_in_page) {
+    bool is_in_page,
+    NavigationHandleImpl* handle) {
   // We should only get here for main frame navigations.
   DCHECK(!rfh->GetParent());
 
   // TODO(creis): Classify location.replace as NEW_PAGE instead of EXISTING_PAGE
   // in https://crbug.com/596707.
 
   NavigationEntryImpl* entry;
-  NavigationHandleImpl* handle = rfh->navigation_handle();
   if (params.intended_as_new_entry) {
     // This was intended as a new entry but the pending entry was lost in the
     // meanwhile and no new page was created. We are stuck at the last committed
     // entry.
     entry = GetLastCommittedEntry();
   } else if (params.nav_entry_id) {
     // This is a browser-initiated navigation (back/forward/reload).
     entry = GetEntryWithUniqueID(params.nav_entry_id);
 
     // Needed for the restore case, where the serialized NavigationEntry doesn't
@@ skipping 52 matching lines @@
   // actually change any other state, just kill the pointer.
   DiscardNonCommittedEntriesInternal();
 
   // If a transient entry was removed, the indices might have changed, so we
   // have to query the entry index again.
   last_committed_entry_index_ = GetIndexOfEntry(entry);
 }
 
 void NavigationControllerImpl::RendererDidNavigateToSamePage(
     RenderFrameHostImpl* rfh,
-    const FrameHostMsg_DidCommitProvisionalLoad_Params& params) {
+    const FrameHostMsg_DidCommitProvisionalLoad_Params& params,
+    NavigationHandleImpl* handle) {
   // This classification says that we have a pending entry that's the same as
   // the last committed entry. This entry is guaranteed to exist by
   // ClassifyNavigation. All we need to do is update the existing entry.
   NavigationEntryImpl* existing_entry = GetLastCommittedEntry();
 
   // If we classified this correctly, the SiteInstance should not have changed.
   CHECK_EQ(existing_entry->site_instance(), rfh->GetSiteInstance());
 
   // We assign the entry's unique ID to be that of the new one. Since this is
   // always the result of a user action, we want to dismiss infobars, etc. like
   // a regular user-initiated navigation.
   DCHECK_EQ(pending_entry_->GetUniqueID(), params.nav_entry_id);
   existing_entry->set_unique_id(pending_entry_->GetUniqueID());
 
   // The URL may have changed due to redirects.
   existing_entry->set_page_type(params.url_is_unreachable ? PAGE_TYPE_ERROR
                                                           : PAGE_TYPE_NORMAL);
   if (existing_entry->update_virtual_url_with_url())
     UpdateVirtualURLToURL(existing_entry, params.url);
   existing_entry->SetURL(params.url);
 
   // If a user presses enter in the omnibox and the server redirects, the URL
   // might change (but it's still considered a SAME_PAGE navigation). So we must
   // update the SSL status.
-  existing_entry->GetSSL() = rfh->navigation_handle()->ssl_status();
+  existing_entry->GetSSL() = handle->ssl_status();
 
   // The extra headers may have changed due to reloading with different headers.
   existing_entry->set_extra_headers(pending_entry_->extra_headers());
 
   // Update the existing FrameNavigationEntry to ensure all of its members
   // reflect the parameters coming from the renderer process.
   existing_entry->AddOrUpdateFrameEntry(
       rfh->frame_tree_node(), params.item_sequence_number,
       params.document_sequence_number, rfh->GetSiteInstance(), nullptr,
       params.url, params.referrer, params.redirects, params.page_state,
@@ skipping 802 matching lines @@
     }
   }
 }
 
 void NavigationControllerImpl::SetGetTimestampCallbackForTest(
     const base::Callback<base::Time()>& get_timestamp_callback) {
   get_timestamp_callback_ = get_timestamp_callback;
 }
 
 }  // namespace content