Adds a context message of the security origin for web notifications.

chrome/browser/notifications/platform_notification_service_impl.cc

 // Copyright 2014 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/platform_notification_service_impl.h"
 
+#include "base/prefs/pref_service.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/notifications/notification_object_proxy.h"
 #include "chrome/browser/notifications/notification_ui_manager.h"
 #include "chrome/browser/notifications/persistent_notification_delegate.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_io_data.h"
+#include "chrome/common/pref_names.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "components/content_settings/core/common/content_settings.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/desktop_notification_delegate.h"
 #include "content/public/browser/notification_event_dispatcher.h"
 #include "content/public/common/platform_notification_data.h"
+#include "net/base/net_util.h"
 #include "ui/message_center/notifier_settings.h"
 
 #if defined(ENABLE_EXTENSIONS)
 #include "chrome/browser/notifications/desktop_notification_service.h"
 #include "chrome/browser/notifications/desktop_notification_service_factory.h"
 #include "extensions/browser/extension_system.h"
 #include "extensions/browser/info_map.h"
 #include "extensions/common/constants.h"
 #include "extensions/common/extension_set.h"
 #include "extensions/common/permissions/api_permission.h"
@@ skipping 162 matching lines @@
       profile, origin, render_process_id);
 
   // TODO(peter): Icons for Web Notifications are currently always requested for
   // 1x scale, whereas the displays on which they can be displayed can have a
   // different pixel density. Be smarter about this when the API gets updated
   // with a way for developers to specify images of different resolutions.
   Notification notification(origin, notification_data.title,
       notification_data.body, gfx::Image::CreateFrom1xBitmap(icon),
       display_source, notification_data.tag, delegate);
 
+  notification.set_context_message(display_source);
+
   // Web Notifications do not timeout.
   notification.set_never_timeout(true);
 
   return notification;
 }
 
 NotificationUIManager*
 PlatformNotificationServiceImpl::GetNotificationUIManager() const {
   if (notification_ui_manager_for_tests_)
     return notification_ui_manager_for_tests_;
 
   return g_browser_process->notification_ui_manager();
 }
 
 void PlatformNotificationServiceImpl::SetNotificationUIManagerForTesting(
     NotificationUIManager* manager) {
   notification_ui_manager_for_tests_ = manager;
 }
 
 base::string16 PlatformNotificationServiceImpl::DisplayNameForOriginInProcessId(
-    Profile* profile, const GURL& origin, int process_id) const {
+    Profile* profile,
+    const GURL& origin,
+    int process_id) const {
 #if defined(ENABLE_EXTENSIONS)
   // If the source is an extension, lookup the display name.
   if (origin.SchemeIs(extensions::kExtensionScheme)) {
-    extensions::InfoMap* extension_info_map =
-        extensions::ExtensionSystem::Get(profile)->info_map();
-    if (extension_info_map) {
-      extensions::ExtensionSet extensions;
-      extension_info_map->GetExtensionsWithAPIPermissionForSecurityOrigin(
-          origin,
-          process_id,
-          extensions::APIPermission::kNotifications,
-          &extensions);
-      DesktopNotificationService* desktop_notification_service =
-          DesktopNotificationServiceFactory::GetForProfile(profile);
-      DCHECK(desktop_notification_service);
-
-      for (const auto& extension : extensions) {
-        NotifierId notifier_id(NotifierId::APPLICATION, extension->id());
-        if (desktop_notification_service->IsNotifierEnabled(notifier_id))
-          return base::UTF8ToUTF16(extension->name());
-      }
+    base::string16 extension_display_name;
+    if (ExtensionDisplayName(profile, origin, process_id,
+                             &extension_display_name)) {
+      return extension_display_name;
     }
   }
 #endif
 
-  return base::UTF8ToUTF16(origin.host());
+  std::string languages =
+      profile->GetPrefs()->GetString(prefs::kAcceptLanguages);
+
+  return OriginDisplayName(origin, languages);
 }
+
+#if defined(ENABLE_EXTENSIONS)
+bool PlatformNotificationServiceImpl::ExtensionDisplayName(
+    Profile* profile,
+    const GURL& origin,
+    int process_id,
+    base::string16* out) const {
+  DCHECK(origin.SchemeIs(extensions::kExtensionScheme));
+  DCHECK(out);
+
+  extensions::InfoMap* extension_info_map =
+      extensions::ExtensionSystem::Get(profile)->info_map();

[Peter Beverloo, 2015/01/10 00:03:33]

This is the code that's violating threading constraints I was talking about.
extensions::InfoMap has a class-level comment that its methods may only be
called on the UI thread.

Given that we'll start doing security checks on the input per CL 834263004
already, I think that we can be a little bit less pedantic here about doing
another layer of validation. The only observable effect from all these checks
today is that we display a "weird" extension URL rather than an extension's
name.

We know that |origin| is an extension URL, so we can avoid doing the loop
altogether and instead just get the Extension* directly. We can also make the
assumption that the notifier is enabled, and DCHECK for that instead. After all,
this method only has to retrieve the extension's name.

What about the following?

==========
  const extensions::Extension* extension =
      extensions::ExtensionRegistry::Get(browser_context)->GetExtensionById(
          origin.host(),
          extensions::ExtensionRegistry::ENABLED);
  DCHECK(extension);

  return base::UTF8ToUTF16(extension->name());
==========

This approach:

    * Is safe to be called on the UI thread.
    * Can be in-lined into DisplayNameForOriginInProcessId, avoiding the need
for another method.
    * Avoids introducing a new dependencies on (a) Profile*, and (b)
DesktopNotificationService.
    * Allows the |render_process_id| argument to be removed from
Display(Persistent)Notification.

If desired, we can always DCHECK about the notifier being enabled here, but I
don't think we have to.

[dewittj, 2015/01/13 22:18:01]

Yes, I know this is the threading problem; just moving code around.

I'm happy to take your approach, although it appears that we already use methods
on Profile*: ->GetHostContentSettingsMap and (also from this patch ->GetPrefs) 
so we still have some work to do to remove all Profile* dereferences here.

Also prefer to use ExtensionRegistry::EVERYTHING so that we don't ever run into
a nullptr in non-debug code.  WDYT?

[Peter Beverloo, 2015/01/14 19:18:14]

That seems good to me. If this does ever happen in the wild, we should
understand why (disabled extensions shouldn't show notifications).

+  if (extension_info_map) {
+    extensions::ExtensionSet extensions;
+    extension_info_map->GetExtensionsWithAPIPermissionForSecurityOrigin(
+        origin, process_id, extensions::APIPermission::kNotifications,
+        &extensions);
+
+    DesktopNotificationService* desktop_notification_service =
+        DesktopNotificationServiceFactory::GetForProfile(profile);
+    DCHECK(desktop_notification_service);
+
+    for (auto& extension : extensions) {
+      NotifierId notifier_id(NotifierId::APPLICATION, extension->id());
+      if (desktop_notification_service->IsNotifierEnabled(notifier_id)) {
+        *out = base::UTF8ToUTF16(extension->name());
+        return true;
+      }
+    }
+  }
+  return false;
+}
+#endif  // defined(ENABLE_EXTENSIONS)
+
+// static
+base::string16 PlatformNotificationServiceImpl::OriginDisplayName(
+    const GURL& origin,
+    const std::string& languages) {
+  if (origin.SchemeIsHTTPOrHTTPS()) {
+    base::string16 formatted_origin =
+        net::IDNToUnicode(origin.host(), languages);
+    if (origin.has_port()) {
+      formatted_origin.push_back(':');
+      formatted_origin.append(base::UTF8ToUTF16(origin.port()));
+    }
+    return formatted_origin;
+  }
+
+  // TODO(dewittj): Once file:// URLs are passed in to the origin
+  // GURL here, begin returning the path as the display name.
+  return net::FormatUrl(origin, languages);
+}
+
+