Allow getting both reading/setting cookie settings at one time

components/content_settings/core/browser/cookie_settings.cc

 // Copyright (c) 2012 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 "components/content_settings/core/browser/cookie_settings.h"
 
 #include "base/bind.h"
 #include "base/logging.h"
 #include "components/content_settings/core/browser/content_settings_utils.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
@@ skipping 40 matching lines @@
 }
 
 ContentSetting CookieSettings::GetDefaultCookieSetting(
     std::string* provider_id) const {
   return host_content_settings_map_->GetDefaultContentSetting(
       CONTENT_SETTINGS_TYPE_COOKIES, provider_id);
 }
 
 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);
 }
 
 void CookieSettings::GetCookieSettings(
     ContentSettingsForOneType* settings) const {
   host_content_settings_map_->GetSettingsForOneType(
       CONTENT_SETTINGS_TYPE_COOKIES, std::string(), settings);
 }
 
@@ skipping 32 matching lines @@
       CONTENT_SETTINGS_TYPE_DURABLE_STORAGE,
       std::string() /*resource_identifier*/);
   return setting == CONTENT_SETTING_ALLOW;
 }
 
 void CookieSettings::ShutdownOnUIThread() {
   DCHECK(thread_checker_.CalledOnValidThread());
   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
 
   // First get any host-specific settings.
   SettingInfo info;
   std::unique_ptr<base::Value> value =
       host_content_settings_map_->GetWebsiteSetting(
           url, first_party_url, CONTENT_SETTINGS_TYPE_COOKIES, std::string(),
           &info);
   if (source)
     *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() {
 }
 
 void CookieSettings::OnBlockThirdPartyCookiesChanged() {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   base::AutoLock auto_lock(lock_);
   block_third_party_cookies_ = pref_change_registrar_.prefs()->GetBoolean(
       prefs::kBlockThirdPartyCookies);
 }
 
 bool CookieSettings::ShouldBlockThirdPartyCookies() const {
   base::AutoLock auto_lock(lock_);
   return block_third_party_cookies_;
 }
 
 }  // namespace content_settings