Classify navigations without page id in parallel to the existing classifier.

content/renderer/render_frame_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.
 
 #include "content/renderer/render_frame_impl.h"
 
 #include <map>
 #include <string>
 
 #include "base/auto_reset.h"
@@ skipping 1073 matching lines @@
     bool ignore_cache = (common_params.navigation_type ==
                          FrameMsg_Navigate_Type::RELOAD_IGNORING_CACHE);
 
     if (reload_original_url)
       frame->reloadWithOverrideURL(common_params.url, true);
     else
       frame->reload(ignore_cache);
   } else if (is_history_navigation) {
     // We must know the page ID of the page we are navigating back to.
     DCHECK_NE(history_params.page_id, -1);
+    // We must know the nav entry ID of the page we are navigating back to,
+    // which should be the case because history navigations are browser-
+    // initiated.
+    DCHECK_NE(0, history_params.nav_entry_id);
     scoped_ptr<HistoryEntry> entry =
         PageStateToHistoryEntry(history_params.page_state);
     if (entry) {
       // Ensure we didn't save the swapped out URL in UpdateState, since the
       // browser should never be telling us to navigate to swappedout://.
       CHECK(entry->root().urlString() != WebString::fromUTF8(kSwappedOutURL));
       render_view_->history_controller()->GoToEntry(entry.Pass(), cache_policy);
     }
   } else if (!common_params.base_url_for_data_url.is_empty()) {
     LoadDataURL(common_params, frame);
@@ skipping 24 matching lines @@
         data = reinterpret_cast<const char*>(
             &start_params.browser_initiated_post_data.front());
       }
       http_body.appendData(
           WebData(data, start_params.browser_initiated_post_data.size()));
       request.setHTTPBody(http_body);
     }
 
     // A session history navigation should have been accompanied by state.
     CHECK_EQ(history_params.page_id, -1);
+    // FYIREMOVEME(avi): Page id being -1 will happen with browser-initiated new
+    // (non-history) navigations, and that's the test here. Nav entry unique
+    // ids, however, are always provided with browser-initiated navigations,
+    // history or new, so this test isn't valid for nav entry unique ids.
 
     // Record this before starting the load, we need a lower bound of this time
     // to sanitize the navigationStart override set below.
     base::TimeTicks renderer_navigation_start = base::TimeTicks::Now();
     frame->loadRequest(request);
 
     UpdateFrameNavigationTiming(frame, commit_params.browser_navigation_start,
                                 renderer_navigation_start);
   }
 
@@ skipping 1281 matching lines @@
   // If this is a failed back/forward/reload navigation, then we need to do a
   // 'replace' load.  This is necessary to avoid messing up session history.
   // Otherwise, we do a normal load, which simulates a 'go' navigation as far
   // as session history is concerned.
   //
   // AUTO_SUBFRAME loads should always be treated as loads that do not advance
   // the page id.
   //
   // TODO(davidben): This should also take the failed navigation's replacement
   // state into account, if a location.replace() failed.
+  ui::PageTransition transition = navigation_state->transition_type();
   bool replace =
       navigation_state->pending_page_id() != -1 ||
-      ui::PageTransitionCoreTypeIs(navigation_state->transition_type(),
-                               ui::PAGE_TRANSITION_AUTO_SUBFRAME);
+      ui::PageTransitionCoreTypeIs(transition,
+                                   ui::PAGE_TRANSITION_AUTO_SUBFRAME);
+  bool replace2 =
+      ui::PageTransitionCoreTypeIs(transition, ui::PAGE_TRANSITION_RELOAD) ||
+      transition & ui::PAGE_TRANSITION_FORWARD_BACK ||
+      ui::PageTransitionCoreTypeIs(transition,
+                                   ui::PAGE_TRANSITION_AUTO_SUBFRAME);
+  DCHECK_EQ(replace, replace2);
 
   // If we failed on a browser initiated request, then make sure that our error
   // page load is regarded as the same browser initiated request.
   if (!navigation_state->is_content_initiated()) {
     render_view_->pending_navigation_params_.reset(new NavigationParams(
         CommonNavigationParams(
             error.unreachableURL, Referrer(),
             navigation_state->transition_type(), FrameMsg_Navigate_Type::NORMAL,
             true, base::TimeTicks(),
             FrameMsg_UILoadMetricsReportType::NO_REPORT, GURL(), GURL()),
         StartNavigationParams(false, std::string(),
                               std::vector<unsigned char>(), replace, -1, -1),
         CommitNavigationParams(false, base::TimeTicks(), std::vector<GURL>(),
                                false, std::string(),
                                document_state->request_time()),
         HistoryNavigationParams(
             PageState(), navigation_state->pending_page_id(),
+            navigation_state->pending_nav_entry_id(),
             navigation_state->pending_history_list_offset(), -1, 0,
             navigation_state->history_list_was_cleared())));
   }
 
   // Load an error page.
   LoadNavigationErrorPage(failed_request, error, replace);
 }
 
 void RenderFrameImpl::didCommitProvisionalLoad(
     blink::WebLocalFrame* frame,
@@ skipping 28 matching lines @@
 
   if (document_state->commit_load_time().is_null())
     document_state->set_commit_load_time(Time::Now());
 
   if (internal_data->must_reset_scroll_and_scale_state()) {
     render_view_->webview()->resetScrollAndScaleState();
     internal_data->set_must_reset_scroll_and_scale_state(false);
   }
   internal_data->set_use_error_page(false);
 
+  bool successful_history_nav = false;
   bool is_new_navigation = commit_type == blink::WebStandardCommit;
   if (is_new_navigation) {
     // We bump our Page ID to correspond with the new session history entry.
     render_view_->page_id_ = render_view_->next_page_id_++;
 
     // Don't update history list values for kSwappedOutURL, since
     // we don't want to forget the entry that was there, and since we will
     // never come back to kSwappedOutURL.  Note that we have to call
     // UpdateSessionHistory and update page_id_ even in this case, so that
     // the current entry gets a state update and so that we don't send a
@@ skipping 17 matching lines @@
     // history, the first committed frame suffices.  We keep track of whether
     // we've seen this commit before so that only capture session history once
     // per navigation.
     //
     // Note that we need to check if the page ID changed. In the case of a
     // reload, the page ID doesn't change, and UpdateSessionHistory gets the
     // previous URL and the current page ID, which would be wrong.
     if (navigation_state->pending_page_id() != -1 &&
         navigation_state->pending_page_id() != render_view_->page_id_ &&
         !navigation_state->request_committed()) {
+      successful_history_nav = true;

[Charlie Reis, 2015/03/12 18:28:17]

nit: Put below the following comment.

[Avi (use Gerrit), 2015/03/12 19:21:18]

Done.

       // This is a successful session history navigation!
       render_view_->page_id_ = navigation_state->pending_page_id();
 
       render_view_->history_list_offset_ =
           navigation_state->pending_history_list_offset();
     }
   }
 
+  if (commit_type == blink::WebBackForwardCommit) {
+    // Checking the commit type is planned to be the replacement for the if()
+    // block containing the comment "This is a successful session history
+    // navigation!" Make sure that it fires correspondingly.
+    DCHECK(successful_history_nav);
+  }
+
   bool sent = Send(
       new FrameHostMsg_DidAssignPageId(routing_id_, render_view_->page_id_));
   CHECK(sent);  // http://crbug.com/407376
 
   FOR_EACH_OBSERVER(RenderViewObserver, render_view_->observers_,
                     DidCommitProvisionalLoad(frame, is_new_navigation));
   FOR_EACH_OBSERVER(RenderFrameObserver, observers_,
                     DidCommitProvisionalLoad(
                         is_new_navigation,
                         navigation_state->was_within_same_page()));
@@ skipping 1126 matching lines @@
 
   DocumentState* document_state = DocumentState::FromDataSource(ds);
   NavigationState* navigation_state = document_state->navigation_state();
   InternalDocumentStateData* internal_data =
       InternalDocumentStateData::FromDocumentState(document_state);
 
   FrameHostMsg_DidCommitProvisionalLoad_Params params;
   params.http_status_code = response.httpStatusCode();
   params.url_is_unreachable = ds->hasUnreachableURL();
   params.is_post = false;
+  params.commit_type = commit_type;

[Charlie Reis, 2015/03/12 18:28:17]

I'd like to avoid exposing this directly if possible.  It looks like we only
need a bool for whether it's a standard commit in the long run, since we're only
checking whether it's Inert in DCHECKs.

(I'm fine with it for now as we explore what's needed to get this to work.)

[Avi (use Gerrit), 2015/03/12 19:21:17]

Let me hack on getting the tests to pass, but I'm OK with making it a boolean.

   params.post_id = -1;
   params.page_id = render_view_->page_id_;
+  params.nav_entry_id = navigation_state->pending_nav_entry_id();
   // We need to track the RenderViewHost routing_id because of downstream
   // dependencies (crbug.com/392171 DownloadRequestHandle, SaveFileManager,
   // ResourceDispatcherHostImpl, MediaStreamUIProxy,
   // SpeechRecognitionDispatcherHost and possibly others). They look up the view
   // based on the ID stored in the resource requests. Once those dependencies
   // are unwound or moved to RenderFrameHost (crbug.com/304341) we can move the
   // client to be based on the routing_id of the RenderFrameHost.
   params.render_view_routing_id = render_view_->routing_id();
   params.socket_address.set_host(response.remoteIPAddress().utf8());
   params.socket_address.set_port(response.remotePort());
@@ skipping 804 matching lines @@
 
 #if defined(ENABLE_BROWSER_CDMS)
 RendererCdmManager* RenderFrameImpl::GetCdmManager() {
   if (!cdm_manager_)
     cdm_manager_ = new RendererCdmManager(this);
   return cdm_manager_;
 }
 #endif  // defined(ENABLE_BROWSER_CDMS)
 
 }  // namespace content