Linux native notifications: Support image notifications

chrome/browser/notifications/notification_platform_bridge_linux.cc

 // Copyright 2017 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/notification_platform_bridge_linux.h"
 
 #include <algorithm>
 #include <memory>
 #include <set>
 #include <unordered_map>
@@ skipping 18 matching lines @@
 #include "chrome/browser/notifications/notification.h"
 #include "chrome/browser/notifications/notification_display_service_factory.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/shell_integration_linux.h"
 #include "chrome/grit/generated_resources.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"
 
 namespace {
 
 // DBus name / path.
 const char kFreedesktopNotificationsName[] = "org.freedesktop.Notifications";
 const char kFreedesktopNotificationsPath[] = "/org/freedesktop/Notifications";
 
 // DBus methods.
@@ skipping 15 matching lines @@
 const char kCapabilityBodyMarkup[] = "body-markup";
 const char kCapabilityIconMulti[] = "icon-multi";
 const char kCapabilityIconStatic[] = "icon-static";
 const char kCapabilityPersistence[] = "persistence";
 const char kCapabilitySound[] = "sound";
 
 // Button IDs.
 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
 // added at the end, before NUM_ITEMS.
 enum class ConnectionInitializationStatusCode {
   SUCCESS = 0,
   NATIVE_NOTIFICATIONS_NOT_SUPPORTED = 1,
   MISSING_REQUIRED_CAPABILITIES = 2,
   COULD_NOT_CONNECT_TO_SIGNALS = 3,
   INCOMPATIBLE_SPEC_VERSION = 4,
   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) {
     title += base::FormatPercent(notification.progress());
     title += base::UTF8ToUTF16(" - ");
   }
   title += notification.title();
   return title;
 }
 
@@ skipping 17 matching lines @@
     case message_center::HIGH_PRIORITY:
     case message_center::MAX_PRIORITY:
       return CRITICAL;
     default:
       NOTREACHED();
     case message_center::DEFAULT_PRIORITY:
       return NORMAL;
   }
 }
 
+// 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) {
+  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,
                            NotificationCommon::Type notification_type,
                            const std::string& origin,
                            const std::string& notification_id,
                            int action_index,
                            const base::NullableString16& reply,
                            Profile* profile) {
   if (!profile)
@@ skipping 94 matching lines @@
                bool is_incognito,
                const Notification& notification) override {
     DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
     // Notifications contain gfx::Image's which have reference counts
     // that are not thread safe.  Because of this, we duplicate the
     // notification and its images.  Wrap the notification in a
     // unique_ptr to transfer ownership of the notification (and the
     // 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());
 
     PostTaskToTaskRunnerThread(base::BindOnce(
         &NotificationPlatformBridgeLinuxImpl::DisplayOnTaskRunner, this,
         notification_type, notification_id, profile_id, is_incognito,
         base::Passed(&notification_copy)));
   }
 
@@ skipping 81 matching lines @@
   void Observe(int type,
                const content::NotificationSource& source,
                const content::NotificationDetails& details) override {
     DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
     DCHECK_EQ(chrome::NOTIFICATION_APP_TERMINATING, type);
     // The browser process is about to exit.  Post the CleanUp() task
     // while we still can.
     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(
         content::BrowserThread::UI, FROM_HERE, std::move(closure));
     DCHECK(success);
   }
 
   void PostTaskToTaskRunnerThread(base::OnceClosure closure) const {
     DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
     DCHECK(task_runner_);
@@ skipping 29 matching lines @@
         notification_proxy_->CallMethodAndBlock(
             &get_capabilities_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT);
     if (capabilities_response) {
       dbus::MessageReader reader(capabilities_response.get());
       std::vector<std::string> capabilities;
       reader.PopArrayOfStrings(&capabilities);
       for (const std::string& capability : capabilities)
         capabilities_.insert(capability);
     }
     RecordMetricsForCapabilities();
+    PostTaskToUiThread(base::BindOnce(
+        &NotificationPlatformBridgeLinuxImpl::SetBodyImagesSupported, this,
+        base::ContainsKey(capabilities_, kCapabilityBodyImages)));
 
     dbus::MethodCall get_server_information_call(kFreedesktopNotificationsName,
                                                  kMethodGetServerInformation);
     std::unique_ptr<dbus::Response> server_information_response =
         notification_proxy_->CallMethodAndBlock(
             &get_server_information_call,
             dbus::ObjectProxy::TIMEOUT_USE_DEFAULT);
     if (server_information_response) {
       dbus::MessageReader reader(server_information_response.get());
       std::string spec_version;
@@ skipping 69 matching lines @@
 
     // app_icon passed implicitly via desktop-entry.
     writer.AppendString("");
 
     writer.AppendString(
         base::UTF16ToUTF8(CreateNotificationTitle(*notification)));
 
     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=\"\"/>";

[Peter Beverloo, 2017/05/15 16:16:00]

Let's use an std::stringstream for building |body|. Every + operator can lead to
reallocation, which we have a lot of right now. std::stringstream will also
allow you to use the much nicer << operator.

[Tom (Use chromium acct), 2017/05/15 19:18:39]

Done.

+          data->resource_files.push_back(std::move(image_file));
+        }
+      }
     }
     writer.AppendString(body);
 
     // Even-indexed elements in this vector are action IDs passed back to
     // us in OnActionInvoked().  Odd-indexed ones contain the button text.
     std::vector<std::string> actions;
     if (base::ContainsKey(capabilities_, kCapabilityActions)) {
       data->action_start = data->action_end;
       for (const auto& button_info : notification->buttons()) {
         // FDO notification buttons can contain either an icon or a label,
@@ skipping 49 matching lines @@
               ? "image_path"
               : "image-path");
       image_path_writer.AppendVariantOfString(icon_file->file_path().value());
       hints_writer.CloseContainer(&image_path_writer);
       data->resource_files.push_back(std::move(icon_file));
     }
 
     writer.CloseContainer(&hints_writer);
 
     const int32_t kExpireTimeoutDefault = -1;
-    writer.AppendInt32(kExpireTimeoutDefault);
+    const int32_t kExpireTimeoutNever = 0;
+    writer.AppendInt32(notification->never_timeout() ? kExpireTimeoutNever

[Peter Beverloo, 2017/05/15 16:16:00]

As a general comment, some of your CLs are a bit tricky to review because they
include other CLs. Both Gerrit and Rietveld can work with dependent patch sets
for that - create a local Git branch that tracks another branch. The try-bots
can deal with this just fine, while the diff will only show the actual diff, and
not that of the CLs it depends on.

[Tom (Use chromium acct), 2017/05/15 19:18:39]

Done.  Sorry, the dependent patch set somehow got removed between patch set 3
and patch set 4.

+                                                     : kExpireTimeoutDefault);
 
     std::unique_ptr<dbus::Response> response =
         notification_proxy_->CallMethodAndBlock(
             &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT);
     if (response) {
       dbus::MessageReader reader(response.get());
       reader.PopUint32(&data->dbus_id);
     }
     if (!data->dbus_id) {
       // There was some sort of error with creating the notification.
@@ skipping 195 matching lines @@
 
   scoped_refptr<base::SequencedTaskRunner> task_runner_;
 
   content::NotificationRegistrar registrar_;
 
   // State necessary for OnConnectionInitializationFinished() and
   // SetReadyCallback().
   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;

[Peter Beverloo, 2017/05/15 16:16:00]

Technically we should never call Display() before initialization succeeds,
right?

[Tom (Use chromium acct), 2017/05/15 19:18:39]

In the case of unit testing, calls to Display() are made before init finishes

[Lei Zhang, 2017/05/15 21:48:40]

Is it a lot of work to change the unit tests to not do that?

[Tom (Use chromium acct), 2017/05/16 00:22:00]

Nope, just a one line change in CreatePlatformBridgeLinux().

Done.  And I made this variable a base::Optional<bool> so that a DCHECK will
trigger if this is ever NOT true.

+
   //////////////////////////////////////////////////////////////////////////////
   // Members used only on the task runner thread.
 
   scoped_refptr<dbus::Bus> bus_;
 
   dbus::ObjectProxy* notification_proxy_ = nullptr;
 
   std::unordered_set<std::string> capabilities_;
 
   base::Version spec_version_;
@@ skipping 46 matching lines @@
 }
 
 void NotificationPlatformBridgeLinux::SetReadyCallback(
     NotificationBridgeReadyCallback callback) {
   impl_->SetReadyCallback(std::move(callback));
 }
 
 void NotificationPlatformBridgeLinux::CleanUp() {
   impl_->CleanUp();
 }