Make it possible to revoke Web Notification exceptions in content settings.

chrome/browser/ui/webui/options/content_settings_handler.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 "chrome/browser/ui/webui/options/content_settings_handler.h"
 
 #include <algorithm>
 #include <map>
 #include <vector>
 
@@ skipping 156 matching lines @@
   exception->SetString(kSetting, ContentSettingToString(setting));
   exception->SetString(kOrigin, origin.ToString());
   exception->SetString(kEmbeddingOrigin, embedding_origin.ToString());
   return exception;
 }
 
 // Create a DictionaryValue* that will act as a data source for a single row
 // in the desktop notifications exceptions table. Ownership of the pointer is
 // passed to the caller.
 base::DictionaryValue* GetNotificationExceptionForPage(
-    const ContentSettingsPattern& pattern,
+    const ContentSettingsPattern& primary_pattern,
+    const ContentSettingsPattern& secondary_pattern,
     ContentSetting setting,
     const std::string& provider_name) {
   base::DictionaryValue* exception = new base::DictionaryValue();
   exception->SetString(kSetting, ContentSettingToString(setting));
-  exception->SetString(kOrigin, pattern.ToString());
+  exception->SetString(kOrigin, primary_pattern.ToString());
+  exception->SetString(kEmbeddingOrigin,
+                       secondary_pattern == ContentSettingsPattern::Wildcard() ?
+                           std::string() :

[Bernhard Bauer, 2014/09/11 14:56:35]

Nit: I would probably use a local variable for this and conditionally assign it.

[Peter Beverloo, 2014/09/11 15:30:43]

Done.

+                           secondary_pattern.ToString());
   exception->SetString(kSource, provider_name);
   return exception;
 }
 
 // Returns true whenever the |extension| is hosted and has |permission|.
 // Must have the AppFilter signature.
 template <APIPermission::ID permission>
 bool HostedAppHasPermission(const extensions::Extension& extension,
                             content::BrowserContext* /* context */) {
   return extension.is_hosted_app() &&
@@ skipping 618 matching lines @@
   base::StringValue type_string(
       ContentSettingsTypeToGroupName(CONTENT_SETTINGS_TYPE_GEOLOCATION));
   web_ui()->CallJavascriptFunction("ContentSettings.setExceptions",
                                    type_string, exceptions);
 
   // This is mainly here to keep this function ideologically parallel to
   // UpdateExceptionsViewFromHostContentSettingsMap().
   UpdateSettingDefaultFromModel(CONTENT_SETTINGS_TYPE_GEOLOCATION);
 }
 
 void ContentSettingsHandler::UpdateNotificationExceptionsView() {

[Bernhard Bauer, 2014/09/11 14:56:35]

Could we do this with the regular UpdateExceptionsViewFromHostContentSettingsMap
method now?

[Peter Beverloo, 2014/09/11 15:30:43]

I am not sure. We also add exceptions granted to hosted apps for notifications
and geolocation as part of UpdateNotificationExceptionsView(), which I don't see
in any of the other code paths.

That said, the content settings work for permissions granted or denied to normal
webpages when I remove all the Notification special-casing.

It would be great to understand why geolocation and notifications are different
from other content settings. We can change behavior if we have to (and where it
makes sense) to align much more closely.

[Bernhard Bauer, 2014/09/11 15:52:43]

AFAICT, mostly historical reasons.
+1.

   Profile* profile = Profile::FromWebUI(web_ui());
   ContentSettingsForOneType settings;
   DesktopNotificationProfileUtil::GetNotificationsSettings(profile, &settings);
 
   base::ListValue exceptions;
   AddExceptionsGrantedByHostedApps(
       profile,
       HostedAppHasPermission<APIPermission::kNotifications>,
       &exceptions);
 
   for (ContentSettingsForOneType::const_iterator i =
            settings.begin();
        i != settings.end();
        ++i) {
     // Don't add default settings.
     if (i->primary_pattern == ContentSettingsPattern::Wildcard() &&
         i->secondary_pattern == ContentSettingsPattern::Wildcard() &&
         i->source != kPreferencesSource) {
       continue;
     }
 
     exceptions.Append(
-        GetNotificationExceptionForPage(i->primary_pattern, i->setting,
+        GetNotificationExceptionForPage(i->primary_pattern,
+                                        i->secondary_pattern,
+                                        i->setting,
                                         i->source));
   }
 
   base::StringValue type_string(
       ContentSettingsTypeToGroupName(CONTENT_SETTINGS_TYPE_NOTIFICATIONS));
   web_ui()->CallJavascriptFunction("ContentSettings.setExceptions",
                                    type_string, exceptions);
 
   // This is mainly here to keep this function ideologically parallel to
   // UpdateExceptionsViewFromHostContentSettingsMap().
@@ skipping 265 matching lines @@
     }
   }
 
   for (size_t i = 0; i < all_provider_exceptions.size(); ++i) {
     for (size_t j = 0; j < all_provider_exceptions[i].size(); ++j) {
       exceptions->Append(all_provider_exceptions[i][j]);
     }
   }
 }
 
-void ContentSettingsHandler::RemoveNotificationException(
-    const base::ListValue* args) {
-  Profile* profile = Profile::FromWebUI(web_ui());
-
-  std::string origin;
-  std::string setting;
-  bool rv = args->GetString(1, &origin);
-  DCHECK(rv);
-  rv = args->GetString(2, &setting);
-  DCHECK(rv);
-  ContentSetting content_setting = ContentSettingFromString(setting);
-
-  DCHECK(content_setting == CONTENT_SETTING_ALLOW ||
-         content_setting == CONTENT_SETTING_BLOCK);
-  DesktopNotificationProfileUtil::ClearSetting(profile,
-      ContentSettingsPattern::FromString(origin));
-}
-
 void ContentSettingsHandler::RemoveMediaException(const base::ListValue* args) {
   std::string mode;
   bool rv = args->GetString(1, &mode);
   DCHECK(rv);
 
   std::string pattern;
   rv = args->GetString(2, &pattern);
   DCHECK(rv);
 
   HostContentSettingsMap* settings_map =
@@ skipping 162 matching lines @@
 
   // Zoom levels are no actual content type so we need to handle them
   // separately. They would not be recognized by
   // ContentSettingsTypeFromGroupName.
   if (type_string == kZoomContentType) {
     RemoveZoomLevelException(args);
     return;
   }
 
   ContentSettingsType type = ContentSettingsTypeFromGroupName(type_string);
-  switch (type) {
-    case CONTENT_SETTINGS_TYPE_NOTIFICATIONS:
-      RemoveNotificationException(args);
-      break;
-    case CONTENT_SETTINGS_TYPE_MEDIASTREAM:
-      RemoveMediaException(args);
-      break;
-    default:
-      RemoveExceptionFromHostContentSettingsMap(args, type);
-      break;
-  }
+  if (type == CONTENT_SETTINGS_TYPE_MEDIASTREAM)
+    RemoveMediaException(args);
+  else
+    RemoveExceptionFromHostContentSettingsMap(args, type);
 }
 
 void ContentSettingsHandler::SetException(const base::ListValue* args) {
   std::string type_string;
   CHECK(args->GetString(0, &type_string));
   std::string mode;
   CHECK(args->GetString(1, &mode));
   std::string pattern;
   CHECK(args->GetString(2, &pattern));
   std::string setting;
@@ skipping 158 matching lines @@
 void ContentSettingsHandler::UpdateProtectedContentExceptionsButton() {
   PrefService* prefs = user_prefs::UserPrefs::Get(GetBrowserContext(web_ui()));
   // Exceptions apply only when the feature is enabled.
   bool enable_exceptions = prefs->GetBoolean(prefs::kEnableDRM);
   web_ui()->CallJavascriptFunction(
       "ContentSettings.enableProtectedContentExceptions",
       base::FundamentalValue(enable_exceptions));
 }
 
 }  // namespace options