Linux native notifications: Delete notification after sending CloseNotification.

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>
+#include <utility>
+#include <vector>

[Tom (Use chromium acct), 2017/05/03 21:42:56]

out of curiosity, did you do this by hand?

[Lei Zhang, 2017/05/03 21:45:24]

"git cl lint" checks for these.

 
 #include "base/barrier_closure.h"
 #include "base/files/file_util.h"
 #include "base/strings/nullable_string16.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/task_scheduler/post_task.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chrome_notification_types.h"
@@ skipping 47 matching lines @@
 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)
     return;
 
-  NotificationDisplayService* display_service =
-      NotificationDisplayServiceFactory::GetForProfile(profile);
-
-  static_cast<NativeNotificationDisplayService*>(display_service)
-      ->ProcessNotificationOperation(operation, notification_type, origin,
-                                     notification_id, action_index, reply);
+  auto* display_service = static_cast<NativeNotificationDisplayService*>(
+      NotificationDisplayServiceFactory::GetForProfile(profile));
+  display_service->ProcessNotificationOperation(operation, notification_type,
+                                                origin, notification_id,
+                                                action_index, reply);
 }
 
 void ForwardNotificationOperationOnUiThread(
     NotificationCommon::Operation operation,
     NotificationCommon::Type notification_type,
     const std::string& origin,
     const std::string& notification_id,
     int action_index,
     const std::string& profile_id,
     bool is_incognito) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  ProfileManager* profile_manager = g_browser_process->profile_manager();
-
-  profile_manager->LoadProfile(
+  g_browser_process->profile_manager()->LoadProfile(
       profile_id, is_incognito,
       base::Bind(&ProfileLoadedCallback, operation, notification_type, origin,
                  notification_id, action_index, base::NullableString16()));
 }
 
-// Writes |data| to a new temporary file and returns its path.
-// Returns base::FilePath() on failure.
-base::FilePath WriteDataToTmpFile(
+class ResourceFile {
+ public:
+  explicit ResourceFile(const base::FilePath& file_path)
+      : file_path_(file_path) {
+    DCHECK(!file_path.empty());
+  }
+  ~ResourceFile() { base::DeleteFile(file_path_, false); }
+
+  const base::FilePath& file_path() const { return file_path_; }
+
+ private:
+  const base::FilePath file_path_;
+
+  DISALLOW_COPY_AND_ASSIGN(ResourceFile);
+};
+
+// Writes |data| to a new temporary file and returns the ResourceFile
+// that holds it.
+std::unique_ptr<ResourceFile> WriteDataToTmpFile(
     const scoped_refptr<base::RefCountedMemory>& data) {
   int data_len = data->size();
   if (data_len == 0)
-    return base::FilePath();
+    return nullptr;
   base::FilePath file_path;
   if (!base::CreateTemporaryFile(&file_path))
-    return base::FilePath();
+    return nullptr;
+
+  auto resource_file = base::MakeUnique<ResourceFile>(file_path);
   if (base::WriteFile(file_path, data->front_as<char>(), data_len) !=
       data_len) {
-    base::DeleteFile(file_path, false);
-    return base::FilePath();
+    resource_file.reset();
   }
-  return file_path;
+  return resource_file;
 }
 
 }  // namespace
 
 // static
 NotificationPlatformBridge* NotificationPlatformBridge::Create() {
   return new NotificationPlatformBridgeLinux();
 }
 
 class NotificationPlatformBridgeLinuxImpl
@@ skipping 75 matching lines @@
 
   void CleanUp() {
     DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
     PostTaskToTaskRunnerThread(base::BindOnce(
         &NotificationPlatformBridgeLinuxImpl::CleanUpOnTaskRunner, this));
   }
 
  private:
   friend class base::RefCountedThreadSafe<NotificationPlatformBridgeLinuxImpl>;
 
-  struct ResourceFile {
-    explicit ResourceFile(const base::FilePath& file_path)
-        : file_path(file_path) {}
-    ~ResourceFile() { base::DeleteFile(file_path, false); }
-    const base::FilePath file_path;
-  };
-
   struct NotificationData {
     NotificationData(NotificationCommon::Type notification_type,
                      const std::string& notification_id,
                      const std::string& profile_id,
                      bool is_incognito,
                      const GURL& origin_url)
         : notification_type(notification_type),
           notification_id(notification_id),
           profile_id(profile_id),
           is_incognito(is_incognito),
@@ skipping 177 matching lines @@
     const char kDesktopFileSuffix[] = ".desktop";
     DCHECK(base::EndsWith(desktop_file.value(), kDesktopFileSuffix,
                           base::CompareCase::SENSITIVE));
     desktop_file = desktop_file.RemoveFinalExtension();
     dbus::MessageWriter desktop_entry_writer(nullptr);
     hints_writer.OpenDictEntry(&desktop_entry_writer);
     desktop_entry_writer.AppendString("desktop-entry");
     desktop_entry_writer.AppendVariantOfString(desktop_file.value());
     hints_writer.CloseContainer(&desktop_entry_writer);
 
-    base::FilePath icon_file =
+    std::unique_ptr<ResourceFile> icon_file =
         WriteDataToTmpFile(notification->icon().As1xPNGBytes());
-    if (!icon_file.empty()) {
+    if (icon_file) {
       dbus::MessageWriter image_path_writer(nullptr);
       hints_writer.OpenDictEntry(&image_path_writer);
       image_path_writer.AppendString("image-path");
-      image_path_writer.AppendVariantOfString(icon_file.value());
+      image_path_writer.AppendVariantOfString(icon_file->file_path().value());
       hints_writer.CloseContainer(&image_path_writer);
-      data->resource_files.push_back(base::MakeUnique<ResourceFile>(icon_file));
+      data->resource_files.push_back(std::move(icon_file));
     }
 
     writer.CloseContainer(&hints_writer);
 
     const int32_t kExpireTimeoutDefault = -1;
     writer.AppendInt32(kExpireTimeoutDefault);
 
     std::unique_ptr<dbus::Response> response =
         notification_proxy_->CallMethodAndBlock(
             &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT);
@@ skipping 15 matching lines @@
     for (const auto& pair : notifications_) {
       NotificationData* data = pair.first;
       if (data->notification_id == notification_id &&
           data->profile_id == profile_id) {
         dbus::MethodCall method_call(kFreedesktopNotificationsName,
                                      "CloseNotification");
         dbus::MessageWriter writer(&method_call);
         writer.AppendUint32(data->dbus_id);
         notification_proxy_->CallMethodAndBlock(
             &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT);
+        to_erase.push_back(data);
       }
     }
     for (NotificationData* data : to_erase)
       notifications_.erase(data);
   }
 
   void GetDisplayedOnTaskRunner(
       const std::string& profile_id,
       bool incognito,
       const GetDisplayedNotificationsCallback& callback) const {
     DCHECK(task_runner_->RunsTasksOnCurrentThread());
     auto displayed = base::MakeUnique<std::set<std::string>>();
-    for (const auto& notification : notifications_) {
-      if (notification.first->profile_id == profile_id &&
-          notification.first->is_incognito == incognito) {
-        displayed->insert(notification.first->notification_id);
-      }
+    for (const auto& pair : notifications_) {
+      NotificationData* data = pair.first;
+      if (data->profile_id == profile_id && data->is_incognito == incognito)
+        displayed->insert(data->notification_id);
     }
     PostTaskToUiThread(base::BindOnce(callback, std::move(displayed), true));
   }
 
   NotificationData* FindNotificationData(const std::string& notification_id,
                                          const std::string& profile_id,
                                          bool is_incognito) {
     DCHECK(task_runner_->RunsTasksOnCurrentThread());
     for (const auto& pair : notifications_) {
       NotificationData* data = pair.first;
       if (data->notification_id == notification_id &&
           data->profile_id == profile_id &&
           data->is_incognito == is_incognito) {
         return data;
       }
     }
 
     return nullptr;
   }
 
-  NotificationData* FindNotificationData(uint32_t dbus_id) {
+  NotificationData* FindNotificationDataWithDBusId(uint32_t dbus_id) {
     DCHECK(task_runner_->RunsTasksOnCurrentThread());
+    DCHECK(dbus_id);
     for (const auto& pair : notifications_) {
       NotificationData* data = pair.first;
       if (data->dbus_id == dbus_id)
         return data;
     }
 
     return nullptr;
   }
 
   void ForwardNotificationOperation(NotificationData* data,
                                     NotificationCommon::Operation operation,
                                     int action_index) {
     DCHECK(task_runner_->RunsTasksOnCurrentThread());
     PostTaskToUiThread(base::BindOnce(
         ForwardNotificationOperationOnUiThread, operation,
         data->notification_type, data->origin_url.spec(), data->notification_id,
         action_index, data->profile_id, data->is_incognito));
   }
 
   void OnActionInvoked(dbus::Signal* signal) {
     DCHECK(task_runner_->RunsTasksOnCurrentThread());
     dbus::MessageReader reader(signal);
     uint32_t dbus_id;
-    if (!reader.PopUint32(&dbus_id))
+    if (!reader.PopUint32(&dbus_id) || !dbus_id)
       return;
     std::string action;
     if (!reader.PopString(&action))
       return;
 
-    NotificationData* data = FindNotificationData(dbus_id);
+    NotificationData* data = FindNotificationDataWithDBusId(dbus_id);
     if (!data)
       return;
 
     if (action == kDefaultButtonId) {
       ForwardNotificationOperation(data, NotificationCommon::CLICK, -1);
     } else if (action == kSettingsButtonId) {
       ForwardNotificationOperation(data, NotificationCommon::SETTINGS, -1);
     } else {
       size_t id;
       if (!base::StringToSizeT(action, &id))
         return;
       size_t n_buttons = data->action_end - data->action_start;
       size_t id_zero_based = id - data->action_start;
       if (id_zero_based >= n_buttons)
         return;
       ForwardNotificationOperation(data, NotificationCommon::CLICK,
                                    id_zero_based);
     }
   }
 
   void OnNotificationClosed(dbus::Signal* signal) {
     DCHECK(task_runner_->RunsTasksOnCurrentThread());
     dbus::MessageReader reader(signal);
     uint32_t dbus_id;
-    if (!reader.PopUint32(&dbus_id))
+    if (!reader.PopUint32(&dbus_id) || !dbus_id)
       return;
 
-    NotificationData* data = FindNotificationData(dbus_id);
+    NotificationData* data = FindNotificationDataWithDBusId(dbus_id);
     if (!data)
       return;
 
     ForwardNotificationOperation(data, NotificationCommon::CLOSE, -1);
     notifications_.erase(data);
   }
 
   // Called once the connection has been set up (or not).  |success|
   // indicates the connection is ready to use.
   void OnConnectionInitializationFinishedOnUiThread(bool success) {
@@ skipping 92 matching lines @@
 }
 
 void NotificationPlatformBridgeLinux::SetReadyCallback(
     NotificationBridgeReadyCallback callback) {
   impl_->SetReadyCallback(std::move(callback));
 }
 
 void NotificationPlatformBridgeLinux::CleanUp() {
   impl_->CleanUp();
 }