Simplify permission-related code for Web Notifications.

chrome/browser/notifications/notification_permission_context.cc

Index: chrome/browser/notifications/notification_permission_context.cc
diff --git a/chrome/browser/notifications/notification_permission_context.cc b/chrome/browser/notifications/notification_permission_context.cc
new file mode 100644
index 0000000000000000000000000000000000000000..ce5182871ac41f9e67624b89a82e2f7d24606cab
--- /dev/null
+++ b/chrome/browser/notifications/notification_permission_context.cc
@@ -0,0 +1,37 @@
+// Copyright 2015 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/notification_permission_context.h"
+
+#include "chrome/browser/notifications/desktop_notification_profile_util.h"
+
+NotificationPermissionContext::NotificationPermissionContext(Profile* profile)
+    : PermissionContextBase(profile, CONTENT_SETTINGS_TYPE_NOTIFICATIONS) {}
+
+NotificationPermissionContext::~NotificationPermissionContext() {}
+
+// Unlike other permission types, granting a notification for a given origin
+// will not take into account the |embedder_origin|, it will only be based
+// on the requesting iframe origin.
+// TODO(mukai) Consider why notifications behave differently than
+// other permissions. crbug.com/416894

[mlamouri (slow - plz ping), 2015/06/29 10:57:42]

nit: add "https://"

[Peter Beverloo, 2015/06/29 12:54:21]

Done.

+void NotificationPermissionContext::UpdateContentSetting(
+    const GURL& requesting_origin,
+    const GURL& embedder_origin,
+    ContentSetting content_setting) {
+  DCHECK(content_setting == CONTENT_SETTING_ALLOW ||
+         content_setting == CONTENT_SETTING_BLOCK);
+
+  if (content_setting == CONTENT_SETTING_ALLOW) {
+    DesktopNotificationProfileUtil::GrantPermission(profile(),

[dewittj, 2015/06/26 16:37:15]

I get that it's correct, but from a readability perspective it's a little
unclear that GrantPermission/DenyPermission need to be compatible with
PermissionContextBase::ResetPermission, which is not overridden by this class. 
It'd be clearer if {Grant,Deny}Permission were a part of this class; did you
explore moving that out of DesktopNotificationProfileUtil?

[Peter Beverloo, 2015/06/26 17:42:44]

Yes, in removing all the DesktopNotification* bits the util class will
definitely go :-).

I'd like to do so in a separate CL though, since it'll be a clean-up by itself
and might involve some logic changes. We still have too many tests that actually
use an infobar/bubble for permission requests, for example, which could easily
be switched over to changing the content setting.

+                                                    requesting_origin);
+  } else {
+    DesktopNotificationProfileUtil::DenyPermission(profile(),

[mlamouri (slow - plz ping), 2015/06/29 10:57:42]

I think we could get ride of or at least greatly reduce
DesktopNotificationProfile: all the permission work could be done via the
NotificationPermissionContext instead of using the
DesktopNotificationProfileUtil. Could you file a follow-up for that?

[Peter Beverloo, 2015/06/29 12:54:21]

As replied to Jusin - that'll be done in a follow up.

+                                                   requesting_origin);
+  }
+}
+
+bool NotificationPermissionContext::IsRestrictedToSecureOrigins() const {
+  return false;
+}