Use Chrome to transport Chrome OS metrics.

Chrome periodically reads the content of a well-know file, 
and parses it into name-value pairs, each representing a 
Chrome OS metrics event. The events are then logged using 
the normal UMA mechanism. The file is then truncated to 
zero size. Chrome uses flock() to synchronize accesses 
to the file. 
BUG=none 
TEST=compiled and run Linux and Chrome OS versions. 
Verified that uploaded Chrome OS events appear in about:histograms. 
Also external_metrics_unittest.cc tests the collection of metrics 
messages from the well-known file.

patch written by semenzato_google.com
original code review: http://codereview.chromium.org/378013
(plus http://codereview.chromium.org/346041)


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

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

11 files changed, 495 insertions, 134 deletions

diff --git a/chrome/browser/browser_main.cc b/chrome/browser/browser_main.cc
index dc82b1fd..81d8c7c 100644
--- a/chrome/browser/browser_main.cc
+++ b/chrome/browser/browser_main.cc
@@ -124,6 +124,7 @@
 #if defined(OS_CHROMEOS)
 #include "chrome/browser/chromeos/cros_library.h"
 #include "chrome/browser/chromeos/external_cookie_handler.h"
+#include "chrome/browser/chromeos/external_metrics.h"
 #endif
 
 namespace {
@@ -870,6 +871,10 @@ int BrowserMain(const MainFunctionParams& parameters) {
   // should display the entry in the context menu or not.
   browser_process->CheckForInspectorFiles();
 
+#if defined(OS_CHROMEOS)
+  metrics->StartExternalMetrics(profile);
+#endif
+
   int result_code = ResultCodes::NORMAL_EXIT;
   if (parameters.ui_task) {
     // We are in test mode. Run one task and enter the main message loop.
diff --git a/chrome/browser/chromeos/external_metrics.cc b/chrome/browser/chromeos/external_metrics.cc
new file mode 100644
index 0000000..27c3f31
--- /dev/null
+++ b/chrome/browser/chromeos/external_metrics.cc
@@ -0,0 +1,252 @@
+// Copyright (c) 2009 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/chromeos/external_metrics.h"
+
+#include <fcntl.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <sys/file.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+
+#include "base/basictypes.h"
+#include "base/eintr_wrapper.h"
+#include "base/histogram.h"
+#include "base/time.h"
+#include "chrome/browser/metrics/user_metrics.h"
+#include "chrome/browser/profile.h"
+
+namespace chromeos {
+
+// The interval between external metrics collections, in milliseconds.
+static const int kExternalMetricsCollectionIntervalMs = 30 * 1000;
+
+// External_metrics_profile could be a member of ExternalMetrics, but then all
+// the RecordSomething functions would have to be member functions too, and
+// would have to be declared in the class, and there is really no point.
+static Profile* external_metrics_profile = NULL;
+
+// There is one of the following functions for every user action as we have to
+// call RecordAction in a way that gets picked up by the processing scripts.
+
+static void RecordTabOverviewKeystroke(const char* ignore) {
+  UserMetrics::RecordAction(L"TabOverview_Keystroke", external_metrics_profile);
+}
+
+static void RecordTabOverviewExitMouse(const char* ignore) {
+  UserMetrics::RecordAction(L"TabOverview_ExitMouse", external_metrics_profile);
+}
+
+static void RecordBootTime(const char* info) {
+  // Time string is a whole number of milliseconds.
+  int64 time = atol(info);
+  UMA_HISTOGRAM_CUSTOM_TIMES("ChromeOS.Boot Time",
+                             base::TimeDelta::FromMilliseconds(time),
+                             base::TimeDelta::FromSeconds(0),
+                             base::TimeDelta::FromSeconds(60),
+                             100);
+}
+
+static void RecordUpTime(const char* info) {
+  int64 time = atol(info);
+  UMA_HISTOGRAM_LONG_TIMES("ChromeOS.Uptime",
+                           base::TimeDelta::FromSeconds(time));
+}
+
+void ExternalMetrics::Start(Profile* profile) {
+  DCHECK(external_metrics_profile == NULL);
+  external_metrics_profile = profile;
+  SetRecorder(&RecordEvent);
+  InitializeFunctionTable();
+  ScheduleCollector();
+}
+
+ExternalMetrics::~ExternalMetrics() {
+  LOG_IF(WARNING, external_metrics_profile == NULL) <<
+    "external metrics class was never started";
+  external_metrics_profile = NULL;
+}
+
+void ExternalMetrics::RecordActionWrapper(RecordFunctionType f) {
+  if (external_metrics_profile != NULL) {
+    f(NULL);
+  }
+}
+
+// Record Function Entry
+#define RF_ENTRY(s, type) { #s, Record ## s, type }
+
+ExternalMetrics::RecordFunctionTableEntry ExternalMetrics::function_table_[] = {
+  // These entries MUST be in alphabetical order.
+  RF_ENTRY(BootTime, EVENT_TYPE_HISTOGRAM),
+  RF_ENTRY(TabOverviewExitMouse, EVENT_TYPE_ACTION),
+  RF_ENTRY(TabOverviewKeystroke, EVENT_TYPE_ACTION),
+  RF_ENTRY(UpTime, EVENT_TYPE_HISTOGRAM),
+};
+
+// Finds the table entry for |name|.
+const ExternalMetrics::RecordFunctionTableEntry*
+  ExternalMetrics::FindRecordEntry(const char* name) {
+  // Use binary search. (TODO(semenzato): map, hash map?)
+  int low = 0;
+  int high = ARRAYSIZE_UNSAFE(function_table_);
+
+  while (low < high) {
+    int middle = (high + low) / 2;
+    int comparison = strcmp(name, function_table_[middle].name);
+    if (comparison == 0) {
+      return &function_table_[middle];
+    } else if (comparison < 0) {
+      high = middle;
+    } else {
+      low = middle + 1;
+    }
+  }
+  return NULL;
+}
+
+void ExternalMetrics::InitializeFunctionTable() {
+  int n = ARRAYSIZE_UNSAFE(function_table_);
+  // Check that table is in alphabetical order.  This should be a compile-time
+  // check, but this ain't Lisp so we settle for checking the debug builds.
+  for (int i = 0; i < n - 1; i++) {
+    DCHECK(strcmp(function_table_[i].name, function_table_[i+1].name) < 0);
+  }
+}
+
+void ExternalMetrics::RecordEvent(const char* name, const char* value) {
+  const RecordFunctionTableEntry* entry = FindRecordEntry(name);
+  if (entry == NULL) {
+    // TODO(semenzato): should do this only once for each name.
+    LOG(WARNING) << "no UMA recording function for external event " << name;
+  } else {
+    switch (entry->type) {
+    case EVENT_TYPE_ACTION:
+      ChromeThread::PostTask(
+        ChromeThread::UI, FROM_HERE,
+        NewRunnableFunction(&RecordActionWrapper, entry->function));
+      break;
+    case EVENT_TYPE_HISTOGRAM:
+      entry->function(value);
+      break;
+    default:
+      NOTREACHED();
+      break;
+    }
+  }
+}
+
+void ExternalMetrics::CollectEvents() {
+  const char* event_file_path = "/tmp/.chromeos-metrics";
+  struct stat stat_buf;
+  int result;
+  result = stat(event_file_path, &stat_buf);
+  if (result < 0) {
+    if (errno != ENOENT) {
+      PLOG(ERROR) << event_file_path << ": cannot collect metrics";
+    }
+    // Nothing to collect---try later.
+    return;
+  }
+  if (stat_buf.st_size == 0) {
+    // Also nothing to collect.
+    return;
+  }
+  int fd = open(event_file_path, O_RDWR);
+  if (fd < 0) {
+    PLOG(ERROR) << event_file_path << ": cannot open";
+    return;
+  }
+  result = flock(fd, LOCK_EX);
+  if (result < 0) {
+    PLOG(ERROR) << event_file_path << ": cannot lock";
+    return;
+  }
+  // This processes all messages in the log.  Each message starts with a 4-byte
+  // field containing the length of the entire message.  The length is followed
+  // by a name-value pair of null-terminated strings.  When all messages are
+  // read and processed, or an error occurs, truncate the file to zero size.
+  for (;;) {
+    int32 message_size;
+    result = HANDLE_EINTR(read(fd, &message_size, sizeof(message_size)));
+    if (result < 0) {
+      PLOG(ERROR) << "reading metrics message header";
+      break;
+    }
+    if (result == 0) {  // normal EOF
+      break;
+    }
+    if (result < static_cast<int>(sizeof(message_size))) {
+      LOG(ERROR) << "bad read size " << result <<
+                    ", expecting " << sizeof(message_size);
+      break;
+    }
+    // kMetricsMessageMaxLength applies to the entire message: the 4-byte
+    // length field and the two null-terminated strings.
+    if (message_size < 2 + static_cast<int>(sizeof(message_size)) ||
+        message_size > static_cast<int>(kMetricsMessageMaxLength)) {
+      LOG(ERROR) << "bad message size " << message_size;
+      break;
+    }
+    message_size -= sizeof(message_size);  // already read this much
+    uint8 buffer[kMetricsMessageMaxLength];
+    result = HANDLE_EINTR(read(fd, buffer, message_size));
+    if (result < 0) {
+      PLOG(ERROR) << "reading metrics message body";
+      break;
+    }
+    if (result < message_size) {
+      LOG(ERROR) << "message too short: length " << result <<
+                    ", expected " << message_size;
+      break;
+    }
+    // The buffer should now contain a pair of null-terminated strings.
+    uint8* p = reinterpret_cast<uint8*>(memchr(buffer, '\0', message_size));
+    uint8* q = NULL;
+    if (p != NULL) {
+      q = reinterpret_cast<uint8*>(
+        memchr(p + 1, '\0', message_size - (p + 1 - buffer)));
+    }
+    if (q == NULL) {
+      LOG(ERROR) << "bad name-value pair for metrics";
+      break;
+    } else {
+      char* name = reinterpret_cast<char*>(buffer);
+      char* value = reinterpret_cast<char*>(p + 1);
+      recorder_(name, value);
+    }
+  }
+
+  result = ftruncate(fd, 0);
+  if (result < 0) {
+    PLOG(ERROR) << "truncate metrics log";
+  }
+  result = flock(fd, LOCK_UN);
+  if (result < 0) {
+    PLOG(ERROR) << "unlock metrics log";
+  }
+  result = close(fd);
+  if (result < 0) {
+    PLOG(ERROR) << "close metrics log";
+  }
+}
+
+void ExternalMetrics::CollectEventsAndReschedule() {
+  CollectEvents();
+  ScheduleCollector();
+}
+
+void ExternalMetrics::ScheduleCollector() {
+  bool result;
+  result = ChromeThread::PostDelayedTask(
+    ChromeThread::FILE, FROM_HERE, NewRunnableMethod(
+      this, &chromeos::ExternalMetrics::CollectEventsAndReschedule),
+    kExternalMetricsCollectionIntervalMs);
+  DCHECK(result);
+}
+
+}  // namespace chromeos
diff --git a/chrome/browser/chromeos/external_metrics.h b/chrome/browser/chromeos/external_metrics.h
new file mode 100644
index 0000000..2721939
--- /dev/null
+++ b/chrome/browser/chromeos/external_metrics.h
@@ -0,0 +1,98 @@
+// Copyright (c) 2009 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 CHROME_BROWSER_CHROMEOS_EXTERNAL_METRICS_H_
+#define CHROME_BROWSER_CHROMEOS_EXTERNAL_METRICS_H_
+
+#include "base/basictypes.h"
+#include "base/compiler_specific.h"
+#include "base/task.h"
+#include "testing/gtest/include/gtest/gtest_prod.h"  // For FRIEND_TEST
+
+class Profile;
+
+namespace chromeos {
+
+// ExternalMetrics is a service that Chrome offers to Chrome OS to upload
+// metrics to the UMA server on its behalf.  Chrome periodically reads the
+// content of a well-know file, and parses it into name-value pairs, each
+// representing a Chrome OS metrics event. The events are logged using the
+// normal UMA mechanism. The file is then truncated to zero size. Chrome uses
+// flock() to synchronize accesses to the file.
+class ExternalMetrics : public base::RefCountedThreadSafe<ExternalMetrics> {
+  FRIEND_TEST(ExternalMetricsTest, ParseExternalMetricsFile);
+  friend class base::RefCountedThreadSafe<ExternalMetrics>;
+
+ public:
+  ExternalMetrics() {}
+
+  // Begins the external data collection.  Profile is passed through to
+  // UserMetrics::RecordAction.  The lifetime of profile must exceed that of
+  // the external metrics object.
+  void Start(Profile* profile);
+
+ private:
+  // There is one function with this type for each action or histogram.
+  typedef void (*RecordFunctionType)(const char*);
+  // The type of event associated with each name.
+  typedef enum {
+    EVENT_TYPE_ACTION,
+    EVENT_TYPE_HISTOGRAM
+  } MetricsEventType;
+  // Used in mapping names (C strings) into event-recording functions.
+  typedef struct {
+    const char* name;
+    RecordFunctionType function;
+    MetricsEventType type;
+  } RecordFunctionTableEntry;
+  typedef void (*RecorderType)(const char*, const char*);  // See SetRecorder.
+
+  // The max length of a message (name-value pair, plus header)
+  static const int kMetricsMessageMaxLength = 4096;
+
+  ~ExternalMetrics();
+
+  // Protect action recorders from being called when external_metrics_profile is
+  // null.  This could happen when testing, or in the unlikely case that the
+  // order of object deletion at shutdown changes.
+  static void RecordActionWrapper(RecordFunctionType);
+
+  // Maps a name to an entry in the record function table.  Return NULL on
+  // failure.
+  static const ExternalMetrics::RecordFunctionTableEntry* FindRecordEntry(
+      const char* name);
+
+  // Initializes a table that maps a metric name to a function that logs that
+  // metric.
+  void InitializeFunctionTable();
+
+  // Passes an event, either an ACTION or HISTOGRAM depending on |name|, to the
+  // UMA service.  For a histogram, |value| contains the numeric value, in a
+  // format that depends on |name|.
+  static void RecordEvent(const char* name, const char* value);
+
+  // Collects external events from metrics log file.  This is run at periodic
+  // intervals.
+  void CollectEvents();
+
+  // Calls CollectEvents and reschedules a future collection.
+  void CollectEventsAndReschedule();
+
+  // Schedules a metrics event collection in the future.
+  void ScheduleCollector();
+
+  // Sets the event logging function.  Exists only because of testing.
+  void SetRecorder(RecorderType recorder) {
+    recorder_ = recorder;
+  }
+
+  // This table maps event names to event recording functions.
+  static RecordFunctionTableEntry function_table_[];
+  RecorderType recorder_;  // See SetRecorder.
+  DISALLOW_COPY_AND_ASSIGN(ExternalMetrics);
+};
+
+}  // namespace chromeos
+
+#endif  // CHROME_BROWSER_CHROMEOS_EXTERNAL_METRICS_H_
diff --git a/chrome/browser/chromeos/external_metrics_unittest.cc b/chrome/browser/chromeos/external_metrics_unittest.cc
new file mode 100644
index 0000000..b484736
--- /dev/null
+++ b/chrome/browser/chromeos/external_metrics_unittest.cc
@@ -0,0 +1,111 @@
+// Copyright (c) 2009 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 <errno.h>
+#include <sys/file.h>
+
+#include "base/basictypes.h"
+#include "chrome/browser/chromeos/external_metrics.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace chromeos {  // Need this because of the FRIEND_TEST
+
+class ExternalMetricsTest : public testing::Test {
+};
+
+// Because the metrics service is not essential, errors will not cause the
+// program to terminate.  However, the errors produce logs.
+
+#define MAXLENGTH ExternalMetrics::kMetricsMessageMaxLength
+
+static void SendMessage(const char* path, const char* name, const char* value) {
+  int fd = open(path, O_CREAT | O_APPEND | O_WRONLY, 0666);
+  int32 l = strlen(name) + strlen(value) + 2 + sizeof(l);
+  write(fd, &l, sizeof(l));
+  write(fd, name, strlen(name) + 1);
+  write(fd, value, strlen(value) + 1);
+  close(fd);
+}
+
+const char* received_name = NULL;
+const char* received_value = NULL;
+int received_count = 0;
+
+static void ReceiveMessage(const char* name, const char* value) {
+  received_name = name;
+  received_value = value;
+  received_count++;
+}
+
+static void CheckMessage(const char* name, const char* value, int count) {
+  EXPECT_EQ(0, strcmp(received_name, name));
+  EXPECT_EQ(0, strcmp(received_value, value));
+  EXPECT_EQ(received_count, count);
+}
+
+TEST(ExternalMetricsTest, ParseExternalMetricsFile) {
+  struct {
+    const char* name;
+    const char* value;
+  } pairs[] = {
+    {"BootTime", "9500"},
+    {"BootTime", "10000"},
+    {"BootTime", "9200"},
+  };
+  int npairs = ARRAYSIZE_UNSAFE(pairs);
+  int32 i;
+  const char* path = "/tmp/.chromeos-metrics";
+
+  chromeos::ExternalMetrics* external_metrics = new chromeos::ExternalMetrics();
+  external_metrics->SetRecorder(&ReceiveMessage);
+
+  EXPECT_TRUE(unlink(path) == 0 || errno == ENOENT);
+
+  // Send a few valid messages.
+  for (i = 0; i < npairs; i++) {
+    SendMessage(path, pairs[i].name, pairs[i].value);
+  }
+
+  external_metrics->CollectEvents();
+  CheckMessage(pairs[npairs-1].name, pairs[npairs-1].value, npairs);
+
+  // Send a message that's too large.
+  char b[MAXLENGTH + 100];
+  for (i = 0; i < MAXLENGTH + 99; i++) {
+    b[i] = 'x';
+  }
+  b[i] = '\0';
+  SendMessage(path, b, "yyy");
+  external_metrics->CollectEvents();
+  EXPECT_EQ(received_count, npairs);
+
+  // Send a malformed message (first string is not null-terminated).
+  i = 100 + sizeof(i);
+  int fd = open(path, O_CREAT | O_WRONLY);
+  EXPECT_GT(fd, 0);
+  EXPECT_EQ(write(fd, &i, sizeof(i)), static_cast<int>(sizeof(i)));
+  EXPECT_EQ(write(fd, b, i), i);
+  EXPECT_EQ(close(fd), 0);
+
+  external_metrics->CollectEvents();
+  EXPECT_EQ(received_count, npairs);
+
+  // Send a malformed message (second string is not null-terminated).
+  b[50] = '\0';
+  fd = open(path, O_CREAT | O_WRONLY);
+  EXPECT_GT(fd, 0);
+  EXPECT_EQ(write(fd, &i, sizeof(i)), static_cast<int>(sizeof(i)));
+  EXPECT_EQ(write(fd, b, i), i);
+  EXPECT_EQ(close(fd), 0);
+
+  external_metrics->CollectEvents();
+  EXPECT_EQ(received_count, npairs);
+
+  // Check that we survive when file doesn't exist.
+  EXPECT_EQ(unlink(path), 0);
+  external_metrics->CollectEvents();
+  EXPECT_EQ(received_count, npairs);
+}
+
+}  // namespace chromeos
diff --git a/chrome/browser/metrics/metrics_service.cc b/chrome/browser/metrics/metrics_service.cc
index a16ced2..c49886a 100644
--- a/chrome/browser/metrics/metrics_service.cc
+++ b/chrome/browser/metrics/metrics_service.cc
@@ -197,6 +197,10 @@
 #include "chrome/installer/util/browser_distribution.h"
 #endif
 
+#if defined(OS_CHROMEOS)
+#include "chrome/browser/chromeos/external_metrics.h"
+#endif
+
 using base::Time;
 using base::TimeDelta;
 
@@ -1921,3 +1925,10 @@ static bool IsSingleThreaded() {
     thread_id = PlatformThread::CurrentId();
   return PlatformThread::CurrentId() == thread_id;
 }
+
+#if defined(OS_CHROMEOS)
+void MetricsService::StartExternalMetrics(Profile* profile) {
+  external_metrics_ = new chromeos::ExternalMetrics;
+  external_metrics_->Start(profile);
+}
+#endif
diff --git a/chrome/browser/metrics/metrics_service.h b/chrome/browser/metrics/metrics_service.h
index 03b181f..9fa21fc 100644
--- a/chrome/browser/metrics/metrics_service.h
+++ b/chrome/browser/metrics/metrics_service.h
@@ -25,6 +25,10 @@
 #include "webkit/glue/webplugininfo.h"
 #include "testing/gtest/include/gtest/gtest_prod.h"
 
+#if defined(OS_CHROMEOS)
+#include "chrome/browser/chromeos/external_metrics.h"
+#endif
+
 class BookmarkModel;
 class BookmarkNode;
 class HistogramSynchronizer;
@@ -120,6 +124,12 @@ class MetricsService : public NotificationObserver,
   // at shutdown, but we can do it as we reduce the list as well.
   void StoreUnsentLogs();
 
+#if defined(OS_CHROMEOS)
+  // Start the external metrics service, which collects metrics from Chrome OS
+  // and passes them to UMA.
+  void StartExternalMetrics(Profile* profile);
+#endif
+
  private:
   // The MetricsService has a lifecycle that is stored as a state.
   // See metrics_service.cc for description of this lifecycle.
@@ -496,6 +506,11 @@ class MetricsService : public NotificationObserver,
   // Indicate that a timer for sending the next log has already been queued.
   bool timer_pending_;
 
+#if defined(OS_CHROMEOS)
+  // The external metric service is used to log ChromeOS UMA events.
+  scoped_refptr<chromeos::ExternalMetrics> external_metrics_;
+#endif
+
   FRIEND_TEST(MetricsServiceTest, ClientIdGeneratesAllZeroes);
   FRIEND_TEST(MetricsServiceTest, ClientIdGeneratesCorrectly);
   FRIEND_TEST(MetricsServiceTest, ClientIdCorrectlyFormatted);
diff --git a/chrome/browser/metrics/system_metrics_logger.h b/chrome/browser/metrics/system_metrics_logger.h
deleted file mode 100644
index 83f94f6..0000000
--- a/chrome/browser/metrics/system_metrics_logger.h
+++ /dev/null
@@ -1,28 +0,0 @@
-// Copyright (c) 2009 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 CHROME_BROWSER_METRICS_SYSTEM_METRICS_LOGGER_H_
-#define CHROME_BROWSER_METRICS_SYSTEM_METRICS_LOGGER_H_
-
-#include "base/basictypes.h"
-
-class Profile;
-
-// This is the abstract base class for a simple class that wraps up some
-// calls to chromium metrics logging helper functions.  This design will
-// allow for easy mocking in unit tests.
-
-class SystemMetricsLogger {
- public:
-  SystemMetricsLogger() {}
-  virtual ~SystemMetricsLogger() {}
-  virtual void RecordOverviewKeystroke(Profile *profile) = 0;
-  virtual void RecordOverviewExitMouse(Profile *profile) = 0;
-  virtual void RecordOverviewExitKeystroke(Profile *profile) = 0;
-  virtual void RecordWindowCycleKeystroke(Profile *profile) = 0;
-  virtual void RecordBootTime(int64 time) = 0;
-  virtual void RecordUpTime(int64 time) = 0;
-};
-
-#endif  // CHROME_BROWSER_METRICS_SYSTEM_METRICS_LOGGER_H_
diff --git a/chrome/browser/metrics/system_metrics_logger_impl.cc b/chrome/browser/metrics/system_metrics_logger_impl.cc
deleted file mode 100644
index b16fe5d..0000000
--- a/chrome/browser/metrics/system_metrics_logger_impl.cc
+++ /dev/null
@@ -1,44 +0,0 @@
-// Copyright (c) 2009 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/metrics/system_metrics_logger_impl.h"
-
-#include "base/basictypes.h"
-#include "base/histogram.h"
-#include "base/time.h"
-#include "chrome/browser/profile.h"
-#include "chrome/browser/metrics/user_metrics.h"
-
-SystemMetricsLoggerImpl::SystemMetricsLoggerImpl() {}
-
-SystemMetricsLoggerImpl::~SystemMetricsLoggerImpl() {}
-
-void SystemMetricsLoggerImpl::RecordOverviewKeystroke(Profile *profile) {
-  UserMetrics::RecordAction(L"TabOverview_Keystroke", profile);
-}
-
-void SystemMetricsLoggerImpl::RecordOverviewExitMouse(Profile *profile) {
-  UserMetrics::RecordAction(L"TabOverview_ExitMouse", profile);
-}
-
-void SystemMetricsLoggerImpl::RecordOverviewExitKeystroke(Profile *profile) {
-  UserMetrics::RecordAction(L"TabOverview_ExitKeystroke", profile);
-}
-
-void SystemMetricsLoggerImpl::RecordWindowCycleKeystroke(Profile *profile) {
-  UserMetrics::RecordAction(L"TabOverview_WindowCycleKeystroke", profile);
-}
-
-void SystemMetricsLoggerImpl::RecordBootTime(int64 time) {
-  UMA_HISTOGRAM_CUSTOM_TIMES("ChromeOS.Boot Time",
-                             base::TimeDelta::FromMilliseconds(time),
-                             base::TimeDelta::FromMilliseconds(1),
-                             base::TimeDelta::FromMinutes(1),
-                             50);
-}
-
-void SystemMetricsLoggerImpl::RecordUpTime(int64 time) {
-  UMA_HISTOGRAM_LONG_TIMES("ChromeOS.Uptime",
-                           base::TimeDelta::FromSeconds(time));
-}
diff --git a/chrome/browser/metrics/system_metrics_logger_impl.h b/chrome/browser/metrics/system_metrics_logger_impl.h
deleted file mode 100644
index 8bcfc5b..0000000
--- a/chrome/browser/metrics/system_metrics_logger_impl.h
+++ /dev/null
@@ -1,31 +0,0 @@
-// Copyright (c) 2009 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 CHROME_BROWSER_METRICS_SYSTEM_METRICS_LOGGER_IMPL_H_
-#define CHROME_BROWSER_METRICS_SYSTEM_METRICS_LOGGER_IMPL_H_
-
-#include "base/basictypes.h"
-#include "chrome/browser/metrics/system_metrics_logger.h"
-
-class Profile;
-
-// Wraps calls to UserMetrics::RecordAction() and the appropriate
-// version of the UMA_HISTOGRAM_*_TIMES macros, based on the metric
-// being logged
-
-class SystemMetricsLoggerImpl : public SystemMetricsLogger {
- public:
-  SystemMetricsLoggerImpl();
-  ~SystemMetricsLoggerImpl();
-  void RecordOverviewKeystroke(Profile *profile);
-  void RecordOverviewExitMouse(Profile *profile);
-  void RecordOverviewExitKeystroke(Profile *profile);
-  void RecordWindowCycleKeystroke(Profile *profile);
-  void RecordBootTime(int64 time);
-  void RecordUpTime(int64 time);
- private:
-  DISALLOW_COPY_AND_ASSIGN(SystemMetricsLoggerImpl);
-};
-
-#endif  // CHROME_BROWSER_METRICS_SYSTEM_METRICS_LOGGER_IMPL_H_
diff --git a/chrome/browser/views/tabs/tab_overview_message_listener.cc b/chrome/browser/views/tabs/tab_overview_message_listener.cc
index 68e05a8..be91a6d 100644
--- a/chrome/browser/views/tabs/tab_overview_message_listener.cc
+++ b/chrome/browser/views/tabs/tab_overview_message_listener.cc
@@ -11,8 +11,6 @@
 #else
 #include "chrome/browser/gtk/browser_window_gtk.h"
 #endif
-#include "chrome/browser/metrics/system_metrics_logger_impl.h"
-#include "chrome/browser/metrics/system_metrics.pb.h"
 #include "chrome/browser/metrics/user_metrics.h"
 #include "chrome/browser/views/new_browser_window_widget.h"
 #include "chrome/browser/views/tabs/tab_overview_controller.h"
@@ -47,34 +45,12 @@ BrowserView* TabOverviewMessageListener::GetBrowserViewForGdkWindow(
 void TabOverviewMessageListener::WillProcessEvent(GdkEvent* event) {
 }
 
-namespace {
-void ProcessSystemMetricsString(const std::string& message) {
-  SystemMetricsLoggerImpl logger;
-  chrome_os_pb::SystemMetrics system_metrics;
-  if (!system_metrics.ParseFromString(message)) {
-    DLOG(ERROR) << "Could not parse system metrics protobuffer!";
-    return;
-  }
-  // For now, boot time is the only metric we'll worry about.
-  if (system_metrics.has_boot_time_ms()) {
-    logger.RecordBootTime(system_metrics.boot_time_ms());
-  }
-}
-}  // namespace
-
 void TabOverviewMessageListener::DidProcessEvent(GdkEvent* event) {
   if (event->type == GDK_CLIENT_EVENT) {
     TabOverviewTypes::Message message;
     GdkEventClient* client_event = reinterpret_cast<GdkEventClient*>(event);
     if (TabOverviewTypes::instance()->DecodeMessage(*client_event, &message))
       ProcessMessage(message, client_event->window);
-  } else if (event->type == GDK_PROPERTY_NOTIFY) {
-    std::string message;
-    GdkEventProperty* client_event = reinterpret_cast<GdkEventProperty*>(event);
-    if (TabOverviewTypes::instance()->DecodeStringMessage(*client_event,
-                                                          &message)) {
-      ProcessSystemMetricsString(message);
-    }
   }
 }
 
diff --git a/chrome/chrome.gyp b/chrome/chrome.gyp
index a378e17..fa6d8a7 100755
--- a/chrome/chrome.gyp
+++ b/chrome/chrome.gyp
@@ -991,6 +991,8 @@
         'browser/chromeos/cros_library.h',
         'browser/chromeos/external_cookie_handler.cc',
         'browser/chromeos/external_cookie_handler.h',
+        'browser/chromeos/external_metrics.cc',
+        'browser/chromeos/external_metrics.h',
         'browser/chromeos/external_protocol_dialog.cc',
         'browser/chromeos/external_protocol_dialog.h',
         'browser/chromeos/gview_request_interceptor.cc',
@@ -3028,13 +3030,6 @@
                   ],
                 },
               ],
-              'sources': [
-                '<(INTERMEDIATE_DIR)/chrome/browser/metrics/system_metrics.pb.h',
-                '<(INTERMEDIATE_DIR)/chrome/browser/metrics/system_metrics.pb.cc',
-                'browser/metrics/system_metrics_logger.h',
-                'browser/metrics/system_metrics_logger_impl.cc',
-                'browser/metrics/system_metrics_logger_impl.h',
-              ],
               'include_dirs': [
                 '<(INTERMEDIATE_DIR)',
                 '<(INTERMEDIATE_DIR)/chrome',
@@ -4536,6 +4531,7 @@
         'browser/child_process_security_policy_unittest.cc',
         'browser/chrome_thread_unittest.cc',
         'browser/chromeos/external_cookie_handler_unittest.cc',
+        'browser/chromeos/external_metrics_unittest.cc',
         'browser/chromeos/gview_request_interceptor_unittest.cc',
         'browser/chromeos/pipe_reader_unittest.cc',
         'browser/chromeos/version_loader_unittest.cc',