X-Chrome-Connected is stripped when it should not be in headers.

chrome/browser/signin/chrome_signin_helper.cc

Index: chrome/browser/signin/chrome_signin_helper.cc
diff --git a/chrome/browser/signin/chrome_signin_helper.cc b/chrome/browser/signin/chrome_signin_helper.cc
index 6787db2f6026f1d4c358474b069de8e7e694bf78..c3911204de053269c6147e4a3c8ef5c05bd96fbb 100644
--- a/chrome/browser/signin/chrome_signin_helper.cc
+++ b/chrome/browser/signin/chrome_signin_helper.cc
@@ -118,11 +118,11 @@ ManageAccountsParams BuildManageAccountsParamsHelper(net::URLRequest* request,
 
 }  // namespace
 
-bool AppendMirrorRequestHeaderHelper(net::URLRequest* request,
-                                     const GURL& redirect_url,
-                                     ProfileIOData* io_data,
-                                     int child_id,
-                                     int route_id) {
+bool FixMirrorRequestHeaderHelper(net::URLRequest* request,
+                                  const GURL& redirect_url,
+                                  ProfileIOData* io_data,
+                                  int child_id,
+                                  int route_id) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
 
   if (io_data->IsOffTheRecord())
@@ -148,9 +148,23 @@ bool AppendMirrorRequestHeaderHelper(net::URLRequest* request,
     profile_mode_mask |= PROFILE_MODE_INCOGNITO_DISABLED;
   }
 

[mmenke, 2016/08/30 19:12:26]

Think everything below this point may belong in a single method in
signin_header_helper.h - seems like the code that handles
AppendMirrorRequestHeaderIfPossible belongs in the same file that handles
removing the header on redirect as well.

[Ramin Halavati, 2016/09/01 10:41:44]

Done.

The removed part is now moved to signin_header_helper.cc and the function
AppendMirrorRequestHeaderIfPossible is renamed to
AppendOrRemoveMirrorRequestHeaderIfPossible. Do you suggest that this one would
also become void?

-  return AppendMirrorRequestHeaderIfPossible(
-      request, redirect_url, io_data->google_services_account_id()->GetValue(),
-      io_data->GetCookieSettings(), profile_mode_mask);
+  // If new url is eligible to have the header, return true, otherwise if
+  // redirecting to another site and x-chrome-header exists, and the redirected

[mmenke, 2016/08/30 19:12:26]

x-chrome-header -> x-chrome-connected header (Or x-chrome header, I suppose)

[Ramin Halavati, 2016/09/01 10:41:44]

Done.

+  // site is not illigible and current site was illigible, remove it.

[mmenke, 2016/08/30 19:12:27]

illigible -> eligible (x2)

[Ramin Halavati, 2016/09/01 10:41:44]

Done.

+  if (AppendMirrorRequestHeaderIfPossible(
+          request, redirect_url,
+          io_data->google_services_account_id()->GetValue(),
+          io_data->GetCookieSettings(), profile_mode_mask)) {
+    return true;

[mmenke, 2016/08/30 19:12:27]

The return value isn't used, and it isn't clear what it means.  Let's just make
this method void.

[Ramin Halavati, 2016/09/01 10:41:44]

Done.

+  } else {

[mmenke, 2016/08/30 19:12:27]

Since you have the early return, this else isn't needed.

[Ramin Halavati, 2016/09/01 10:41:44]

Done.

+    if (!redirect_url.is_empty() &&  // redirecting
+        request->extra_request_headers().HasHeader(
+            signin::kChromeConnectedHeader) &&  // x-chrome-header exists

[mmenke, 2016/08/30 19:12:27]

Rather than inline comments, suggest writing out description in full sentences,
just before the if.

i.e. "If the request is being redirected from Google URL to a non-Google URL,
remove the x-chrome-connected header, if present."

[Ramin Halavati, 2016/09/01 10:41:44]

Done.

+        signin::IsUrlElligibleForXChromeConnectedHeader(request->url()) &&
+        !signin::IsUrlElligibleForXChromeConnectedHeader(redirect_url))
+      request->RemoveRequestHeaderByName(signin::kChromeConnectedHeader);

[mmenke, 2016/08/30 19:12:27]

Use braces when an if condition takes multiple lines.

[Ramin Halavati, 2016/09/01 10:41:44]

Done.

+  }
+  return false;
 }
 
 void ProcessMirrorResponseHeaderIfExists(net::URLRequest* request,