Remove the use of page id from building the commit params.

content/browser/frame_host/navigator_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/browser/frame_host/navigator_impl.h"
 
 #include "base/command_line.h"
 #include "base/metrics/histogram.h"
 #include "base/time/time.h"
 #include "content/browser/frame_host/frame_tree.h"
@@ skipping 489 matching lines @@
   }
 
   FrameHostMsg_DidCommitProvisionalLoad_Params params(input_params);
   FrameTree* frame_tree = render_frame_host->frame_tree_node()->frame_tree();
   bool use_site_per_process = base::CommandLine::ForCurrentProcess()->HasSwitch(
       switches::kSitePerProcess);
 
   if (use_site_per_process) {
     // TODO(creis): Until we mirror the frame tree in the subframe's process,
     // cross-process subframe navigations happen in a renderer's main frame.
-    // Correct the transition type here if we know it is for a subframe.
+    // Correct the transition type here if we know it is for a subframe, but it
+    // doesn't have a subframe transition type.
     NavigationEntryImpl* pending_entry =
         NavigationEntryImpl::FromNavigationEntry(
             controller_->GetPendingEntry());
     if (!render_frame_host->frame_tree_node()->IsMainFrame() &&
+        params.transition != ui::PAGE_TRANSITION_AUTO_SUBFRAME &&
+        params.transition != ui::PAGE_TRANSITION_MANUAL_SUBFRAME &&
         pending_entry &&
         pending_entry->frame_tree_node_id() ==
             render_frame_host->frame_tree_node()->frame_tree_node_id()) {
       params.transition = ui::PAGE_TRANSITION_AUTO_SUBFRAME;
     }
   }

[Avi (use Gerrit), 2015/02/02 20:59:36]

My question is whether this block is useful any more. It was smashing the
transition in the unit test, but it turns out that when we do a cross-process
subframe transition, we're getting the correct value!

I made a quick CL to remove it (https://codereview.chromium.org/894843002) but
without a --site-per-process trybot, try results there are useless.

[Avi (use Gerrit), 2015/02/02 21:06:21]

What I mean is, in the
NavigationControllerBrowserTest.ManualAndAutoSubframeNavigationClassification
test, where we explicitly generate both types of subframe navigations, it passes
in --site-per-process with this block completely gone.

Because that is stated in the comments as being the point of the block, is the
block useless now?

[Charlie Reis, 2015/02/02 21:24:29]

Yes!  I'm pretty sure that your render_frame_impl.cc change makes it
unnecessary.  I'm happy to see this block die.

(Indeed, I'd already killed it for similar reasons in the draft session history
CL:
https://codereview.chromium.org/281653003/diff/100001/content/browser/frame_h...)

 
   if (ui::PageTransitionIsMainFrame(params.transition)) {
     if (delegate_) {
       // When overscroll navigation gesture is enabled, a screenshot of the page
       // in its current state is taken so that it can be used during the
       // nav-gesture. It is necessary to take the screenshot here, before
       // calling RenderFrameHostManager::DidNavigateMainFrame, because that can
       // change WebContents::GetRenderViewHost to return the new host, instead
       // of the one that may have just been swapped out.
       if (delegate_->CanOverscrollContent()) {
@@ skipping 450 matching lines @@
         "Navigation.TimeToCommit_ExistingRenderer_BeforeUnloadDiscounted",
         time_to_commit);
     UMA_HISTOGRAM_TIMES(
         "Navigation.TimeToURLJobStart_ExistingRenderer_BeforeUnloadDiscounted",
         time_to_network);
   }
   navigation_data_.reset();
 }
 
 }  // namespace content