Fix broken threading model in CheckDesktopNotificationPermission

Fix broken threading model in CheckDesktopNotificationPermission

Over time, this function (which is called on the IO thread) was updated
to use DesktopNotificationService.  While the methods in that service
were enforced to run on the correct thread, it's not possible to get
the notification service from the ProfileIOData safely.  This removes the
dependency on DesktopNotificationService from the IO-thread notification
functions.

BUG=256638
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=234886

[dewittj, 2013-11-05 23:52:30]

If you know anyone else that should review this, please suggest.  I'm not
terribly familiar with all the infrastructure surrounding the IO thread and
profiles etc so I expect that you'll have some nontrivial changes for me.

I solved this issue another way (mostly) in
https://codereview.chromium.org/25492006/ by making the response to
CheckNotificationPermission asynchronous, but this method is simpler (no changes
to Content API.)

[Matt Perry, 2013-11-06 00:57:18]

I think this makes sense. LGTM

https://codereview.chromium.org/61323002/diff/1/chrome/browser/extensions/ext...
File chrome/browser/extensions/extension_info_map.h (right):

https://codereview.chromium.org/61323002/diff/1/chrome/browser/extensions/ext...
chrome/browser/extensions/extension_info_map.h:92: bool
GetNotificationsDisabledForExtension(const std::string& extension_id)
nit: bool functions should read like a question: "AreNotificationsDisabled".

[dewittj, 2013-11-06 19:20:36]

Also added a test and updated existing unit tests with the new AddExtension
signature.

https://codereview.chromium.org/61323002/diff/1/chrome/browser/extensions/ext...
File chrome/browser/extensions/extension_info_map.h (right):

https://codereview.chromium.org/61323002/diff/1/chrome/browser/extensions/ext...
chrome/browser/extensions/extension_info_map.h:92: bool
GetNotificationsDisabledForExtension(const std::string& extension_id)
On 2013/11/06 00:57:18, Matt Perry wrote:
> nit: bool functions should read like a question: "AreNotificationsDisabled".

Done.

[Matt Perry, 2013-11-06 19:34:42]

lgtm

[awong, 2013-11-06 20:16:29]

ProfileIOData lgtm

[commit-bot: I haz the power, 2013-11-08 00:40:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dewittj@chromium.org/61323002/60001

[commit-bot: I haz the power, 2013-11-08 00:40:50]

Failed to apply patch for chrome/browser/chrome_content_browser_client.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file chrome/browser/chrome_content_browser_client.cc
  Hunk #1 FAILED at 1938.
  1 out of 1 hunk FAILED -- saving rejects to file
chrome/browser/chrome_content_browser_client.cc.rej

Patch:       chrome/browser/chrome_content_browser_client.cc
Index: chrome/browser/chrome_content_browser_client.cc
diff --git a/chrome/browser/chrome_content_browser_client.cc
b/chrome/browser/chrome_content_browser_client.cc
index
3255a210a4b86cedca12deb9f2ab6b88eb2fd1dc..a3b6385ae6a067f3ed4d60125e329c0aca4e735e
100644
--- a/chrome/browser/chrome_content_browser_client.cc
+++ b/chrome/browser/chrome_content_browser_client.cc
@@ -1938,31 +1938,37 @@ WebKit::WebNotificationPresenter::Permission
         int render_process_id) {
 #if defined(ENABLE_NOTIFICATIONS)
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  // Sometimes a notification may be invoked during the shutdown.
-  // See http://crbug.com/256638
-  if (browser_shutdown::IsTryingToQuit())
-    return WebKit::WebNotificationPresenter::PermissionNotAllowed;
 
   ProfileIOData* io_data = ProfileIOData::FromResourceContext(context);
-
-  DesktopNotificationService* notification_service =
-      io_data->GetNotificationService();
-  if (notification_service) {
-    ExtensionInfoMap* extension_info_map = io_data->GetExtensionInfoMap();
-    ExtensionSet extensions;
-    extension_info_map->GetExtensionsWithAPIPermissionForSecurityOrigin(
-        source_origin, render_process_id,
-        extensions::APIPermission::kNotification, &extensions);
-    for (ExtensionSet::const_iterator iter = extensions.begin();
-         iter != extensions.end(); ++iter) {
-      NotifierId notifier_id(NotifierId::APPLICATION, (*iter)->id());
-      if (notification_service->IsNotifierEnabled(notifier_id))
-        return WebKit::WebNotificationPresenter::PermissionAllowed;
-    }
-
-    return notification_service->HasPermission(source_origin);
-  }
-
+  ExtensionInfoMap* extension_info_map = io_data->GetExtensionInfoMap();
+
+  // We want to see if there is an extension that hasn't been manually disabled
+  // that has the notifications permission and applies to this security origin.
+  // First, get the list of extensions with permission for the origin.
+  ExtensionSet extensions;
+  extension_info_map->GetExtensionsWithAPIPermissionForSecurityOrigin(
+      source_origin, render_process_id,
+      extensions::APIPermission::kNotification, &extensions);
+  for (ExtensionSet::const_iterator iter = extensions.begin();
+       iter != extensions.end(); ++iter) {
+    // Then, check to see if it's been disabled by the user.
+    if (!extension_info_map->AreNotificationsDisabled((*iter)->id()))
+      return WebKit::WebNotificationPresenter::PermissionAllowed;
+  }
+
+  // No enabled extensions exist, so check the normal host content settings.
+  HostContentSettingsMap* host_content_settings_map =
+      io_data->GetHostContentSettingsMap();
+  ContentSetting setting = host_content_settings_map->GetContentSetting(
+      source_origin,
+      source_origin,
+      CONTENT_SETTINGS_TYPE_NOTIFICATIONS,
+      NO_RESOURCE_IDENTIFIER);
+
+  if (setting == CONTENT_SETTING_ALLOW)
+    return WebKit::WebNotificationPresenter::PermissionAllowed;
+  if (setting == CONTENT_SETTING_BLOCK)
+    return WebKit::WebNotificationPresenter::PermissionDenied;
   return WebKit::WebNotificationPresenter::PermissionNotAllowed;
 #else
   return WebKit::WebNotificationPresenter::PermissionAllowed;

[commit-bot: I haz the power, 2013-11-08 18:50:43]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dewittj@chromium.org/61323002/210001

[commit-bot: I haz the power, 2013-11-08 19:27:44]

Sorry for I got bad news for ya.
Compile failed with a clobber build on android_clang_dbg.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_cl...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-11-08 22:41:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dewittj@chromium.org/61323002/590001

[commit-bot: I haz the power, 2013-11-09 01:53:05]

Retried try job too often on win7_aura for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win7_aura&...

[commit-bot: I haz the power, 2013-11-11 17:08:44]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dewittj@chromium.org/61323002/590001

[commit-bot: I haz the power, 2013-11-11 17:08:54]

Failed to apply patch for chrome/browser/chrome_content_browser_client.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file chrome/browser/chrome_content_browser_client.cc
  Hunk #1 FAILED at 1937.
  1 out of 1 hunk FAILED -- saving rejects to file
chrome/browser/chrome_content_browser_client.cc.rej

Patch:       chrome/browser/chrome_content_browser_client.cc
Index: chrome/browser/chrome_content_browser_client.cc
diff --git a/chrome/browser/chrome_content_browser_client.cc
b/chrome/browser/chrome_content_browser_client.cc
index
b994e240c393e5553ec060bdfbcc6efb4e56d060..3205f9bc941d66eced326f3f2899a6462d3b6505
100644
--- a/chrome/browser/chrome_content_browser_client.cc
+++ b/chrome/browser/chrome_content_browser_client.cc
@@ -1937,31 +1937,37 @@ blink::WebNotificationPresenter::Permission
         int render_process_id) {
 #if defined(ENABLE_NOTIFICATIONS)
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  // Sometimes a notification may be invoked during the shutdown.
-  // See http://crbug.com/256638
-  if (browser_shutdown::IsTryingToQuit())
-    return blink::WebNotificationPresenter::PermissionNotAllowed;
 
   ProfileIOData* io_data = ProfileIOData::FromResourceContext(context);
-
-  DesktopNotificationService* notification_service =
-      io_data->GetNotificationService();
-  if (notification_service) {
-    ExtensionInfoMap* extension_info_map = io_data->GetExtensionInfoMap();
-    ExtensionSet extensions;
-    extension_info_map->GetExtensionsWithAPIPermissionForSecurityOrigin(
-        source_origin, render_process_id,
-        extensions::APIPermission::kNotification, &extensions);
-    for (ExtensionSet::const_iterator iter = extensions.begin();
-         iter != extensions.end(); ++iter) {
-      NotifierId notifier_id(NotifierId::APPLICATION, (*iter)->id());
-      if (notification_service->IsNotifierEnabled(notifier_id))
-        return blink::WebNotificationPresenter::PermissionAllowed;
-    }
-
-    return notification_service->HasPermission(source_origin);
-  }
-
+  ExtensionInfoMap* extension_info_map = io_data->GetExtensionInfoMap();
+
+  // We want to see if there is an extension that hasn't been manually disabled
+  // that has the notifications permission and applies to this security origin.
+  // First, get the list of extensions with permission for the origin.
+  ExtensionSet extensions;
+  extension_info_map->GetExtensionsWithAPIPermissionForSecurityOrigin(
+      source_origin, render_process_id,
+      extensions::APIPermission::kNotification, &extensions);
+  for (ExtensionSet::const_iterator iter = extensions.begin();
+       iter != extensions.end(); ++iter) {
+    // Then, check to see if it's been disabled by the user.
+    if (!extension_info_map->AreNotificationsDisabled((*iter)->id()))
+      return blink::WebNotificationPresenter::PermissionAllowed;
+  }
+
+  // No enabled extensions exist, so check the normal host content settings.
+  HostContentSettingsMap* host_content_settings_map =
+      io_data->GetHostContentSettingsMap();
+  ContentSetting setting = host_content_settings_map->GetContentSetting(
+      source_origin,
+      source_origin,
+      CONTENT_SETTINGS_TYPE_NOTIFICATIONS,
+      NO_RESOURCE_IDENTIFIER);
+
+  if (setting == CONTENT_SETTING_ALLOW)
+    return blink::WebNotificationPresenter::PermissionAllowed;
+  if (setting == CONTENT_SETTING_BLOCK)
+    return blink::WebNotificationPresenter::PermissionDenied;
   return blink::WebNotificationPresenter::PermissionNotAllowed;
 #else
   return blink::WebNotificationPresenter::PermissionAllowed;

[commit-bot: I haz the power, 2013-11-11 18:59:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dewittj@chromium.org/61323002/1100001

[commit-bot: I haz the power, 2013-11-11 19:29:13]

Sorry for I got bad news for ya.
Compile failed with a clobber build on android_clang_dbg.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_cl...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-11-13 17:49:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dewittj@chromium.org/61323002/1370001

[commit-bot: I haz the power, 2013-11-13 20:23:34]

Change committed as 234886