Fix ClearOnExit for content settings with secondary pattern

Currently a <URL>,* exception can control whether cookies are deleted on
exit. Exceptions that only specify a secondary pattern are ignored.

While we don't know the top-frame-origin, it would still be useful to
keep cookies from being deleted that match the url in *,<URL> pattern.

Bug: 967668
Change-Id: I98ad6f1a2d77f53c39f7e04f160e40eec2e18b06
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1807246
Commit-Queue: Christian Dullweber <dullweber@chromium.org>
Reviewed-by: Martin Šrámek <msramek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#697952}

components/content_settings/core/common/cookie_settings_base.cc

@@ -41,7 +41,12 @@ bool CookieSettingsBase::ShouldDeleteCookieOnExit(
   // Check if there is a more precise rule that "domain matches" this cookie.
   bool matches_session_only_rule = false;
   for (const auto& entry : cookie_settings) {
-    const std::string& host = entry.primary_pattern.GetHost();
+    // While we don't know on which top-frame-origin a cookie was set, we still
+    // use exceptions that only specify a secondary pattern to handle cookies
+    // that match this pattern.
+    const std::string& host = entry.primary_pattern.MatchesAllHosts()
+                                  ? entry.secondary_pattern.GetHost()
+                                  : entry.primary_pattern.GetHost();
     if (net::cookie_util::IsDomainMatch(domain, host)) {
       if (entry.GetContentSetting() == CONTENT_SETTING_ALLOW) {
         return false;

components/content_settings/core/common/cookie_settings_base_unittest.cc

@@ -23,6 +23,13 @@ ContentSettingPatternSource CreateSetting(ContentSetting setting) {
       false);
 }
 
+ContentSettingPatternSource CreateThirdPartySetting(ContentSetting setting) {
+  return ContentSettingPatternSource(
+      ContentSettingsPattern::Wildcard(),
+      ContentSettingsPattern::FromString(kDomain), base::Value(setting),
+      std::string(), false);
+}
+
 class CallbackCookieSettings : public CookieSettingsBase {
  public:
   explicit CallbackCookieSettings(GetSettingCallback callback)
@@ -73,6 +80,13 @@ TEST(CookieSettingsBaseTest, ShouldDeleteDomainSettingSessionOnly) {
       {CreateSetting(CONTENT_SETTING_SESSION_ONLY)}, kDomain, false));
 }
 
+TEST(CookieSettingsBaseTest, ShouldDeleteDomainThirdPartySettingSessionOnly) {
+  CallbackCookieSettings settings(
+      base::BindRepeating([](const GURL&) { return CONTENT_SETTING_BLOCK; }));
+  EXPECT_TRUE(settings.ShouldDeleteCookieOnExit(
+      {CreateThirdPartySetting(CONTENT_SETTING_SESSION_ONLY)}, kDomain, false));
+}
+
 TEST(CookieSettingsBaseTest, ShouldNotDeleteDomainSettingAllow) {
   CallbackCookieSettings settings(
       base::BindRepeating([](const GURL&) { return CONTENT_SETTING_BLOCK; }));
@@ -104,6 +118,14 @@ TEST(CookieSettingsBaseTest, ShouldNotDeleteNoDomainMatch) {
       {CreateSetting(CONTENT_SETTING_SESSION_ONLY)}, "other.com", false));
 }
 
+TEST(CookieSettingsBaseTest, ShouldNotDeleteNoThirdPartyDomainMatch) {
+  CallbackCookieSettings settings(
+      base::BindRepeating([](const GURL&) { return CONTENT_SETTING_BLOCK; }));
+  EXPECT_FALSE(settings.ShouldDeleteCookieOnExit(
+      {CreateThirdPartySetting(CONTENT_SETTING_SESSION_ONLY)}, "other.com",
+      false));
+}
+
 TEST(CookieSettingsBaseTest, CookieAccessNotAllowedWithBlockedSetting) {
   CallbackCookieSettings settings(
       base::BindRepeating([](const GURL&) { return CONTENT_SETTING_BLOCK; }));