Restore the iframe behavior of notification permissions

chrome/browser/notifications/desktop_notification_service.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/notifications/desktop_notification_service.h"
 
 #include "base/bind.h"
 #include "base/metrics/histogram.h"
 #include "base/prefs/scoped_user_pref_update.h"
 #include "base/strings/utf_string_conversions.h"
@@ skipping 295 matching lines @@
     return;
 
   // The settings for ephemeral apps will be persisted across cache evictions.
   if (extensions::util::IsEphemeralApp(extension->id(), profile_))
     return;
 
   SetNotifierEnabled(notifier_id, true);
 #endif
 }
 
+// Unlike other permission types, granting a notification for a given origin
+// will grant it even if it is embedded as an iframe in other domains.

[Peter Beverloo, 2014/09/23 15:58:47]

This is phrased a bit ambiguously, what about the following?

Unlike other permission types, granting the notification permission to the
origin of an embedded frame will persist when the origin is loaded autonomously,
or as part of another embedding origin.

[Bernhard Bauer, 2014/09/23 16:13:02]

I would point out in the comment that the embedding origin simply doesn't
matter. Permissions are granted purely based on the requesting iframe origin,
which is why we override this method to ignore |embedder_origin|.

Also, the bikeshed should be red.

[Miguel Garcia, 2014/09/23 16:53:38]

Acknowledged.

[Miguel Garcia, 2014/09/23 16:53:39]

Done.

+void DesktopNotificationService::UpdateContentSetting(
+    const GURL& requesting_origin,
+    const GURL& embedder_origin,
+    bool allowed) {
+  if (allowed)

[Bernhard Bauer, 2014/09/23 16:13:02]

Braces can only be omitted for single-line bodies.

[Miguel Garcia, 2014/09/23 16:53:39]

Done.

+    DesktopNotificationProfileUtil::GrantPermission(
+        profile_, requesting_origin);
+  else
+    DesktopNotificationProfileUtil::DenyPermission(profile_, requesting_origin);
+}
+

[Peter Beverloo, 2014/09/23 15:58:47]

nit: only a single blank line.

[Miguel Garcia, 2014/09/23 16:53:39]

Done.

+
+
 void DesktopNotificationService::OnNotificationPermissionRequested(
     const NotificationPermissionCallback& callback, bool allowed) {
   blink::WebNotificationPermission permission = allowed ?
       blink::WebNotificationPermissionAllowed :
       blink::WebNotificationPermissionDenied;
 
   callback.Run(permission);
 }
 
 void DesktopNotificationService::FirePermissionLevelChangedEvent(
@@ skipping 15 matching lines @@
   // Tell the IO thread that this extension's permission for notifications
   // has changed.
   extensions::InfoMap* extension_info_map =
       extensions::ExtensionSystem::Get(profile_)->info_map();
   BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE,
       base::Bind(&extensions::InfoMap::SetNotificationsDisabled,
                  extension_info_map, notifier_id.id, !enabled));
 #endif
 }