Make NavigationParams clearer

content/browser/frame_host/navigation_entry_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/navigation_entry_impl.h"
 
 #include "base/metrics/histogram.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
-#include "content/browser/frame_host/navigation_controller_impl.h"
 #include "content/common/navigation_params.h"
 #include "content/public/common/content_constants.h"
 #include "content/public/common/url_constants.h"
 #include "net/base/net_util.h"
 #include "ui/gfx/text_elider.h"
 
 // Use this to get a new unique ID for a NavigationEntry during construction.
 // The returned ID is guaranteed to be nonzero (which is the "no ID" indicator).
 static int GetUniqueIDInConstructor() {
   static int unique_id_counter = 0;
@@ skipping 359 matching lines @@
   copy->redirect_chain_ = redirect_chain_;
   // ResetForCommit: should_clear_history_list_
   copy->frame_to_navigate_ = frame_to_navigate_;
   // ResetForCommit: frame_tree_node_id_
   // ResetForCommit: intent_received_timestamp_
   copy->extra_data_ = extra_data_;
 
   return copy;
 }
 
-void NavigationEntryImpl::ResetForCommit() {

[clamy, 2015/03/19 17:12:31]

The git diff is a bit weird in this file. What I'm doing is moving the
Construct* methods above ResetForCommit.

-  // Any state that only matters when a navigation entry is pending should be
-  // cleared here.
-  // TODO(creis): This state should be moved to NavigationRequest once
-  // PlzNavigate is enabled.
-  SetBrowserInitiatedPostData(nullptr);
-  set_source_site_instance(nullptr);
-  set_is_renderer_initiated(false);
-  set_transferred_global_request_id(GlobalRequestID());
-  set_should_replace_entry(false);
-
-  set_should_clear_history_list(false);
-  set_frame_tree_node_id(-1);
-
-#if defined(OS_ANDROID)
-  // Reset the time stamp so that the metrics are not reported if this entry is
-  // loaded again in the future.
-  set_intent_received_timestamp(base::TimeTicks());
-#endif
-}
-
-void NavigationEntryImpl::SetScreenshotPNGData(
-    scoped_refptr<base::RefCountedBytes> png_data) {
-  screenshot_ = png_data;
-  if (screenshot_.get())
-    UMA_HISTOGRAM_MEMORY_KB("Overscroll.ScreenshotSize", screenshot_->size());
-}
-
-GURL NavigationEntryImpl::GetHistoryURLForDataURL() const {
-  return GetBaseURLForDataURL().is_empty() ? GURL() : GetVirtualURL();
-}
-
 CommonNavigationParams NavigationEntryImpl::ConstructCommonNavigationParams(

[clamy, 2015/03/19 17:12:31]

I've left the Construct methods here until we have a better understanding of
where to put them to interact properly with the FrameNavigationEntry.

[Charlie Reis, 2015/03/19 20:45:51]

Acknowledged.

     FrameMsg_Navigate_Type::Value navigation_type) const {
   FrameMsg_UILoadMetricsReportType::Value report_type =
       FrameMsg_UILoadMetricsReportType::NO_REPORT;
   base::TimeTicks ui_timestamp = base::TimeTicks();
 #if defined(OS_ANDROID)
   if (!intent_received_timestamp().is_null())
     report_type = FrameMsg_UILoadMetricsReportType::REPORT_INTENT;
   ui_timestamp = intent_received_timestamp();
 #endif
 
   return CommonNavigationParams(
       GetURL(), GetReferrer(), GetTransitionType(), navigation_type,
       !IsViewSourceMode(), ui_timestamp, report_type, GetBaseURLForDataURL(),
       GetHistoryURLForDataURL());
 }
 
+StartNavigationParams NavigationEntryImpl::ConstructStartNavigationParams()
+    const {
+  std::vector<unsigned char> browser_initiated_post_data;
+  if (GetBrowserInitiatedPostData()) {
+    browser_initiated_post_data.assign(
+        GetBrowserInitiatedPostData()->front(),
+        GetBrowserInitiatedPostData()->front() +
+            GetBrowserInitiatedPostData()->size());
+  }
+
+  return StartNavigationParams(
+      GetHasPostData(), extra_headers(), browser_initiated_post_data,
+      should_replace_entry(), transferred_global_request_id().child_id,
+      transferred_global_request_id().request_id);
+}
+
 CommitNavigationParams NavigationEntryImpl::ConstructCommitNavigationParams(
-    base::TimeTicks navigation_start) const {
+    base::TimeTicks navigation_start,
+    int pending_history_list_offset,
+    int current_history_list_offset,
+    int current_history_list_length) const {
   // Set the redirect chain to the navigation's redirects, unless returning to a
   // completed navigation (whose previous redirects don't apply).
   std::vector<GURL> redirects;
   if (ui::PageTransitionIsNewNavigation(GetTransitionType())) {
     redirects = GetRedirectChain();
   }
 
-  return CommitNavigationParams(GetIsOverridingUserAgent(), navigation_start,
-                                redirects, GetCanLoadLocalResources(),
-                                GetFrameToNavigate(), base::Time::Now());
-}
-
-HistoryNavigationParams NavigationEntryImpl::ConstructHistoryNavigationParams(
-    NavigationControllerImpl* controller) const {
-  int pending_history_list_offset = controller->GetIndexOfEntry(this);
-  int current_history_list_offset = controller->GetLastCommittedEntryIndex();
-  int current_history_list_length = controller->GetEntryCount();
+  int pending_offset_to_send = pending_history_list_offset;
+  int current_offset_to_send = current_history_list_offset;
+  int current_length_to_send = current_history_list_length;
   if (should_clear_history_list()) {
     // Set the history list related parameters to the same values a
     // NavigationController would return before its first navigation. This will
     // fully clear the RenderView's view of the session history.
-    pending_history_list_offset = -1;
-    current_history_list_offset = -1;
-    current_history_list_length = 0;
+    pending_offset_to_send = -1;
+    current_offset_to_send = -1;
+    current_length_to_send = 0;
   }
-  return HistoryNavigationParams(
-      GetPageState(), GetPageID(), pending_history_list_offset,
-      current_history_list_offset, current_history_list_length,
+  return CommitNavigationParams(
+      GetIsOverridingUserAgent(), navigation_start, redirects,
+      GetCanLoadLocalResources(), GetFrameToNavigate(), base::Time::Now(),
+      GetPageState(), GetPageID(), pending_offset_to_send,
+      current_offset_to_send, current_length_to_send,
       should_clear_history_list());
 }
 
-StartNavigationParams NavigationEntryImpl::ConstructStartNavigationParams()
-    const {
-  std::vector<unsigned char> browser_initiated_post_data;
-  if (GetBrowserInitiatedPostData()) {
-    browser_initiated_post_data.assign(
-        GetBrowserInitiatedPostData()->front(),
-        GetBrowserInitiatedPostData()->front() +
-            GetBrowserInitiatedPostData()->size());
-  }
+void NavigationEntryImpl::ResetForCommit() {
+  // Any state that only matters when a navigation entry is pending should be
+  // cleared here.
+  // TODO(creis): This state should be moved to NavigationRequest once
+  // PlzNavigate is enabled.
+  SetBrowserInitiatedPostData(nullptr);
+  set_source_site_instance(nullptr);
+  set_is_renderer_initiated(false);
+  set_transferred_global_request_id(GlobalRequestID());
+  set_should_replace_entry(false);
 
-  return StartNavigationParams(
-      GetHasPostData(), extra_headers(), browser_initiated_post_data,
-      should_replace_entry(), transferred_global_request_id().child_id,
-      transferred_global_request_id().request_id);
+  set_should_clear_history_list(false);
+  set_frame_tree_node_id(-1);
+
+#if defined(OS_ANDROID)
+  // Reset the time stamp so that the metrics are not reported if this entry is
+  // loaded again in the future.
+  set_intent_received_timestamp(base::TimeTicks());
+#endif
+}
+
+void NavigationEntryImpl::SetScreenshotPNGData(
+    scoped_refptr<base::RefCountedBytes> png_data) {
+  screenshot_ = png_data;
+  if (screenshot_.get())
+    UMA_HISTOGRAM_MEMORY_KB("Overscroll.ScreenshotSize", screenshot_->size());
+}
+
+GURL NavigationEntryImpl::GetHistoryURLForDataURL() const {
+  return GetBaseURLForDataURL().is_empty() ? GURL() : GetVirtualURL();
 }
 
 }  // namespace content