Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(472)

Unified Diff: chrome/browser/notifications/desktop_notification_service.cc

Issue 661643002: Adds a context message of the security origin for web notifications. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Addresses peter's comments. Created 6 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/notifications/desktop_notification_service.cc
diff --git a/chrome/browser/notifications/desktop_notification_service.cc b/chrome/browser/notifications/desktop_notification_service.cc
index 2806ce1ed5c0d89cc560a76f93f6ed66666dde74..cabafbafe83fe2a01dbdd4701d17e4d82e51aa1f 100644
--- a/chrome/browser/notifications/desktop_notification_service.cc
+++ b/chrome/browser/notifications/desktop_notification_service.cc
@@ -32,6 +32,7 @@
#include "content/public/browser/render_view_host.h"
#include "content/public/browser/web_contents.h"
#include "content/public/common/show_desktop_notification_params.h"
+#include "net/base/net_util.h"
#include "ui/base/webui/web_ui_util.h"
#include "ui/message_center/notifier_settings.h"
@@ -191,8 +192,12 @@ void DesktopNotificationService::ShowDesktopNotification(
const GURL& origin = params.origin;
NotificationObjectProxy* proxy = new NotificationObjectProxy(delegate.Pass());
- base::string16 display_source = DisplayNameForOriginInProcessId(
- origin, render_frame_host->GetProcess()->GetID());
+ int process_id = render_frame_host->GetProcess()->GetID();
Peter Beverloo 2014/10/29 16:25:19 nit: You probably have to rebase this, since ShowD
dewittj 2014/11/03 17:18:32 Done.
+
+ // TODO(dewittj): The origin of file:// URLs always appears to be the empty
+ // URL. We need a better way to display this than returning an empty string.
+ base::string16 display_source =
+ DisplayNameForOriginInProcessId(origin, 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
@@ -202,6 +207,9 @@ void DesktopNotificationService::ShowDesktopNotification(
gfx::Image::CreateFrom1xBitmap(params.icon),
display_source, params.replace_id, proxy);
+ // Web notifications should display their origin
+ notification.set_context_message(display_source);
+
// The webkit notification doesn't timeout.
notification.set_never_timeout(true);
@@ -216,34 +224,74 @@ void DesktopNotificationService::ShowDesktopNotification(
}
base::string16 DesktopNotificationService::DisplayNameForOriginInProcessId(
- const GURL& origin, int process_id) {
+ 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);
- for (extensions::ExtensionSet::const_iterator iter = extensions.begin();
- iter != extensions.end(); ++iter) {
- NotifierId notifier_id(NotifierId::APPLICATION, (*iter)->id());
- if (IsNotifierEnabled(notifier_id))
- return base::UTF8ToUTF16((*iter)->name());
- }
+ base::string16 extension_display_name;
+ if (ExtensionDisplayName(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 DesktopNotificationService::ExtensionDisplayName(
+ 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();
+ if (extension_info_map) {
+ extensions::ExtensionSet extensions;
+ extension_info_map->GetExtensionsWithAPIPermissionForSecurityOrigin(
+ origin,
+ process_id,
+ extensions::APIPermission::kNotifications,
+ &extensions);
+ for (auto& extension : extensions) {
+ NotifierId notifier_id(NotifierId::APPLICATION, extension->id());
+ if (IsNotifierEnabled(notifier_id)) {
+ *out = base::UTF8ToUTF16(extension->name());
+ return true;
+ }
+ }
+ }
+ return false;
+}
+#endif // defined(ENABLE_EXTENSIONS)
+
+// static
+base::string16 DesktopNotificationService::OriginDisplayName(
+ const GURL& origin,
+ 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);
felt 2014/10/30 06:40:14 is it possible for other schemes to send notificat
dewittj 2014/11/03 17:18:32 I'm not aware of anything preventing ftp:// origin
}
bool DesktopNotificationService::IsNotifierEnabled(
- const NotifierId& notifier_id) {
+ const NotifierId& notifier_id) const {
switch (notifier_id.type) {
case NotifierId::APPLICATION:
return disabled_extension_ids_.find(notifier_id.id) ==

Powered by Google App Engine
This is Rietveld 408576698