Display ETLD+1 instead of the full domain in MacOSX native notifications

chrome/browser/notifications/notification_platform_bridge_mac.mm

Index: chrome/browser/notifications/notification_platform_bridge_mac.mm
diff --git a/chrome/browser/notifications/notification_platform_bridge_mac.mm b/chrome/browser/notifications/notification_platform_bridge_mac.mm
index b3357a6ceb216506db1525ac67c10aba3b35f6dd..2508637b0784459ce68d7f2f4d8f4dc6eb8e40f2 100644
--- a/chrome/browser/notifications/notification_platform_bridge_mac.mm
+++ b/chrome/browser/notifications/notification_platform_bridge_mac.mm
@@ -36,6 +36,7 @@
 #include "chrome/grit/generated_resources.h"
 #include "components/crash/content/app/crashpad.h"
 #include "components/url_formatter/elide_url.h"
+#include "net/base/registry_controlled_domains/registry_controlled_domain.h"
 #include "third_party/WebKit/public/platform/modules/notifications/WebNotificationConstants.h"
 #include "third_party/crashpad/crashpad/client/crashpad_client.h"
 #include "ui/base/l10n/l10n_util_mac.h"
@@ -126,6 +127,29 @@ base::string16 CreateNotificationTitle(const Notification& notification) {
   return title;
 }
 
+base::string16 CreateNotificationContext(
+    const Notification& notification,
+    NotificationCommon::Type notification_type) {

[Peter Beverloo, 2017/04/18 14:48:54]

nit: I'd prefer to pass in `requires_attribution` as an argument as opposed to
`notification_type`. Keeps such type-based decisions together.

[Miguel Garcia, 2017/04/18 14:59:10]

Done.

+  bool requires_attribution =
+      notification.context_message().empty() &&
+      notification_type != NotificationCommon::EXTENSION;
+  if (!requires_attribution)
+    return notification.context_message();
+
+  base::string16 context =
+      base::UTF8ToUTF16(net::registry_controlled_domains::GetDomainAndRegistry(
+          notification.origin_url(),
+          net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES));
+
+  // localhost, raw IPs etc. are not handled by GetDomainAndRegistry

[Peter Beverloo, 2017/04/18 14:48:54]

Maybe explicitly refer the reader to the GetDomainAndRegistry() documentation
for these edge-cases, instead of repeating them? Also a period to finish the
sentence.

[Miguel Garcia, 2017/04/18 14:59:10]

Added the period at the end.  I kind of like having a few examples right here
and GetDomainAndRegistry does not seem to talk about localhost apparently

+  if (context.empty()) {
+    context = url_formatter::FormatOriginForSecurityDisplay(
+        url::Origin(notification.origin_url()),
+        url_formatter::SchemeDisplay::OMIT_HTTP_AND_HTTPS);
+  }
+
+  return context;
+}
 }  // namespace
 
 // A Cocoa class that represents the delegate of NSUserNotificationCenter and
@@ -191,18 +215,9 @@ void NotificationPlatformBridgeMac::Display(
 
   [builder setContextMessage:base::SysUTF16ToNSString(context_message)];
 
-  bool requires_attribution =
-      notification.context_message().empty() &&
-      notification_type != NotificationCommon::EXTENSION;
-
-  base::string16 subtitle =
-      requires_attribution
-          ? url_formatter::FormatOriginForSecurityDisplay(
-                url::Origin(notification.origin_url()),
-                url_formatter::SchemeDisplay::OMIT_HTTP_AND_HTTPS)
-          : notification.context_message();
+  [builder setSubTitle:base::SysUTF16ToNSString(CreateNotificationContext(
+                           notification, notification_type))];
 
-  [builder setSubTitle:base::SysUTF16ToNSString(subtitle)];
   if (!notification.icon().IsEmpty()) {
     [builder setIcon:notification.icon().ToNSImage()];
   }