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 57a92f55a4ee341032d2dc8492133b699d0db368..2f97bc8c34e171fd8bd87618dfe2e655bed90f3e 100644
--- a/content/browser/frame_host/navigation_controller_impl.cc
+++ b/content/browser/frame_host/navigation_controller_impl.cc
@@ -705,7 +705,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);
@@ -826,6 +826,8 @@ bool NavigationControllerImpl::RendererDidNavigate(
   active_entry->SetTimestamp(timestamp);
   active_entry->SetHttpStatusCode(params.http_status_code);
   active_entry->SetPageState(params.page_state);
+  active_entry->SetRedirectChain(

[sky, 2014/02/20 22:12:26]

Seems like we should only need to do this for some types of navigations. For
example, auto-subframe doesn't seem like it should impact redirects.

[Donn Denman, 2014/02/21 08:18:56]

Agreed, but I don't feel I know how to enumerate those navigations, which is why
I flagged a TODO for myself below.  Maybe you or Brett could recommend someone
to review just this part of the CL so we can address this shortcoming?

[sky, 2014/02/21 17:01:13]

Brett knows this code better than I. Hopefully he can offer a suggestion.

+      GetMergedRedirectChain(params, active_entry->GetRedirectChain()));
 
   // Once it is committed, we no longer need to track several pieces of state on
   // the entry.
@@ -852,6 +854,40 @@ 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 FrameHostMsg_DidCommitProvisionalLoad_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) {

[sky, 2014/02/20 22:12:26]

no else after return (see style guide).

[Donn Denman, 2014/02/21 08:18:56]

Done, and also for the second "else" below.

+    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 FrameHostMsg_DidCommitProvisionalLoad_Params& params) const {