Add the navigation redirect-chain to Sync sessions proto for offline analysis.

content/browser/frame_host/navigation_controller_impl.cc

Index: content/browser/frame_host/navigation_controller_impl.cc
diff --git a/content/browser/frame_host/navigation_controller_impl.cc b/content/browser/frame_host/navigation_controller_impl.cc
index 0526f692218eaa30b184a214ac57c3fa2330fde2..3d672cedf5187b459fa07a4ac8ef2712ec7008e6 100644
--- a/content/browser/frame_host/navigation_controller_impl.cc
+++ b/content/browser/frame_host/navigation_controller_impl.cc
@@ -694,7 +694,7 @@ void NavigationControllerImpl::LoadURLWithParams(const LoadURLParams& params) {
   if (params.frame_tree_node_id != -1)
     entry->set_frame_tree_node_id(params.frame_tree_node_id);
   if (params.redirect_chain.size() > 0)
-    entry->set_redirect_chain(params.redirect_chain);
+    entry->SetRedirectChain(params.redirect_chain);
   if (params.should_replace_current_entry)
     entry->set_should_replace_entry(true);
   entry->set_should_clear_history_list(params.should_clear_history_list);
@@ -815,6 +815,8 @@ bool NavigationControllerImpl::RendererDidNavigate(
   active_entry->SetTimestamp(timestamp);
   active_entry->SetHttpStatusCode(params.http_status_code);
   active_entry->SetPageState(params.page_state);
+  active_entry->SetRedirectChain(
+      GetMergedRedirectChain(params, active_entry->GetRedirectChain()));

[haitaol1, 2014/01/17 23:52:23]

I think previous version just overwrote entry's chain with params' chain and
seems working fine. I'm just curious in what case merging is needed. Is there a
risk that merging can lead to mixing of redirect chains of different
navigations.

[Donn Denman, 2014/01/18 01:30:19]

Actually this seems to happen quite frequently.  One example is case I put in
the design doc of voice.google.com.  I just tested this, and I see an active
entry of length 2 and params of length 2 that need to be merged to get the
correct 3-entry chain.

Frankly I don't know if there's a risk of mixing redirect chains from different
navigations, do you?

[haitaol1, 2014/01/23 23:19:43]

I don't know either. It would be better if you can confirm with Brett. But I
won't hold off you on this.

On 2014/01/18 01:30:19, Donn Denman wrote:

 
   // Once it is committed, we no longer need to track several pieces of state on
   // the entry.
@@ -841,6 +843,39 @@ bool NavigationControllerImpl::RendererDidNavigate(
   return true;
 }
 
+// Produces a redirect chain by merging the active entry redirects with the
+// redirects given in the params, taking into account the type of navigation
+// specified in the params, and eliminating some overlap.
+// static
+std::vector<GURL> NavigationControllerImpl::GetMergedRedirectChain(
+    const ViewHostMsg_FrameNavigate_Params& params,
+    const std::vector<GURL>& active_entry_redirects) const {
+  // TODO(donnd): are there other transition cases we need to handle specially?
+  bool is_transition_reload = PageTransitionCoreTypeIs(
+      params.transition, content::PAGE_TRANSITION_RELOAD);
+  bool is_transition_forward_back = PageTransitionIsForwardBack(
+      params.transition);
+  if (is_transition_forward_back || is_transition_reload) {
+    // Returning to an existing page so the active entry is complete.
+    return active_entry_redirects;
+  } else if (active_entry_redirects.size() == 0) {
+    return params.redirects;
+  } else {
+    // Merge new redirects into the active entry.
+    std::vector<GURL> merged_redirects = active_entry_redirects;
+    // Remove entries that duplicate the last URL from the active entry.
+    std::vector<GURL>::const_iterator first_different_param =
+        params.redirects.begin();
+    while (first_different_param != params.redirects.end() &&
+        *first_different_param == merged_redirects.back()) {
+      first_different_param++;
+    }
+    merged_redirects.insert(
+        merged_redirects.end(), first_different_param, params.redirects.end());
+    return merged_redirects;
+  }
+}
+
 NavigationType NavigationControllerImpl::ClassifyNavigation(
     RenderViewHost* rvh,
     const ViewHostMsg_FrameNavigate_Params& params) const {