Only resort to etl+1 when the full domain does not fit.

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 83d4badc3b5a1c054dd6f1d67db542fa1658a703..96547de92a3b0e9a7e42e1d3ca64aff6097be270 100644
--- a/chrome/browser/notifications/notification_platform_bridge_mac.mm
+++ b/chrome/browser/notifications/notification_platform_bridge_mac.mm
@@ -133,19 +133,42 @@ base::string16 CreateNotificationContext(const Notification& notification,
   if (!requires_attribution)
     return notification.context_message();
 
-  base::string16 context =
+  // OSX notifications don't provide a good way to elide the domain (or tell you

[Peter Beverloo, 2017/05/05 14:55:51]

nit: OSX -> Mac OS

[Miguel Garcia, 2017/05/08 10:20:11]

Done.

+  // the maximum widh of the subtitle field) Therefore we use a bit of a mixed

[Peter Beverloo, 2017/05/05 14:55:51]

nit: widh -> width
nit: period after the closing parenthesis

[Miguel Garcia, 2017/05/08 10:20:11]

Done.

+  // approach We have experimentally determined the maximum number of characters

[Peter Beverloo, 2017/05/05 14:55:51]

nit: period after "approach". Or just remove "Therefore...approach" altogether.

[Miguel Garcia, 2017/05/08 10:20:11]

Done.

+  // that fit using the widest possible character (m) If the domain fits in

[Peter Beverloo, 2017/05/05 14:55:51]

nit: period after (m)

[Miguel Garcia, 2017/05/08 10:20:11]

Done.

+  // those character we show it completely Otherwise we use etld+1

[Peter Beverloo, 2017/05/05 14:55:51]

nit: period after "completely"
nit: etld+1 -> eTLD + 1. (capitalisation of "TLD" is significant)

[Miguel Garcia, 2017/05/08 10:20:11]

Done.

+
+  // These numbers have been obtained through experimentation on various
+  // android platforms.

[Peter Beverloo, 2017/05/05 14:55:51]

???????????????????????

[Miguel Garcia, 2017/05/08 10:20:11]

Acknowledged.

+
+  // Corresponds to the string "mmmmmmmmmmmmmm"
+  const uint kMaxDomainLenghtAlert = 14;
+
+  // Corresponds to the string "mmmmmmmmmmmmmmmmmmmmm"
+  const uint kMaxDomainLenghtBanner = 21;

[Peter Beverloo, 2017/05/05 14:55:51]

constexpr size_t kMaxDomainLength{Alert,Banner}

[Miguel Garcia, 2017/05/08 10:20:11]

For variables I think const and constexpr are the same right?

+
+  uint max_characters = notification.never_timeout() ? kMaxDomainLenghtAlert

[Peter Beverloo, 2017/05/05 14:55:50]

size_t here too (we don't use the "uint" type w/o a length)

[Peter Beverloo, 2017/05/05 14:55:51]

Do we have to consider progress notifications here too?

[Miguel Garcia, 2017/05/08 10:20:11]

Done.

[Miguel Garcia, 2017/05/08 10:20:11]

Let's add it just for safety although since progress notifications don't require
attribution this should not be a problem.

+                                                     : kMaxDomainLenghtBanner;
+
+  base::string16 origin = url_formatter::FormatOriginForSecurityDisplay(
+      url::Origin(notification.origin_url()),
+      url_formatter::SchemeDisplay::OMIT_HTTP_AND_HTTPS);
+
+  if (origin.size() <= max_characters)
+    return origin;
+
+  // Too long, use etld+1
+  base::string16 etldplusone =
       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.
-  if (context.empty()) {
-    context = url_formatter::FormatOriginForSecurityDisplay(
-        url::Origin(notification.origin_url()),
-        url_formatter::SchemeDisplay::OMIT_HTTP_AND_HTTPS);
-  }
+  if (etldplusone.empty())
+    return origin;
 
-  return context;
+  return etldplusone;
 }
 }  // namespace