[signin] Move Mirror code to ChromeConnectedHeaderHelper

chrome/browser/signin/chrome_signin_helper.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 "chrome/browser/signin/chrome_signin_helper.h"
 
 #include "base/strings/string_util.h"
 #include "build/build_config.h"
 #include "chrome/browser/prefs/incognito_mode_prefs.h"
 #include "chrome/browser/profiles/profile_io_data.h"
 #include "chrome/browser/signin/account_reconcilor_factory.h"
 #include "chrome/browser/signin/chrome_signin_client.h"
 #include "chrome/browser/tab_contents/tab_util.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/common/url_constants.h"
 #include "components/signin/core/browser/account_reconcilor.h"
+#include "components/signin/core/browser/chrome_connected_header_helper.h"
 #include "components/signin/core/browser/signin_header_helper.h"
+#include "components/signin/core/common/profile_management_switches.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/resource_request_info.h"
 #include "content/public/browser/web_contents.h"
 #include "google_apis/gaia/gaia_auth_util.h"
+#include "net/http/http_response_headers.h"
 #include "net/url_request/url_request.h"
 
 #if defined(OS_ANDROID)
 #include "chrome/browser/android/signin/account_management_screen_helper.h"
 #else
 #include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/browser_finder.h"
 #include "extensions/browser/guest_view/web_view/web_view_renderer_state.h"
 #endif  // defined(OS_ANDROID)
 
 namespace signin {
 
 namespace {
 
+const char kChromeManageAccountsHeader[] = "X-Chrome-Manage-Accounts";
+
 // Processes the mirror response header on the UI thread. Currently depending
 // on the value of |header_value|, it either shows the profile avatar menu, or
 // opens an incognito window/tab.
 void ProcessMirrorHeaderUIThread(int child_id,
                                  int route_id,
                                  ManageAccountsParams manage_accounts_params) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
+  if (switches::GetAccountConsistencyMethod() !=

[msarda, 2017/06/12 12:21:14]

I think we should not add this if - Gaia is not supposed to send an Manage
Accounts header when Mirror is not enabled.

[droger, 2017/06/12 12:46:38]

I agree that Gaia should not do it.

The question is: should a Gaia bug be able to crash Chrome? I changed it from a
DCHECK because I think we have to handle it gracefully, even if it is not
supposed to happen. Because this is not something that depends on Chrome code
and state, it depends on server input.

[msarda, 2017/06/12 14:08:57]

The DCHECK also included the fact that it does not work in incognito. I think it
would make sense to the the following:
* on ProcessMirrorResponseHeaderIfExists, we should add an if
(io_data->IsOffTheRecord() && switches::GetAccountConsistencyMethod() !=
switches::AccountConsistencyMethod::kMirror) (I suppose we can use the switches
on the IO thread, but I may be wrong).
* in ProcessMirrorHeaderUIThread, we should probably have a DCHECK(!incognito);
DCHECK_EQ(switches::AccountConsistencyMethod::kMirror,
switches::GetAccountConsistencyMethod())

+      switches::AccountConsistencyMethod::kMirror) {
+    return;
+  }
+
   GAIAServiceType service_type = manage_accounts_params.service_type;
   DCHECK_NE(GAIA_SERVICE_TYPE_NONE, service_type);
 
   content::WebContents* web_contents =
       tab_util::GetWebContentsByID(child_id, route_id);
   if (!web_contents)
     return;
 
   Profile* profile =
       Profile::FromBrowserContext(web_contents->GetBrowserContext());
@@ skipping 34 matching lines @@
         ui::PAGE_TRANSITION_AUTO_TOPLEVEL, false));
   } else {
     signin_metrics::LogAccountReconcilorStateOnGaiaResponse(
         account_reconcilor->GetState());
     AccountManagementScreenHelper::OpenAccountManagementScreen(profile,
                                                                service_type);
   }
 #endif  // !defined(OS_ANDROID)
 }
 
-// Returns the parameters contained in the X-Chrome-Manage-Accounts response
-// header.
-// If the request does not have a response header or if the header contains
-// garbage, then |service_type| is set to |GAIA_SERVICE_TYPE_NONE|.
-// Must be called on IO thread.
-ManageAccountsParams BuildManageAccountsParamsHelper(net::URLRequest* request,
-                                                     ProfileIOData* io_data) {
-  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
-
-  const content::ResourceRequestInfo* info =
-      content::ResourceRequestInfo::ForRequest(request);
-  if (!(info && info->GetResourceType() == content::RESOURCE_TYPE_MAIN_FRAME)) {
-    ManageAccountsParams empty_params;
-    empty_params.service_type = GAIA_SERVICE_TYPE_NONE;
-    return empty_params;
-  }
-
-  return BuildManageAccountsParamsIfExists(request, io_data->IsOffTheRecord());
-}
-
 }  // namespace
 
 void FixAccountConsistencyRequestHeader(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())
@@ skipping 22 matching lines @@
   // If new url is eligible to have the header, add it, otherwise remove it.
   AppendOrRemoveAccountConsistentyRequestHeader(
       request, redirect_url, io_data->google_services_account_id()->GetValue(),
       io_data->GetCookieSettings(), profile_mode_mask);
 }
 
 void ProcessMirrorResponseHeaderIfExists(net::URLRequest* request,
                                          ProfileIOData* io_data,
                                          int child_id,
                                          int route_id) {
-  ManageAccountsParams params =
-      BuildManageAccountsParamsHelper(request, io_data);
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+
+  if (io_data->IsOffTheRecord())

[msarda, 2017/06/12 14:08:57]

I think it would somehow make sense to have a NOTREACHED here as this is a
brreakage in the Chrome / Gaia protocol and if this starts to happen then there
is definitely a problem in the product that we need to fix. Or should this be
covered by an integration test?

+    return;
+
+  const content::ResourceRequestInfo* info =
+      content::ResourceRequestInfo::ForRequest(request);
+  if (!(info && info->GetResourceType() == content::RESOURCE_TYPE_MAIN_FRAME))
+    return;
+
+  if (!gaia::IsGaiaSignonRealm(request->url().GetOrigin()))
+    return;
+
+  net::HttpResponseHeaders* response_headers = request->response_headers();
+  if (!response_headers)
+    return;
+
+  std::string header_value;
+  if (!response_headers->GetNormalizedHeader(kChromeManageAccountsHeader,
+                                             &header_value)) {
+    return;
+  }
+
+  ManageAccountsParams params = BuildManageAccountsParams(header_value);
+  // If the request does not have a response header or if the header contains
+  // garbage, then |service_type| is set to |GAIA_SERVICE_TYPE_NONE|.
   if (params.service_type == GAIA_SERVICE_TYPE_NONE)
     return;
 
   params.child_id = child_id;
   params.route_id = route_id;
   content::BrowserThread::PostTask(
       content::BrowserThread::UI, FROM_HERE,
       base::Bind(ProcessMirrorHeaderUIThread, child_id, route_id, params));
 }
 
 }  // namespace signin