[Dice] Send Dice request header even when cookies are blocked

Dice headers were not sent when cookies are blocked, which is the same
behavior as Mirror.
However, in the Mirror case, not sending the header was disabling the
feature much earlier (i.e. we would not even show the signin page).
For Dice, the signin page is shown anyway, and thus there is no point in
having this extra check.
Removing that check allows the Gaia page to better detect what is going
wrong, and show a better error page to the user (telling to the user to
re-enable their cookies, instead of a generic "Something went wrong"
message).

Bug: 860038
Change-Id: I63cca38737ac85df420f011d9d59b699ccb3f104
Reviewed-on: https://chromium-review.googlesource.com/1128870Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Commit-Queue: David Roger <droger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574109}

components/signin/core/browser/chrome_connected_header_helper.cc

@@ -11,6 +11,7 @@
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "components/google/core/browser/google_util.h"
+#include "components/signin/core/browser/cookie_settings_util.h"
 #include "google_apis/gaia/gaia_auth_util.h"
 #include "net/base/registry_controlled_domains/registry_controlled_domain.h"
 #include "url/gurl.h"
@@ -94,6 +95,20 @@ ManageAccountsParams ChromeConnectedHeaderHelper::BuildManageAccountsParams(
   return params;
 }
 
+bool ChromeConnectedHeaderHelper::ShouldBuildRequestHeader(
+    const GURL& url,
+    const content_settings::CookieSettings* cookie_settings) {
+  // If signin cookies are not allowed, don't add the header.
+  if (!SettingsAllowSigninCookies(cookie_settings))
+    return false;
+
+  // Check if url is eligible for the header.
+  if (!IsUrlEligibleForRequestHeader(url))
+    return false;
+
+  return true;
+}
+
 bool ChromeConnectedHeaderHelper::IsUrlEligibleToIncludeGaiaId(
     const GURL& url,
     bool is_header_request) {

components/signin/core/browser/chrome_connected_header_helper.h

@@ -42,6 +42,11 @@ class ChromeConnectedHeaderHelper : public SigninHeaderHelper {
                                  const std::string& account_id,
                                  int profile_mode_mask);
 
+  // SigninHeaderHelper implementation:
+  bool ShouldBuildRequestHeader(
+      const GURL& url,
+      const content_settings::CookieSettings* cookie_settings) override;
+
  private:
   // Whether mirror account consistency should be used.
   AccountConsistencyMethod account_consistency_;

components/signin/core/browser/dice_header_helper.cc

@@ -181,6 +181,12 @@ DiceResponseParams DiceHeaderHelper::BuildDiceSignoutResponseParams(
   return params;
 }
 
+bool DiceHeaderHelper::ShouldBuildRequestHeader(
+    const GURL& url,
+    const content_settings::CookieSettings* cookie_settings) {
+  return IsUrlEligibleForRequestHeader(url);
+}
+
 bool DiceHeaderHelper::IsUrlEligibleForRequestHeader(const GURL& url) {
   if (account_consistency_ == AccountConsistencyMethod::kDisabled ||
       account_consistency_ == AccountConsistencyMethod::kMirror) {

components/signin/core/browser/dice_header_helper.h

@@ -44,6 +44,11 @@ class DiceHeaderHelper : public SigninHeaderHelper {
   std::string BuildRequestHeader(const std::string& sync_account_id,
                                  const std::string& device_id);
 
+  // SigninHeaderHelper implementation:
+  bool ShouldBuildRequestHeader(
+      const GURL& url,
+      const content_settings::CookieSettings* cookie_settings) override;
+
  private:
   // SigninHeaderHelper implementation:
   bool IsUrlEligibleForRequestHeader(const GURL& url) override;

components/signin/core/browser/signin_header_helper.cc

@@ -11,7 +11,6 @@
 #include "base/strings/string_split.h"
 #include "components/google/core/browser/google_util.h"
 #include "components/signin/core/browser/chrome_connected_header_helper.h"
-#include "components/signin/core/browser/cookie_settings_util.h"
 #include "google_apis/gaia/gaia_auth_util.h"
 #include "net/base/escape.h"
 #include "net/url_request/url_request.h"
@@ -139,20 +138,6 @@ SigninHeaderHelper::ParseAccountConsistencyResponseHeader(
   return dictionary;
 }
 
-bool SigninHeaderHelper::ShouldBuildRequestHeader(
-    const GURL& url,
-    const content_settings::CookieSettings* cookie_settings) {
-  // If signin cookies are not allowed, don't add the header.
-  if (!SettingsAllowSigninCookies(cookie_settings))
-    return false;
-
-  // Check if url is eligible for the header.
-  if (!IsUrlEligibleForRequestHeader(url))
-    return false;
-
-  return true;
-}
-
 void AppendOrRemoveMirrorRequestHeader(
     RequestAdapter* request,
     const GURL& redirect_url,

components/signin/core/browser/signin_header_helper.h

@@ -175,9 +175,9 @@ class SigninHeaderHelper {
 
   // Returns wether an account consistency header should be built for this
   // request.
-  bool ShouldBuildRequestHeader(
+  virtual bool ShouldBuildRequestHeader(
       const GURL& url,
-      const content_settings::CookieSettings* cookie_settings);
+      const content_settings::CookieSettings* cookie_settings) = 0;
 
  protected:
   SigninHeaderHelper() {}

components/signin/core/browser/signin_header_helper_unittest.cc

@@ -262,6 +262,21 @@ TEST_F(SigninHeaderHelperTest, TestDiceRequest) {
   CheckDiceHeaderRequest(GURL("https://www.google.com"), "0123456789", "", "");
 }
 
+// When cookies are blocked, only the Dice header is sent.
+TEST_F(SigninHeaderHelperTest, DiceCookiesBlocked) {
+  account_consistency_ = AccountConsistencyMethod::kDice;
+  cookie_settings_->SetDefaultCookieSetting(CONTENT_SETTING_BLOCK);
+
+  std::string client_id = GaiaUrls::GetInstance()->oauth2_chrome_client_id();
+  ASSERT_FALSE(client_id.empty());
+  CheckDiceHeaderRequest(
+      GURL("https://accounts.google.com"), "0123456789", "",
+      base::StringPrintf(
+          "version=%s,client_id=%s,device_id=DeviceID,signin_mode=all_accounts,"
+          "signout_mode=show_confirmation",
+          kDiceProtocolVersion, client_id.c_str()));
+}
+
 // Tests that no Dice request is returned when Dice is not enabled.
 TEST_F(SigninHeaderHelperTest, TestNoDiceRequestWhenDisabled) {
   account_consistency_ = AccountConsistencyMethod::kMirror;