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

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 dfd306ce06c18476918e433156a6a8a0a5fc4d6f..eb6f94ad7ff5c249a7070272c6a20e5775afdaf5 100644
--- a/chrome/browser/notifications/notification_platform_bridge_linux.cc
+++ b/chrome/browser/notifications/notification_platform_bridge_linux.cc
@@ -17,6 +17,7 @@
 #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"
@@ -51,6 +52,7 @@ enum class ConnectionInitializationStatusCode {
   NATIVE_NOTIFICATIONS_NOT_SUPPORTED = 1,
   MISSING_REQUIRED_CAPABILITIES = 2,
   COULD_NOT_CONNECT_TO_SIGNALS = 3,
+  INCOMPATIBLE_SPEC_VERSION = 4,
   NUM_ITEMS
 };
 
@@ -347,6 +349,35 @@ class NotificationPlatformBridgeLinuxImpl
     }
     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,
@@ -472,7 +503,11 @@ class NotificationPlatformBridgeLinuxImpl
     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));
@@ -700,6 +735,9 @@ class NotificationPlatformBridgeLinuxImpl
 
   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