Chrome Remote Desktop will collect and report the host OS name and version.

The Chrome Remote Desktop daemon application will collect the host OS and
versionHost OS name and version will then be collected by the Chrome Remote backend for analysis on error reports to help improve the stability of the product.
Version was missing on linux systems.

R=kelvinp@chromium.org, sergeyu@chromium.org
BUG=534902

Review URL: https://codereview.chromium.org/1365983002

Cr-Commit-Position: refs/heads/master@{#351366}

7 files changed, 116 insertions, 31 deletions

diff --git a/remoting/host/heartbeat_sender.cc b/remoting/host/heartbeat_sender.cc
index d244f1e..3c60ebe 100644
--- a/remoting/host/heartbeat_sender.cc
+++ b/remoting/host/heartbeat_sender.cc
@@ -15,6 +15,7 @@
 #include "base/time/time.h"
 #include "remoting/base/constants.h"
 #include "remoting/base/logging.h"
+#include "remoting/host/host_details.h"
 #include "remoting/host/server_log_entry_host.h"
 #include "remoting/signaling/iq_sender.h"
 #include "remoting/signaling/jid_util.h"
@@ -36,6 +37,8 @@ const char kHostVersionTag[] = "host-version";
 const char kHeartbeatSignatureTag[] = "signature";
 const char kSequenceIdAttr[] = "sequence-id";
 const char kHostOfflineReasonAttr[] = "host-offline-reason";
+const char kHostOperatingSystemNameTag[] = "os-name";
+const char kHostOperatingSystemVersionTag[] = "os-version";
 
 const char kErrorTag[] = "error";
 const char kNotFoundTag[] = "item-not-found";
@@ -319,6 +322,19 @@ scoped_ptr<XmlElement> HeartbeatSender::CreateHeartbeatMessage() {
       QName(kChromotingXmlNamespace, kHostVersionTag)));
   version_tag->AddText(STRINGIZE(VERSION));
   heartbeat->AddElement(version_tag.release());
+  // If we have not recorded a heartbeat success, continue sending host OS info.
+  if (!heartbeat_succeeded_) {
+    // Append host OS name.
+    scoped_ptr<XmlElement> os_name_tag(new XmlElement(
+        QName(kChromotingXmlNamespace, kHostOperatingSystemNameTag)));
+    os_name_tag->AddText(GetHostOperatingSystemName());
+    heartbeat->AddElement(os_name_tag.release());
+    // Append host OS version.
+    scoped_ptr<XmlElement> os_version_tag(new XmlElement(
+        QName(kChromotingXmlNamespace, kHostOperatingSystemVersionTag)));
+    os_version_tag->AddText(GetHostOperatingSystemVersion());
+    heartbeat->AddElement(os_version_tag.release());
+  }
   // Append log message (which isn't signed).
   scoped_ptr<XmlElement> log(ServerLogEntry::MakeStanza());
   scoped_ptr<ServerLogEntry> log_entry(MakeLogEntryForHeartbeat());
diff --git a/remoting/host/heartbeat_sender_unittest.cc b/remoting/host/heartbeat_sender_unittest.cc
index 35a9782..33ef1f4 100644
--- a/remoting/host/heartbeat_sender_unittest.cc
+++ b/remoting/host/heartbeat_sender_unittest.cc
@@ -69,7 +69,6 @@ class HeartbeatSenderTest
   void SetUp() override {
     key_pair_ = RsaKeyPair::FromString(kTestRsaKeyPair);
     ASSERT_TRUE(key_pair_.get());
-
     EXPECT_CALL(signal_strategy_, GetState())
         .WillOnce(Return(SignalStrategy::DISCONNECTED));
     EXPECT_CALL(signal_strategy_, AddListener(NotNull()))
@@ -332,6 +331,39 @@ TEST_F(HeartbeatSenderTest, ProcessHostOfflineResponses) {
   base::RunLoop().RunUntilIdle();
 }
 
+// The first heartbeat should include host OS information.
+TEST_F(HeartbeatSenderTest, HostOSInfo) {
+  XmlElement* sent_iq = nullptr;
+  EXPECT_CALL(signal_strategy_, GetLocalJid())
+      .WillRepeatedly(Return(kTestJid));
+  EXPECT_CALL(signal_strategy_, GetNextId())
+      .WillOnce(Return(kStanzaId));
+  EXPECT_CALL(signal_strategy_, SendStanzaPtr(NotNull()))
+      .WillOnce(DoAll(SaveArg<0>(&sent_iq), Return(true)));
+  EXPECT_CALL(signal_strategy_, GetState())
+      .WillRepeatedly(Return(SignalStrategy::CONNECTED));
+
+  heartbeat_sender_->OnSignalStrategyStateChange(SignalStrategy::CONNECTED);
+  base::RunLoop().RunUntilIdle();
+
+  scoped_ptr<XmlElement> stanza(sent_iq);
+  ASSERT_TRUE(stanza != nullptr);
+
+  XmlElement* heartbeat_stanza =
+      stanza->FirstNamed(QName(kChromotingXmlNamespace, "heartbeat"));
+
+  std::string os_name =
+      heartbeat_stanza->TextNamed(QName(kChromotingXmlNamespace, "os-name"));
+  EXPECT_TRUE(!os_name.empty());
+
+  std::string os_version =
+      heartbeat_stanza->TextNamed(QName(kChromotingXmlNamespace, "os-version"));
+  EXPECT_TRUE(!os_version.empty());
+
+  heartbeat_sender_->OnSignalStrategyStateChange(SignalStrategy::DISCONNECTED);
+  base::RunLoop().RunUntilIdle();
+}
+
 // Validate a heartbeat stanza.
 void HeartbeatSenderTest::ValidateHeartbeatStanza(
     XmlElement* stanza,
diff --git a/remoting/host/host_details.cc b/remoting/host/host_details.cc
new file mode 100644
index 0000000..d9e792a
--- /dev/null
+++ b/remoting/host/host_details.cc
@@ -0,0 +1,39 @@
+// Copyright 2015 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 "remoting/host/host_details.h"
+
+#include "base/sys_info.h"
+
+#if defined(OS_LINUX)
+#include "base/linux_util.h"
+#endif
+
+namespace remoting {
+
+// Get the host Operating System Name, removing the need to check for OS
+// definitions and keeps the keys used consistant.
+std::string GetHostOperatingSystemName() {
+#if defined(OS_WIN)
+  return "Windows";
+#elif defined(OS_MACOSX)
+  return "Mac";
+#elif defined(OS_CHROMEOS)
+  return "ChromeOS";
+#elif defined(OS_LINUX)
+  return "Linux";
+#endif
+}
+
+// Get the host Operating System Version, removing the need to check for OS
+// definitions and keeps the format used consistant.
+std::string GetHostOperatingSystemVersion() {
+#if defined(OS_WIN) || defined(OS_MACOSX) || defined(OS_CHROMEOS)
+  return base::SysInfo::OperatingSystemVersion();
+#elif defined(OS_LINUX)
+  return base::GetLinuxDistro();
+#endif
+}
+
+}  // namespace remoting
diff --git a/remoting/host/host_details.h b/remoting/host/host_details.h
new file mode 100644
index 0000000..fdc010b
--- /dev/null
+++ b/remoting/host/host_details.h
@@ -0,0 +1,20 @@
+// Copyright 2015 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.
+
+#ifndef REMOTING_HOST_HOST_DETAILS_H_
+#define REMOTING_HOST_HOST_DETAILS_H_
+
+#include <string>
+
+namespace remoting {
+
+// Returns the host OS name in a standard format for any build target.
+std::string GetHostOperatingSystemName();
+
+// Returns the host OS version in a standard format for any build target.
+std::string GetHostOperatingSystemVersion();
+
+}  // namespace remoting
+
+#endif  // REMOTING_HOST_HOST_DETAILS_H_
diff --git a/remoting/host/server_log_entry_host.cc b/remoting/host/server_log_entry_host.cc
index 7c25865..f864496 100644
--- a/remoting/host/server_log_entry_host.cc
+++ b/remoting/host/server_log_entry_host.cc
@@ -5,11 +5,9 @@
 #include "remoting/host/server_log_entry_host.h"
 
 #include "base/strings/stringize_macros.h"
-#include "base/sys_info.h"
+#include "remoting/host/host_details.h"
 #include "remoting/signaling/server_log_entry.h"
 
-using base::SysInfo;
-
 namespace remoting {
 
 namespace {
@@ -23,10 +21,7 @@ const char kValueSessionStateConnected[] = "connected";
 const char kValueSessionStateClosed[] = "closed";
 
 const char kKeyOsName[] = "os-name";
-
-#if defined(OS_WIN) || defined(OS_MACOSX) || defined(OS_CHROMEOS)
 const char kKeyOsVersion[] = "os-version";
-#endif
 
 const char kKeyHostVersion[] = "host-version";
 
@@ -55,30 +50,10 @@ scoped_ptr<ServerLogEntry> MakeLogEntryForHeartbeat() {
 }
 
 void AddHostFieldsToLogEntry(ServerLogEntry* entry) {
-#if defined(OS_WIN)
-  entry->Set(kKeyOsName, "Windows");
-#elif defined(OS_MACOSX)
-  entry->Set(kKeyOsName, "Mac");
-#elif defined(OS_CHROMEOS)
-  entry->Set(kKeyOsName, "ChromeOS");
-#elif defined(OS_LINUX)
-  entry->Set(kKeyOsName, "Linux");
-#endif
-
-  // SysInfo::OperatingSystemVersionNumbers is only defined for the following
-  // OSes: see base/sys_info_unittest.cc.
-#if defined(OS_WIN) || defined(OS_MACOSX) || defined(OS_CHROMEOS)
-  std::stringstream os_version;
-  int32 os_major_version = 0;
-  int32 os_minor_version = 0;
-  int32 os_bugfix_version = 0;
-  SysInfo::OperatingSystemVersionNumbers(&os_major_version, &os_minor_version,
-                                         &os_bugfix_version);
-  os_version << os_major_version << "." << os_minor_version << "."
-             << os_bugfix_version;
-  entry->Set(kKeyOsVersion, os_version.str());
-#endif
-
+  // TODO os name, os version, and version will be in the main message body,
+  // remove these fields at a later date to remove redundancy.
+  entry->Set(kKeyOsName, GetHostOperatingSystemName());
+  entry->Set(kKeyOsVersion, GetHostOperatingSystemVersion());
   entry->Set(kKeyHostVersion, STRINGIZE(VERSION));
   entry->AddCpuField();
 };
diff --git a/remoting/host/server_log_entry_host_unittest.cc b/remoting/host/server_log_entry_host_unittest.cc
index d249c03..7d69eec 100644
--- a/remoting/host/server_log_entry_host_unittest.cc
+++ b/remoting/host/server_log_entry_host_unittest.cc
@@ -62,6 +62,7 @@ TEST(ServerLogEntryHostTest, AddHostFields) {
   keys.insert("os-version");
 #elif defined(OS_LINUX)
   key_value_pairs["os-name"] = "Linux";
+  keys.insert("os-version");
 #endif
 
   // The check below will compile but fail if VERSION isn't defined (STRINGIZE
diff --git a/remoting/remoting_host_srcs.gypi b/remoting/remoting_host_srcs.gypi
index a23c7ae..363a67d 100644
--- a/remoting/remoting_host_srcs.gypi
+++ b/remoting/remoting_host_srcs.gypi
@@ -114,6 +114,8 @@
       'host/host_config.cc',
       'host/host_config.h',
       'host/host_config_constants.cc',
+      'host/host_details.cc',
+      'host/host_details.h',
       'host/host_event_logger.h',
       'host/host_event_logger_posix.cc',
       'host/host_event_logger_win.cc',