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

content/browser/frame_host/navigator_impl.cc

Index: content/browser/frame_host/navigator_impl.cc
diff --git a/content/browser/frame_host/navigator_impl.cc b/content/browser/frame_host/navigator_impl.cc
index f01dc531fca5073272ebf8e64a56d9a65e265bc1..38d09ece5e8ad61f1833d5f0fd7271318dc63864 100644
--- a/content/browser/frame_host/navigator_impl.cc
+++ b/content/browser/frame_host/navigator_impl.cc
@@ -105,7 +105,17 @@ void MakeNavigateParams(const NavigationEntryImpl& entry,
             entry.GetBrowserInitiatedPostData()->size());
   }
 
-  params->redirects = entry.redirect_chain();
+  // Set the redirect chain to the navigation's redirects, unless we are
+  // returning to a completed navigation (whose previous redirects don't apply).
+  bool is_transition_reload = PageTransitionCoreTypeIs(
+      params->transition, content::PAGE_TRANSITION_RELOAD);
+  bool is_transition_forward_back = PageTransitionIsForwardBack(

[Charlie Reis, 2014/03/31 22:06:08]

Does PageTransitionIsRedirect not suffice here?

[donnd, 2014/04/01 04:36:26]

No, returning to any previously committed navigation will pick up those
redirects, but we don't want to send them to sync as if we just did the redirect
all over again.

[brettw, 2014/04/01 06:28:52]

Ah, so this answer seems to be different than we were talking about on Friday. I
had asked you why you needed to clear here and you thought the list was getting
appended to, but weren't sure where.

In this comment, it seems like you're saying the redirects are preserved from
the original navigation "properly" when doing back-forward, but you don't want
them?

Is my summary correct?

If so, why don't we want these in sync? I don't think the contents of what's in
the navigation entry should be driven by what's desired by sync (although in
this case I question the desire) bur rather what makes sense to have in a
navigation entry. And I would expect the redirects to be there.

[donnd, 2014/04/01 18:31:09]

No, they need to be preserved in the original navigation but should not be
passed on to the new navigation on back-forward.
Sorry I have not been clear in communicating what happens in these cases.  When
I said "appended to" on Friday I meant that the previous redirects would be
preserved instead of cleared so subsequent navigations append to them.  

I think an example will help: When navigating to A, which redirects to B and
then C, we get a redirect chain ABC.  If the user does a reload or forward or
back, the redirect chain should just be C.  However, in this CL we're preserving
the redirect chain, so it will be appended to, and we'll see ABCC, which is
wrong.  Starting with an empty chain here fixes those cases.  

BTW, when I said we wouldn't want to send that to sync I just meant that as an
example of sending the wrong data to any part of the system.  We're just trying
to send a snapshot of the redirect chain to sync.

+      params->transition);
+  if (is_transition_forward_back || is_transition_reload) {
+    params->redirects = std::vector<GURL>();
+  } else {
+    params->redirects = entry.GetRedirectChain();
+  }
 
   params->can_load_local_resources = entry.GetCanLoadLocalResources();
   params->frame_to_navigate = entry.GetFrameToNavigate();
@@ -168,7 +178,7 @@ void NavigatorImpl::DidStartProvisionalLoad(
         entry->set_transferred_global_request_id(
             pending_entry->transferred_global_request_id());
         entry->set_should_replace_entry(pending_entry->should_replace_entry());
-        entry->set_redirect_chain(pending_entry->redirect_chain());
+        entry->SetRedirectChain(pending_entry->GetRedirectChain());
       }
       controller_->SetPendingEntry(entry);
       if (delegate_)