Allow getting both reading/setting cookie settings at one time

components/content_settings/core/browser/cookie_settings.cc

Index: components/content_settings/core/browser/cookie_settings.cc
diff --git a/components/content_settings/core/browser/cookie_settings.cc b/components/content_settings/core/browser/cookie_settings.cc
index b5db52bd2581bba29c2a8b508f794ab8f945db67..7a6000fcb914f491906533adb6d43f2bd45ac2bf 100644
--- a/components/content_settings/core/browser/cookie_settings.cc
+++ b/components/content_settings/core/browser/cookie_settings.cc
@@ -58,18 +58,36 @@ ContentSetting CookieSettings::GetDefaultCookieSetting(
 
 bool CookieSettings::IsReadingCookieAllowed(const GURL& url,
                                             const GURL& first_party_url) const {
-  ContentSetting setting = GetCookieSetting(url, first_party_url, false, NULL);
-  return IsAllowed(setting);
+  ContentSetting reading_setting;
+  GetCookieSetting(url, first_party_url, nullptr, &reading_setting,
+                   nullptr /* setting_cookie */);
+  return IsAllowed(reading_setting);
 }
 
 bool CookieSettings::IsSettingCookieAllowed(const GURL& url,
                                             const GURL& first_party_url) const {
-  ContentSetting setting = GetCookieSetting(url, first_party_url, true, NULL);
-  return IsAllowed(setting);
+  ContentSetting setting_setting;
+  GetCookieSetting(url, first_party_url, nullptr, nullptr /* reading_cookie */,
+                   &setting_setting);
+  return IsAllowed(setting_setting);
+}
+
+void CookieSettings::GetReadingAndSettingCookieAllowed(
+    const GURL& url,
+    const GURL& first_party_url,
+    bool* reading_cookie_allowed,
+    bool* setting_cookie_allowed) const {
+  ContentSetting reading_setting;
+  ContentSetting setting_setting;
+  GetCookieSetting(url, first_party_url, nullptr, &reading_setting,
+                   &setting_setting);
+  *reading_cookie_allowed = IsAllowed(reading_setting);
+  *setting_cookie_allowed = IsAllowed(setting_setting);
 }
 
 bool CookieSettings::IsCookieSessionOnly(const GURL& origin) const {
-  ContentSetting setting = GetCookieSetting(origin, origin, true, NULL);
+  ContentSetting setting;
+  GetCookieSetting(origin, origin, nullptr, nullptr, &setting);
   DCHECK(IsValidSetting(setting));
   return (setting == CONTENT_SETTING_SESSION_ONLY);
 }
@@ -122,18 +140,29 @@ void CookieSettings::ShutdownOnUIThread() {
   pref_change_registrar_.RemoveAll();
 }
 
-ContentSetting CookieSettings::GetCookieSetting(const GURL& url,
-                                                const GURL& first_party_url,
-                                                bool setting_cookie,
-                                                SettingSource* source) const {
+void CookieSettings::GetCookieSetting(const GURL& url,
+                                      const GURL& first_party_url,
+                                      content_settings::SettingSource* source,
+                                      ContentSetting* reading_cookie,
+                                      ContentSetting* setting_cookie) const {
   // Auto-allow in extensions or for WebUI embedded in a secure origin.
-  if (url.SchemeIsCryptographic() && first_party_url.SchemeIs(kChromeUIScheme))
-    return CONTENT_SETTING_ALLOW;
+  if (url.SchemeIsCryptographic() &&
+      first_party_url.SchemeIs(kChromeUIScheme)) {

[mmenke, 2016/11/16 17:37:44]

Shouldn't it make sense to flip these?  Not a huge per difference, but
SchemeIsCryptographic does 3 comparisons, and probably returns true more often
than the second check, so we'll often have to do 4 string compares.  Flip the
order, and we're usually doing just 1.

Probably not a huge difference, as I assume the "GetWebsiteSetting" and
"StaticCookiePolicy" calls are what really matters.

[Charlie Harrison, 2016/11/16 17:50:03]

Swapped them. Sometimes string comparisons / memcmps lose their stacks with the
profiler toolchain I'm using so it's a bit of a blindspot. Better safe than
sorry!

+    if (reading_cookie)
+      *reading_cookie = CONTENT_SETTING_ALLOW;
+    if (setting_cookie)
+      *setting_cookie = CONTENT_SETTING_ALLOW;
+    return;
+  }
 
 #if BUILDFLAG(ENABLE_EXTENSIONS)
   if (url.SchemeIs(kExtensionScheme) &&
       first_party_url.SchemeIs(kExtensionScheme)) {
-    return CONTENT_SETTING_ALLOW;
+    if (reading_cookie)
+      *reading_cookie = CONTENT_SETTING_ALLOW;
+    if (setting_cookie)
+      *setting_cookie = CONTENT_SETTING_ALLOW;
+    return;
   }
 #endif
 
@@ -147,26 +176,27 @@ ContentSetting CookieSettings::GetCookieSetting(const GURL& url,
     *source = info.source;
 
   // If no explicit exception has been made and third-party cookies are blocked
-  // by default, apply that rule.
-  if (info.primary_pattern.MatchesAllHosts() &&
-      info.secondary_pattern.MatchesAllHosts() &&
-      ShouldBlockThirdPartyCookies() &&
-      !first_party_url.SchemeIs(extension_scheme_)) {
-    net::StaticCookiePolicy policy(
-        net::StaticCookiePolicy::BLOCK_ALL_THIRD_PARTY_COOKIES);
-    int rv;
-    if (setting_cookie)
-      rv = policy.CanSetCookie(url, first_party_url);
-    else
-      rv = policy.CanGetCookies(url, first_party_url);
-    DCHECK_NE(net::ERR_IO_PENDING, rv);
-    if (rv != net::OK)
-      return CONTENT_SETTING_BLOCK;
-  }
+  // by default, apply CONTENT_SETTING_BLOCKED.
+  bool block_third = info.primary_pattern.MatchesAllHosts() &&
+                     info.secondary_pattern.MatchesAllHosts() &&
+                     ShouldBlockThirdPartyCookies() &&
+                     !first_party_url.SchemeIs(extension_scheme_);
+  net::StaticCookiePolicy policy(
+      net::StaticCookiePolicy::BLOCK_ALL_THIRD_PARTY_COOKIES);
 
   // We should always have a value, at least from the default provider.
   DCHECK(value.get());
-  return ValueToContentSetting(value.get());
+  ContentSetting setting = ValueToContentSetting(value.get());
+  if (reading_cookie) {
+    bool block =
+        block_third && policy.CanGetCookies(url, first_party_url) != net::OK;
+    *reading_cookie = block ? CONTENT_SETTING_BLOCK : setting;
+  }
+  if (setting_cookie) {
+    bool block =
+        block_third && policy.CanSetCookie(url, first_party_url) != net::OK;
+    *setting_cookie = block ? CONTENT_SETTING_BLOCK : setting;
+  }
 }
 
 CookieSettings::~CookieSettings() {