Submitting CL http://codereview.chromium.org/2324001 on behalf of petkov@chromium.org:

Add the hardwareclass UMA log field for Chrome OS builds.

This field will be used to distinguish histograms across Chrome OS hardware
platforms.

BUG=none
TEST=unit tests, tested on target device by reviewing the logs


Review URL: http://codereview.chromium.org/2738003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@49197 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 187 insertions, 48 deletions

diff --git a/chrome/browser/metrics/metrics_log.cc b/chrome/browser/metrics/metrics_log.cc
index baa7ec3..cf25e58 100644
--- a/chrome/browser/metrics/metrics_log.cc
+++ b/chrome/browser/metrics/metrics_log.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
 
@@ -10,6 +10,7 @@
 #include "base/file_util.h"
 #include "base/file_version_info.h"
 #include "base/md5.h"
+#include "base/perftimer.h"
 #include "base/scoped_ptr.h"
 #include "base/string_util.h"
 #include "base/sys_info.h"
@@ -51,6 +52,7 @@ MetricsLog::MetricsLog(const std::string& client_id, int session_id)
       client_id_(client_id),
       session_id_(IntToString(session_id)),
       locked_(false),
+      doc_(NULL),
       buffer_(NULL),
       writer_(NULL),
       num_events_(0) {
@@ -58,7 +60,11 @@ MetricsLog::MetricsLog(const std::string& client_id, int session_id)
   buffer_ = xmlBufferCreate();
   DCHECK(buffer_);
 
-  writer_ = xmlNewTextWriterMemory(buffer_, 0);
+#if defined(OS_CHROMEOS)
+  writer_ = xmlNewTextWriterDoc(&doc_, /* compression */ 0);
+#else
+  writer_ = xmlNewTextWriterMemory(buffer_, /* compression */ 0);
+#endif  // OS_CHROMEOS
   DCHECK(writer_);
 
   int result = xmlTextWriterSetIndent(writer_, 2);
@@ -68,16 +74,15 @@ MetricsLog::MetricsLog(const std::string& client_id, int session_id)
   WriteAttribute("clientid", client_id_);
   WriteInt64Attribute("buildtime", GetBuildTime());
   WriteAttribute("appversion", GetVersionString());
-
-  DCHECK_GE(result, 0);
 }
 
 MetricsLog::~MetricsLog() {
-  if (writer_)
-    xmlFreeTextWriter(writer_);
+  FreeDocWriter();
 
-  if (buffer_)
+  if (buffer_) {
     xmlBufferFree(buffer_);
+    buffer_ = NULL;
+  }
 }
 
 void MetricsLog::CloseLog() {
@@ -89,6 +94,27 @@ void MetricsLog::CloseLog() {
 
   result = xmlTextWriterFlush(writer_);
   DCHECK_GE(result, 0);
+
+#if defined(OS_CHROMEOS)
+  xmlNodePtr root = xmlDocGetRootElement(doc_);
+  if (!hardware_class_.empty()) {
+    // The hardware class is determined after the first ongoing log is
+    // constructed, so this adds the root element's "hardwareclass"
+    // attribute when the log is closed instead.
+    xmlNewProp(root, UnsignedChar("hardwareclass"),
+               UnsignedChar(hardware_class_.c_str()));
+  }
+
+  // Flattens the XML tree into a character buffer.
+  PerfTimer dump_timer;
+  result = xmlNodeDump(buffer_, doc_, root, /* level */ 0, /* format */ 1);
+  DCHECK_GE(result, 0);
+  UMA_HISTOGRAM_TIMES("UMA.XMLNodeDumpTime", dump_timer.Elapsed());
+
+  PerfTimer free_timer;
+  FreeDocWriter();
+  UMA_HISTOGRAM_TIMES("UMA.XMLWriterDestructionTime", free_timer.Elapsed());
+#endif  // OS_CHROMEOS
 }
 
 int MetricsLog::GetEncodedLogSize() {
@@ -285,6 +311,18 @@ const char* MetricsLog::WindowEventTypeToString(WindowEventType type) {
   }
 }
 
+void MetricsLog::FreeDocWriter() {
+  if (writer_) {
+    xmlFreeTextWriter(writer_);
+    writer_ = NULL;
+  }
+
+  if (doc_) {
+    xmlFreeDoc(doc_);
+    doc_ = NULL;
+  }
+}
+
 void MetricsLog::StartElement(const char* name) {
   DCHECK(!locked_);
   DCHECK(name);
diff --git a/chrome/browser/metrics/metrics_log.h b/chrome/browser/metrics/metrics_log.h
index 18e3b32..d9d5f38 100644
--- a/chrome/browser/metrics/metrics_log.h
+++ b/chrome/browser/metrics/metrics_log.h
@@ -95,6 +95,9 @@ class MetricsLog {
   int GetElapsedSeconds();
 
   int num_events() { return num_events_; }
+  void set_hardware_class(const std::string& hardware_class) {
+    hardware_class_ = hardware_class;
+  }
 
   // Creates an MD5 hash of the given value, and returns hash as a byte
   // buffer encoded as a std::string.
@@ -107,7 +110,7 @@ class MetricsLog {
   static std::string GetVersionString();
 
   // Get the GMT buildtime for the current binary, expressed in seconds since
-  // Januray 1, 1970 GMT.
+  // January 1, 1970 GMT.
   // The value is used to identify when a new build is run, so that previous
   // reliability stats, from other builds, can be abandoned.
   static int64 GetBuildTime();
@@ -121,6 +124,7 @@ class MetricsLog {
   static void set_version_extension(const std::string& extension) {
     version_extension_ = extension;
   }
+
  protected:
   // Returns a string containing the current time.
   // Virtual so that it can be overridden for testing.
@@ -154,6 +158,11 @@ class MetricsLog {
 
   static const char* WindowEventTypeToString(WindowEventType type);
 
+  // Frees the resources allocated by the XML document writer: the
+  // main writer object as well as the XML tree structure, if
+  // applicable.
+  void FreeDocWriter();
+
   // Convenience versions of xmlWriter functions
   void StartElement(const char* name);
   void EndElement();
@@ -207,12 +216,14 @@ class MetricsLog {
 
   std::string client_id_;
   std::string session_id_;
+  std::string hardware_class_;
 
   // locked_ is true when record has been packed up for sending, and should
   // no longer be written to.  It is only used for sanity checking and is
   // not a real lock.
   bool locked_;
 
+  xmlDocPtr doc_;
   xmlBufferPtr buffer_;
   xmlTextWriterPtr writer_;
   int num_events_;  // the number of events recorded in this log
diff --git a/chrome/browser/metrics/metrics_log_unittest.cc b/chrome/browser/metrics/metrics_log_unittest.cc
index 4ac4cb8..f7f64ae 100644
--- a/chrome/browser/metrics/metrics_log_unittest.cc
+++ b/chrome/browser/metrics/metrics_log_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2006-2010 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.
 
@@ -50,13 +50,40 @@ TEST(MetricsLogTest, EmptyRecord) {
   ASSERT_GT(size, 0);
 
   std::string encoded;
-  ASSERT_TRUE(log.GetEncodedLog(WriteInto(&encoded, size), size));
+  // Leave room for the NUL terminator.
+  ASSERT_TRUE(log.GetEncodedLog(WriteInto(&encoded, size + 1), size));
+  TrimWhitespaceASCII(encoded, TRIM_ALL, &encoded);
   NormalizeBuildtime(&encoded);
   NormalizeBuildtime(&expected_output);
 
   ASSERT_EQ(expected_output, encoded);
 }
 
+#if defined(OS_CHROMEOS)
+TEST(MetricsLogTest, ChromeOSEmptyRecord) {
+  std::string expected_output = StringPrintf(
+      "<log clientid=\"bogus client ID\" buildtime=\"123456789\" "
+      "appversion=\"%s\" hardwareclass=\"sample-class\"/>",
+      MetricsLog::GetVersionString().c_str());
+
+  MetricsLog log("bogus client ID", 0);
+  log.set_hardware_class("sample-class");
+  log.CloseLog();
+
+  int size = log.GetEncodedLogSize();
+  ASSERT_GT(size, 0);
+
+  std::string encoded;
+  // Leave room for the NUL terminator.
+  ASSERT_TRUE(log.GetEncodedLog(WriteInto(&encoded, size + 1), size));
+  TrimWhitespaceASCII(encoded, TRIM_ALL, &encoded);
+  NormalizeBuildtime(&encoded);
+  NormalizeBuildtime(&expected_output);
+
+  ASSERT_EQ(expected_output, encoded);
+}
+#endif  // OS_CHROMEOS
+
 namespace {
 
 class NoTimeMetricsLog : public MetricsLog {
@@ -98,7 +125,9 @@ TEST(MetricsLogTest, WindowEvent) {
   ASSERT_GT(size, 0);
 
   std::string encoded;
-  ASSERT_TRUE(log.GetEncodedLog(WriteInto(&encoded, size), size));
+  // Leave room for the NUL terminator.
+  ASSERT_TRUE(log.GetEncodedLog(WriteInto(&encoded, size + 1), size));
+  TrimWhitespaceASCII(encoded, TRIM_ALL, &encoded);
   NormalizeBuildtime(&encoded);
   NormalizeBuildtime(&expected_output);
 
@@ -125,7 +154,9 @@ TEST(MetricsLogTest, LoadEvent) {
   ASSERT_GT(size, 0);
 
   std::string encoded;
-  ASSERT_TRUE(log.GetEncodedLog(WriteInto(&encoded, size), size));
+  // Leave room for the NUL terminator.
+  ASSERT_TRUE(log.GetEncodedLog(WriteInto(&encoded, size + 1), size));
+  TrimWhitespaceASCII(encoded, TRIM_ALL, &encoded);
   NormalizeBuildtime(&encoded);
   NormalizeBuildtime(&expected_output);
 
diff --git a/chrome/browser/metrics/metrics_service.cc b/chrome/browser/metrics/metrics_service.cc
index 90e4ce5..cc46ab6 100644
--- a/chrome/browser/metrics/metrics_service.cc
+++ b/chrome/browser/metrics/metrics_service.cc
@@ -82,8 +82,8 @@
 // are:
 //
 //    INITIALIZED,            // Constructor was called.
-//    PLUGIN_LIST_REQUESTED,  // Waiting for plugin list to be loaded.
-//    PLUGIN_LIST_ARRIVED,    // Waiting for timer to send initial log.
+//    INIT_TASK_SCHEDULED,    // Waiting for deferred init tasks to complete.
+//    INIT_TASK_DONE,         // Waiting for timer to send initial log.
 //    INITIAL_LOG_READY,      // Initial log generated, and waiting for reply.
 //    SEND_OLD_INITIAL_LOGS,  // Sending unsent logs from previous session.
 //    SENDING_OLD_LOGS,       // Sending unsent logs from previous session.
@@ -95,15 +95,17 @@
 // The MS has been constructed, but has taken no actions to compose the
 // initial log.
 //
-//    PLUGIN_LIST_REQUESTED,  // Waiting for plugin list to be loaded.
+//    INIT_TASK_SCHEDULED,    // Waiting for deferred init tasks to complete.
 // Typically about 30 seconds after startup, a task is sent to a second thread
-// to get the list of plugins.  That task will (when complete) make an async
-// callback (via a Task) to indicate the completion.
+// (the file thread) to perform deferred (lower priority and slower)
+// initialization steps such as getting the list of plugins.  That task will
+// (when complete) make an async callback (via a Task) to indicate the
+// completion.
 //
-//    PLUGIN_LIST_ARRIVED,    // Waiting for timer to send initial log.
+//    INIT_TASK_DONE,         // Waiting for timer to send initial log.
 // The callback has arrived, and it is now possible for an initial log to be
 // created.  This callback typically arrives back less than one second after
-// the task is dispatched.
+// the deferred init task is dispatched.
 //
 //    INITIAL_LOG_READY,      // Initial log generated, and waiting for reply.
 // This state is entered only after an initial log has been composed, and
@@ -197,6 +199,9 @@
 
 #if defined(OS_CHROMEOS)
 #include "chrome/browser/chromeos/external_metrics.h"
+
+static const char kHardwareClassTool[] = "/usr/bin/hardware_class";
+static const char kUnknownHardwareClass[] = "unknown";
 #endif
 
 using base::Time;
@@ -282,29 +287,36 @@ class MetricsMemoryDetails : public MemoryDetails {
   DISALLOW_COPY_AND_ASSIGN(MetricsMemoryDetails);
 };
 
-class MetricsService::GetPluginListTaskComplete : public Task {
+class MetricsService::InitTaskComplete : public Task {
  public:
-  explicit GetPluginListTaskComplete(
-      const std::vector<WebPluginInfo>& plugins) : plugins_(plugins) { }
+  explicit InitTaskComplete(const std::string& hardware_class,
+                            const std::vector<WebPluginInfo>& plugins)
+      : hardware_class_(hardware_class), plugins_(plugins) {}
+
   virtual void Run() {
-    g_browser_process->metrics_service()->OnGetPluginListTaskComplete(plugins_);
+    g_browser_process->metrics_service()->OnInitTaskComplete(
+        hardware_class_, plugins_);
   }
 
  private:
+  std::string hardware_class_;
   std::vector<WebPluginInfo> plugins_;
 };
 
-class MetricsService::GetPluginListTask : public Task {
+class MetricsService::InitTask : public Task {
  public:
-  explicit GetPluginListTask(MessageLoop* callback_loop)
+  explicit InitTask(MessageLoop* callback_loop)
       : callback_loop_(callback_loop) {}
 
   virtual void Run() {
     std::vector<WebPluginInfo> plugins;
     NPAPI::PluginList::Singleton()->GetPlugins(false, &plugins);
-
-    callback_loop_->PostTask(
-        FROM_HERE, new GetPluginListTaskComplete(plugins));
+    std::string hardware_class;  // Empty string by default.
+#if defined(OS_CHROMEOS)
+    hardware_class = MetricsService::GetHardwareClass();
+#endif  // OS_CHROMEOS
+    callback_loop_->PostTask(FROM_HERE, new InitTaskComplete(
+        hardware_class, plugins));
   }
 
  private:
@@ -745,12 +757,14 @@ void MetricsService::InitializeMetricsState() {
   ScheduleNextStateSave();
 }
 
-void MetricsService::OnGetPluginListTaskComplete(
+void MetricsService::OnInitTaskComplete(
+    const std::string& hardware_class,
     const std::vector<WebPluginInfo>& plugins) {
-  DCHECK(state_ == PLUGIN_LIST_REQUESTED);
+  DCHECK(state_ == INIT_TASK_SCHEDULED);
+  hardware_class_ = hardware_class;
   plugins_ = plugins;
-  if (state_ == PLUGIN_LIST_REQUESTED)
-    state_ = PLUGIN_LIST_ARRIVED;
+  if (state_ == INIT_TASK_SCHEDULED)
+    state_ = INIT_TASK_DONE;
 }
 
 std::string MetricsService::GenerateClientID() {
@@ -822,12 +836,14 @@ void MetricsService::StartRecording() {
   current_log_ = new MetricsLog(client_id_, session_id_);
   if (state_ == INITIALIZED) {
     // We only need to schedule that run once.
-    state_ = PLUGIN_LIST_REQUESTED;
+    state_ = INIT_TASK_SCHEDULED;
 
-    // Make sure the plugin list is loaded before the inital log is sent, so
-    // that the main thread isn't blocked generating the list.
+    // Schedules a task on the file thread for execution of slower
+    // initialization steps (such as plugin list generation) necessary
+    // for sending the initial log.  This avoids blocking the main UI
+    // thread.
     g_browser_process->file_thread()->message_loop()->PostDelayedTask(FROM_HERE,
-        new GetPluginListTask(MessageLoop::current()),
+        new InitTask(MessageLoop::current()),
         kInitialInterlogDuration * 1000 / 2);
   }
 }
@@ -836,6 +852,8 @@ void MetricsService::StopRecording(MetricsLog** log) {
   if (!current_log_)
     return;
 
+  current_log_->set_hardware_class(hardware_class_);  // Adds to ongoing logs.
+
   // TODO(jar): Integrate bounds on log recording more consistently, so that we
   // can stop recording logs that are too big much sooner.
   if (current_log_->num_events() > log_event_limit_) {
@@ -1050,16 +1068,16 @@ void MetricsService::MakePendingLog() {
 
   switch (state_) {
     case INITIALIZED:
-    case PLUGIN_LIST_REQUESTED:  // We should be further along by now.
+    case INIT_TASK_SCHEDULED:  // We should be further along by now.
       DCHECK(false);
       return;
 
-    case PLUGIN_LIST_ARRIVED:
+    case INIT_TASK_DONE:
       // We need to wait for the initial log to be ready before sending
       // anything, because the server will tell us whether it wants to hear
       // from us.
       PrepareInitialLog();
-      DCHECK(state_ == PLUGIN_LIST_ARRIVED);
+      DCHECK(state_ == INIT_TASK_DONE);
       RecallUnsentLogs();
       state_ = INITIAL_LOG_READY;
       break;
@@ -1116,9 +1134,10 @@ bool MetricsService::TransmissionPermitted() const {
 }
 
 void MetricsService::PrepareInitialLog() {
-  DCHECK(state_ == PLUGIN_LIST_ARRIVED);
+  DCHECK(state_ == INIT_TASK_DONE);
 
   MetricsLog* log = new MetricsLog(client_id_, session_id_);
+  log->set_hardware_class(hardware_class_);  // Adds to initial log.
   log->RecordEnvironment(plugins_, profile_dictionary_.get());
 
   // Histograms only get written to current_log_, so setup for the write.
@@ -1910,6 +1929,20 @@ static bool IsSingleThreaded() {
 }
 
 #if defined(OS_CHROMEOS)
+// static
+std::string MetricsService::GetHardwareClass() {
+  DCHECK(!ChromeThread::CurrentlyOn(ChromeThread::UI));
+  std::string hardware_class;
+  FilePath tool(kHardwareClassTool);
+  CommandLine command(tool);
+  if (base::GetAppOutput(command, &hardware_class)) {
+    TrimWhitespaceASCII(hardware_class, TRIM_ALL, &hardware_class);
+  } else {
+    hardware_class = kUnknownHardwareClass;
+  }
+  return hardware_class;
+}
+
 void MetricsService::StartExternalMetrics() {
   external_metrics_ = new chromeos::ExternalMetrics;
   external_metrics_->Start();
diff --git a/chrome/browser/metrics/metrics_service.h b/chrome/browser/metrics/metrics_service.h
index 51a22c3..9292856 100644
--- a/chrome/browser/metrics/metrics_service.h
+++ b/chrome/browser/metrics/metrics_service.h
@@ -116,14 +116,19 @@ class MetricsService : public NotificationObserver,
   // This count is eventually send via UMA logs.
   void RecordBreakpadHasDebugger(bool has_debugger);
 
-  // Callback to let us knew that the plugin list is warmed up.
-  void OnGetPluginListTaskComplete(const std::vector<WebPluginInfo>& plugins);
-
   // Save any unsent logs into a persistent store in a pref.  We always do this
   // at shutdown, but we can do it as we reduce the list as well.
   void StoreUnsentLogs();
 
 #if defined(OS_CHROMEOS)
+  // Returns the hardware class of the Chrome OS device (e.g.,
+  // hardware qualification ID), or "unknown" if the hardware class is
+  // not available.  The hardware class identifies the configured
+  // system components such us CPU, WiFi adapter, etc.  Note that this
+  // routine invokes an external utility to determine the hardware
+  // class.
+  static std::string GetHardwareClass();
+
   // Start the external metrics service, which collects metrics from Chrome OS
   // and passes them to UMA.
   void StartExternalMetrics();
@@ -137,8 +142,8 @@ class MetricsService : public NotificationObserver,
   // See metrics_service.cc for description of this lifecycle.
   enum State {
     INITIALIZED,            // Constructor was called.
-    PLUGIN_LIST_REQUESTED,  // Waiting for plugin list to be loaded.
-    PLUGIN_LIST_ARRIVED,    // Waiting for timer to send initial log.
+    INIT_TASK_SCHEDULED,    // Waiting for deferred init tasks to complete.
+    INIT_TASK_DONE,         // Waiting for timer to send initial log.
     INITIAL_LOG_READY,      // Initial log generated, and waiting for reply.
     SEND_OLD_INITIAL_LOGS,  // Sending unsent logs from previous session.
     SENDING_OLD_LOGS,       // Sending unsent logs from previous session.
@@ -148,8 +153,13 @@ class MetricsService : public NotificationObserver,
   // Maintain a map of histogram names to the sample stats we've sent.
   typedef std::map<std::string, Histogram::SampleSet> LoggedSampleMap;
 
-  class GetPluginListTask;
-  class GetPluginListTaskComplete;
+  class InitTask;
+  class InitTaskComplete;
+
+  // Callback to let us know that the init task is done.
+  void OnInitTaskComplete(
+      const std::string& hardware_class,
+      const std::vector<WebPluginInfo>& plugins);
 
   // When we start a new version of Chromium (different from our last run), we
   // need to discard the old crash stats so that we don't attribute crashes etc.
@@ -422,6 +432,12 @@ class MetricsService : public NotificationObserver,
   // state.
   State state_;
 
+  // Chrome OS hardware class (e.g., hardware qualification ID). This
+  // class identifies the configured system components such as CPU,
+  // WiFi adapter, etc.  For non Chrome OS hosts, this will be an
+  // empty string.
+  std::string hardware_class_;
+
   // The list of plugins which was retrieved on the file thread.
   std::vector<WebPluginInfo> plugins_;
 
diff --git a/chrome/browser/metrics/metrics_service_unittest.cc b/chrome/browser/metrics/metrics_service_unittest.cc
index 1bd5c77a..9de7644 100644
--- a/chrome/browser/metrics/metrics_service_unittest.cc
+++ b/chrome/browser/metrics/metrics_service_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
 
@@ -33,3 +33,13 @@ TEST(MetricsServiceTest, ClientIdCorrectlyFormatted) {
   }
 }
 #endif
+
+#if defined(OS_CHROMEOS)
+TEST(MetricsServiceTest, GetHardwareClass) {
+  // The assumption is that unit tests run on the build host rather
+  // than on the Chrome OS device so the hardware_class tool is not
+  // available.
+  std::string hardware_class = MetricsService::GetHardwareClass();
+  EXPECT_EQ("unknown", hardware_class);
+}
+#endif  // OS_CHROMEOS