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

content/renderer/render_frame_impl.cc

Index: content/renderer/render_frame_impl.cc
diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc
index aae437eccf675b949c1f14058e2f1c680f971eff..6c5c68cc45be6679613e88efd1f84ca6834ae4a5 100644
--- a/content/renderer/render_frame_impl.cc
+++ b/content/renderer/render_frame_impl.cc
@@ -1091,6 +1091,10 @@ void RenderFrameImpl::OnNavigate(
   } 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) {
@@ -1135,6 +1139,10 @@ void RenderFrameImpl::OnNavigate(
 
     // 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.
@@ -2436,10 +2444,17 @@ void RenderFrameImpl::didFailProvisionalLoad(blink::WebLocalFrame* frame,
   //
   // 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.
@@ -2457,6 +2472,7 @@ void RenderFrameImpl::didFailProvisionalLoad(blink::WebLocalFrame* frame,
                                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())));
   }
@@ -2505,6 +2521,7 @@ void RenderFrameImpl::didCommitProvisionalLoad(
   }
   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.
@@ -2542,6 +2559,7 @@ void RenderFrameImpl::didCommitProvisionalLoad(
     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();
 
@@ -2550,6 +2568,13 @@ void RenderFrameImpl::didCommitProvisionalLoad(
     }
   }
 
+  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
@@ -3696,8 +3721,10 @@ void RenderFrameImpl::SendDidCommitProvisionalLoad(
   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,