Use GURLs instead of patterns in SetContentSetting in notifications

chrome/browser/notifications/message_center_settings_controller.cc

 // Copyright (c) 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/notifications/message_center_settings_controller.h"
 
 #include <algorithm>
 #include <string>
 #include <utility>
 
@@ skipping 229 matching lines @@
     app_icon_loader_->FetchImage(extension->id());
   }
 
   ContentSettingsForOneType settings;
   DesktopNotificationProfileUtil::GetNotificationsSettings(profile, &settings);
 
   favicon::FaviconService* favicon_service =
       FaviconServiceFactory::GetForProfile(profile,
                                            ServiceAccessType::EXPLICIT_ACCESS);
   favicon_tracker_.reset(new base::CancelableTaskTracker());
-  patterns_.clear();
   for (ContentSettingsForOneType::const_iterator iter = settings.begin();
        iter != settings.end(); ++iter) {
     if (iter->primary_pattern == ContentSettingsPattern::Wildcard() &&
         iter->secondary_pattern == ContentSettingsPattern::Wildcard() &&
         iter->source != "preference") {
       continue;
     }
 
     std::string url_pattern = iter->primary_pattern.ToString();
     base::string16 name = base::UTF8ToUTF16(url_pattern);
     GURL url(url_pattern);
     NotifierId notifier_id(url);
     notifiers->push_back(new Notifier(
         notifier_id,
         name,
         notifier_state_tracker->IsNotifierEnabled(notifier_id)));
-    patterns_[name] = iter->primary_pattern;
     // Note that favicon service obtains the favicon from history. This means
     // that it will fail to obtain the image if there are no history data for
     // that URL.
     favicon_service->GetFaviconImageForPageURL(
         url,
         base::Bind(&MessageCenterSettingsController::OnFaviconLoaded,
                    base::Unretained(this),
                    url),
         favicon_tracker_.get());
   }
@@ skipping 39 matching lines @@
     DCHECK(default_setting == CONTENT_SETTING_ALLOW ||
            default_setting == CONTENT_SETTING_BLOCK ||
            default_setting == CONTENT_SETTING_ASK);
 
     // The content setting for notifications needs to clear when it changes to
     // the default value or get explicitly set when it differs from the default.
     bool differs_from_default_value =
         (default_setting != CONTENT_SETTING_ALLOW && enabled) ||
         (default_setting == CONTENT_SETTING_ALLOW && !enabled);
 
+    if (!notifier.notifier_id.url.is_valid()) {
+      LOG(ERROR) << "Invalid url pattern: " << notifier.notifier_id.url.spec();
+    }
+
     if (differs_from_default_value) {
-      if (notifier.notifier_id.url.is_valid()) {
         if (enabled) {
           DesktopNotificationProfileUtil::GrantPermission(
               profile, notifier.notifier_id.url);
         } else {
           DesktopNotificationProfileUtil::DenyPermission(
               profile, notifier.notifier_id.url);
         }
-      } else {
-        LOG(ERROR) << "Invalid url pattern: "
-                   << notifier.notifier_id.url.spec();
-      }
     } else {
-      ContentSettingsPattern pattern;
-
-      const auto& iter = patterns_.find(notifier.name);
-      if (iter != patterns_.end()) {
-        pattern = iter->second;
-      } else if (notifier.notifier_id.url.is_valid()) {
-        pattern =
-            ContentSettingsPattern::FromURLNoWildcard(notifier.notifier_id.url);
-      } else {
-        LOG(ERROR) << "Invalid url pattern: "
-                   << notifier.notifier_id.url.spec();
-      }
-
-      if (pattern.IsValid())
-        DesktopNotificationProfileUtil::ClearSetting(profile, pattern);

[raymes, 2016/03/09 06:53:48]

Hey Liu, I don't fully understand this code but it looks like we might have to
keep it in place. I'm not sure if all the settings here are guaranteed to be
scoped in the way we expect :(. We can chat more tomorrow.

+      DesktopNotificationProfileUtil::ClearSetting(profile,
+                                                   notifier.notifier_id.url);
     }
   } else {
     NotifierStateTrackerFactory::GetForProfile(profile)
         ->SetNotifierEnabled(notifier.notifier_id, enabled);
   }
   FOR_EACH_OBSERVER(message_center::NotifierSettingsObserver,
                     observers_,
                     NotifierEnabledChanged(notifier.notifier_id, enabled));
 }
 
 void MessageCenterSettingsController::OnNotifierSettingsClosing() {
   DCHECK(favicon_tracker_.get());
   favicon_tracker_->TryCancelAll();
-  patterns_.clear();
 }
 
 bool MessageCenterSettingsController::NotifierHasAdvancedSettings(
     const NotifierId& notifier_id) const {
   // TODO(dewittj): Refactor this so that notifiers have a delegate that can
   // handle this in a more appropriate location.
   if (notifier_id.type != NotifierId::APPLICATION)
     return false;
 
   const std::string& extension_id = notifier_id.id;
@@ skipping 176 matching lines @@
             weak_factory_.GetWeakPtr()));
   }
 #endif
 
   if (notify) {
     FOR_EACH_OBSERVER(message_center::NotifierSettingsObserver,
                       observers_,
                       NotifierGroupChanged());
   }
 }