Linux native notifications: use image_path instead of image-path for spec 1.1

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 <unordered_set>
 #include <utility>
 #include <vector>
 
 #include "base/barrier_closure.h"
 #include "base/files/file_util.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/strings/nullable_string16.h"
 #include "base/strings/string_number_conversions.h"
+#include "base/strings/string_split.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"
 #include "chrome/browser/notifications/native_notification_display_service.h"
 #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"
@@ skipping 14 matching lines @@
 
 // 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
 };
 
 gfx::Image DeepCopyImage(const gfx::Image& image) {
   if (image.IsEmpty())
     return gfx::Image();
   std::unique_ptr<gfx::ImageSkia> image_skia(image.CopyImageSkia());
   return gfx::Image(*image_skia);
 }
 
@@ skipping 276 matching lines @@
             &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();
 
+    dbus::MethodCall get_server_information_call(kFreedesktopNotificationsName,
+                                                 "GetServerInformation");
+    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;
+      reader.PopString(&spec_version);  // name
+      reader.PopString(&spec_version);  // vendor
+      reader.PopString(&spec_version);  // version
+      reader.PopString(&spec_version);  // spec_version
+      std::vector<std::string> spec_version_parts = base::SplitString(
+          spec_version, ".", base::TRIM_WHITESPACE, base::SPLIT_WANT_NONEMPTY);
+      if (spec_version_parts.size() >= 2) {
+        base::StringToInt(spec_version_parts[0], &spec_version_major_);
+        base::StringToInt(spec_version_parts[1], &spec_version_minor_);
+      }
+    }
+    // The minimum supported spec version is 1.1, because this was the
+    // version that added image hints.
+    if (spec_version_major_ < 1 ||
+        (spec_version_major_ == 1 && spec_version_minor_ < 1)) {
+      OnConnectionInitializationFinishedOnTaskRunner(
+          ConnectionInitializationStatusCode::INCOMPATIBLE_SPEC_VERSION);
+      return;
+    }
+
     connected_signals_barrier_ = base::BarrierClosure(
         2, base::Bind(&NotificationPlatformBridgeLinuxImpl::
                           OnConnectionInitializationFinishedOnTaskRunner,
                       this, ConnectionInitializationStatusCode::SUCCESS));
     notification_proxy_->ConnectToSignal(
         kFreedesktopNotificationsName, "ActionInvoked",
         base::Bind(&NotificationPlatformBridgeLinuxImpl::OnActionInvoked, this),
         base::Bind(&NotificationPlatformBridgeLinuxImpl::OnSignalConnected,
                    this));
     notification_proxy_->ConnectToSignal(
@@ skipping 105 matching lines @@
     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);
 
     std::unique_ptr<ResourceFile> icon_file =
         WriteDataToTmpFile(notification->icon().As1xPNGBytes());
     if (icon_file) {
       dbus::MessageWriter image_path_writer(nullptr);
       hints_writer.OpenDictEntry(&image_path_writer);
-      image_path_writer.AppendString("image-path");
+      if (spec_version_major_ == 1 && spec_version_minor_ == 1) {
+        image_path_writer.AppendString("image_path");

[Lei Zhang, 2017/05/08 21:51:19]

image_path_writer.AppendString(some_eval ? "image_path" : "image-path"); ?

[Tom (Use chromium acct), 2017/05/08 22:46:07]

Done.

+      } else {
+        image_path_writer.AppendString("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);
 
@@ skipping 207 matching lines @@
 
   //////////////////////////////////////////////////////////////////////////////
   // Members used only on the task runner thread.
 
   scoped_refptr<dbus::Bus> bus_;
 
   dbus::ObjectProxy* notification_proxy_ = nullptr;
 
   std::unordered_set<std::string> capabilities_;
 
+  int spec_version_major_ = -1;

[Lei Zhang, 2017/05/08 21:51:19]

Can we make these local variables to InitOnTaskRunner(), and store an enum (<
1.1, 1.1, 1.2+) here instead?

[Tom (Use chromium acct), 2017/05/08 22:46:07]

Even better I think is a std::pair<int, int> (done in latest patch set). 
std::pair and std::tuple have lexicographic comparisons, so we can do eg.
"spec_version_ < std::make_pair(1, 1)"

[Lei Zhang, 2017/05/08 22:54:25]

FWIW, there's also base::Version. I'm not going to stress out too much about
this.

+  int spec_version_minor_ = -1;
+
   base::Closure connected_signals_barrier_;
 
   // A std::set<std::unique_ptr<T>> doesn't work well because
   // eg. std::set::erase(T) would require a std::unique_ptr<T>
   // argument, so the data would get double-destructed.
   template <typename T>
   using UnorderedUniqueSet = std::unordered_map<T*, std::unique_ptr<T>>;
 
   UnorderedUniqueSet<NotificationData> notifications_;
 
@@ skipping 35 matching lines @@
 }
 
 void NotificationPlatformBridgeLinux::SetReadyCallback(
     NotificationBridgeReadyCallback callback) {
   impl_->SetReadyCallback(std::move(callback));
 }
 
 void NotificationPlatformBridgeLinux::CleanUp() {
   impl_->CleanUp();
 }