Linux native notifications: Add attribution

chrome/browser/notifications/notification_platform_bridge_linux.cc

Index: chrome/browser/notifications/notification_platform_bridge_linux.cc
diff --git a/chrome/browser/notifications/notification_platform_bridge_linux.cc b/chrome/browser/notifications/notification_platform_bridge_linux.cc
index 602b7b3047963c747b8d7246f67cdb8750e763af..50005c6660b77d9771cb5524af0e76aa790b297d 100644
--- a/chrome/browser/notifications/notification_platform_bridge_linux.cc
+++ b/chrome/browser/notifications/notification_platform_bridge_linux.cc
@@ -31,11 +31,13 @@
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/shell_integration_linux.h"
 #include "chrome/grit/generated_resources.h"
+#include "components/url_formatter/elide_url.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_service.h"
 #include "dbus/bus.h"
 #include "dbus/message.h"
 #include "dbus/object_proxy.h"
+#include "skia/ext/image_operations.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/gfx/image/image_skia.h"
 
@@ -71,6 +73,10 @@ const char kCapabilitySound[] = "sound";
 const char kDefaultButtonId[] = "default";
 const char kSettingsButtonId[] = "settings";
 
+// Max image size; specified in the FDO notification specification.
+const int kMaxImageWidth = 200;
+const int kMaxImageHeight = 100;
+
 // The values in this enumeration correspond to those of the
 // Linux.NotificationPlatformBridge.InitializationStatus histogram, so
 // the ordering should not be changed.  New error codes should be
@@ -84,6 +90,10 @@ enum class ConnectionInitializationStatusCode {
   NUM_ITEMS
 };
 
+int ClampInt(int value, int low, int hi) {
+  return std::max(std::min(value, hi), low);
+}
+
 base::string16 CreateNotificationTitle(const Notification& notification) {
   base::string16 title;
   if (notification.type() == message_center::NOTIFICATION_TYPE_PROGRESS) {
@@ -121,6 +131,28 @@ int NotificationPriorityToFdoUrgency(int priority) {
   }
 }
 
+// Constrain |image|'s size to |kMaxImageWidth|x|kMaxImageHeight|. If
+// the image does not need to be resized, or the image is empty,
+// returns |image| directly.
+gfx::Image ResizeImageToFdoMaxSize(const gfx::Image& image) {

[yoshiki, 2017/05/15 02:35:15]

nit: Could you add a DCHECK to ensure it's not on the UI thread? Someone in
future may try to call this on the UI thread wrongly.

DCHECK(!content::BrowserThread::CurrentlyOn(content::BrowserThread::UI))

[Tom (Use chromium acct), 2017/05/15 22:07:04]

Done.

+  if (image.IsEmpty())
+    return image;
+  int width = image.Width();
+  int height = image.Height();
+  if (width <= kMaxImageWidth && height <= kMaxImageHeight) {
+    return image;
+  }
+  const SkBitmap* image_bitmap = image.ToSkBitmap();
+  double scale = std::min(static_cast<double>(kMaxImageWidth) / width,
+                          static_cast<double>(kMaxImageHeight) / height);
+  width = ClampInt(scale * width, 1, kMaxImageWidth);
+  height = ClampInt(scale * height, 1, kMaxImageHeight);
+  return gfx::Image(
+      gfx::ImageSkia::CreateFrom1xBitmap(skia::ImageOperations::Resize(
+          *image_bitmap, skia::ImageOperations::RESIZE_LANCZOS3, width,
+          height)));
+}
+
 // Runs once the profile has been loaded in order to perform a given
 // |operation| on a notification.
 void ProfileLoadedCallback(NotificationCommon::Operation operation,
@@ -235,7 +267,9 @@ class NotificationPlatformBridgeLinuxImpl
     // non-thread-safe reference counts) to the task runner thread.
     auto notification_copy = base::MakeUnique<Notification>(notification);
     notification_copy->set_icon(DeepCopyImage(notification_copy->icon()));
-    notification_copy->set_image(gfx::Image());
+    notification_copy->set_image(body_images_supported_
+                                     ? DeepCopyImage(notification_copy->image())
+                                     : gfx::Image());
     notification_copy->set_small_image(gfx::Image());
     for (size_t i = 0; i < notification_copy->buttons().size(); i++)
       notification_copy->SetButtonIcon(i, gfx::Image());
@@ -337,6 +371,11 @@ class NotificationPlatformBridgeLinuxImpl
     CleanUp();
   }
 
+  void SetBodyImagesSupported(bool body_images_supported) {
+    DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+    body_images_supported_ = body_images_supported;
+  }
+
   void PostTaskToUiThread(base::OnceClosure closure) const {
     DCHECK(task_runner_->RunsTasksOnCurrentThread());
     bool success = content::BrowserThread::PostTask(
@@ -386,6 +425,9 @@ class NotificationPlatformBridgeLinuxImpl
         capabilities_.insert(capability);
     }
     RecordMetricsForCapabilities();
+    PostTaskToUiThread(base::BindOnce(
+        &NotificationPlatformBridgeLinuxImpl::SetBodyImagesSupported, this,
+        base::ContainsKey(capabilities_, kCapabilityBodyImages)));
 
     dbus::MethodCall get_server_information_call(kFreedesktopNotificationsName,
                                                  kMethodGetServerInformation);
@@ -475,11 +517,55 @@ class NotificationPlatformBridgeLinuxImpl
     std::string body;
     if (base::ContainsKey(capabilities_, kCapabilityBody)) {
       body = base::UTF16ToUTF8(notification->message());
-      if (base::ContainsKey(capabilities_, kCapabilityBodyMarkup)) {
+      const bool body_markup =
+          base::ContainsKey(capabilities_, kCapabilityBodyMarkup);
+      if (body_markup) {
         base::ReplaceSubstringsAfterOffset(&body, 0, "&", "&amp;");
         base::ReplaceSubstringsAfterOffset(&body, 0, "<", "&lt;");
         base::ReplaceSubstringsAfterOffset(&body, 0, ">", "&gt;");
       }
+      if (notification->type() == message_center::NOTIFICATION_TYPE_MULTIPLE) {
+        for (const auto& item : notification->items()) {
+          if (!body.empty())
+            body += "\n";
+          const std::string title = base::UTF16ToUTF8(item.title);
+          const std::string message = base::UTF16ToUTF8(item.message);
+          // TODO(peter): Figure out the right way to internationalize
+          // this for RTL languages.
+          if (body_markup)
+            body += "<b>" + title + "</b> " + message;
+          else
+            body += title + " - " + message;
+        }
+      } else if (notification->type() ==
+                     message_center::NOTIFICATION_TYPE_IMAGE &&
+                 base::ContainsKey(capabilities_, "body-images")) {
+        std::unique_ptr<ResourceFile> image_file = WriteDataToTmpFile(
+            ResizeImageToFdoMaxSize(notification->image()).As1xPNGBytes());
+        if (image_file) {
+          if (!body.empty())
+            body += "\n";
+          body +=
+              "<img src=\"" + image_file->file_path().value() + "\" alt=\"\"/>";
+          data->resource_files.push_back(std::move(image_file));
+        }
+      }
+
+      // Attribute this notification to the origin, if any.
+      if (!notification->origin_url().is_empty()) {

[Peter Beverloo, 2017/05/15 16:19:41]

If the notification has a context message, we should display that instead of the
origin. That's freeflow text however.

[Peter Beverloo, 2017/05/15 16:19:41]

How certain are we that the attribution is going to be displayed when at the
bottom? Can a notification center decide to maximize the length of a toast, and
cut it off? (We display the origin *before* the content on Mac OS X.)

[Tom (Use chromium acct), 2017/05/15 22:07:03]

Done.

[Tom (Use chromium acct), 2017/05/15 22:07:04]

Ok, I moved the context to the beginning of the body.

I've added some pictures on the bug for this.

+        if (!body.empty())
+          body += "\n";
+        std::string url_display_text =
+            base::UTF16ToUTF8(url_formatter::FormatUrlForSecurityDisplay(
+                notification->origin_url(),
+                url_formatter::SchemeDisplay::OMIT_HTTP_AND_HTTPS));

[Peter Beverloo, 2017/05/15 16:19:41]

What would wrapped contents look like? For example, super long domain names.

[Tom (Use chromium acct), 2017/05/15 22:07:04]

See the pictures on the bug to see how it looks on GNOME.  In general, there's
no way to know how long a line can be, so we cannot wrap it ourselves.  Giving
the whole thing to the notification server is the correct thing to do, I
believe.

+        std::string spec = notification->origin_url().spec();
+        if (base::ContainsKey(capabilities_, "body-hyperlinks")) {
+          body += "<a href=\"" + spec + "\">" + url_display_text + "</a>";
+        } else {
+          body += url_display_text;
+        }
+      }
     }
     writer.AppendString(body);
 
@@ -549,7 +635,9 @@ class NotificationPlatformBridgeLinuxImpl
     writer.CloseContainer(&hints_writer);
 
     const int32_t kExpireTimeoutDefault = -1;
-    writer.AppendInt32(kExpireTimeoutDefault);
+    const int32_t kExpireTimeoutNever = 0;
+    writer.AppendInt32(notification->never_timeout() ? kExpireTimeoutNever
+                                                     : kExpireTimeoutDefault);
 
     std::unique_ptr<dbus::Response> response =
         notification_proxy_->CallMethodAndBlock(
@@ -765,6 +853,12 @@ class NotificationPlatformBridgeLinuxImpl
   base::Optional<bool> connected_;
   std::vector<NotificationBridgeReadyCallback> on_connected_callbacks_;
 
+  // Notification servers very rarely have the 'body-images'
+  // capability, so try to avoid an image copy if possible. It is
+  // intialized to true so that the UI thread will conservatively copy
+  // the image before the GetCapabilities message completes.
+  bool body_images_supported_ = true;
+
   //////////////////////////////////////////////////////////////////////////////
   // Members used only on the task runner thread.