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

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 3664 matching lines @@
 
     // This message needs to be sent before any of allowScripts(),
     // allowImages(), allowPlugins() is called for the new page, so that when
     // these functions send a ViewHostMsg_ContentBlocked message, it arrives
     // after the FrameHostMsg_DidCommitProvisionalLoad message.
     Send(new FrameHostMsg_DidCommitProvisionalLoad(routing_id_, params));
   } else {
     // Subframe navigation: the type depends on whether this navigation
     // generated a new session history entry. When they do generate a session
     // history entry, it means the user initiated the navigation and we should
-    // mark it as such. This test checks if this is the first time
-    // SendDidCommitProvisionalLoad has been called since WillNavigateToURL was
-    // called to initiate the load.
-    if (render_view_->page_id_ > render_view_->last_page_id_sent_to_browser_)

[Charlie Reis, 2015/01/14 18:18:40]

If this works, that means last_page_id_sent_to_browser_ is dead code now.  Want
to remove it?

+    // mark it as such.
+    if (ds->replacesCurrentHistoryItem())

[Charlie Reis, 2015/01/14 18:18:40]

Interesting.  This is true for the first navigation in a frame?  That's great.

What about back/forward in a subframe?  I think that's supposed to report
AUTO_SUBFRAME.  (If we don't have test coverage for that, we should add it.  I
think most of NavigationController's tests in this area are unit tests that
don't check what the renderer reports.)

[Avi (use Gerrit), 2015/01/14 21:23:20]

Yes, it is. That's the only way in my rework that I can tell the difference.
Dammit, no. Going backwards, replacesCurrentHistoryItem returns false (as it's a
real nav) but the page id will obviously not be bigger.
Ugh. Lemme get to that.

+      params.transition = ui::PAGE_TRANSITION_AUTO_SUBFRAME;
+    else
       params.transition = ui::PAGE_TRANSITION_MANUAL_SUBFRAME;
-    else
-      params.transition = ui::PAGE_TRANSITION_AUTO_SUBFRAME;
 
     DCHECK(!navigation_state->history_list_was_cleared());
     params.history_list_was_cleared = false;
     params.report_type = FrameMsg_UILoadMetricsReportType::NO_REPORT;
 
     // Don't send this message while the subframe is swapped out.
     if (!is_swapped_out())
       Send(new FrameHostMsg_DidCommitProvisionalLoad(routing_id_, params));
   }
 
@@ skipping 629 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