[sync] [chromeos] Get rid of dependencies on chrome/browser from within sync/

As of today, syncer::GetSessionNameSynchronously() utilizes code in
chromeos::system::StatisticsProvider, which lives in
chrome/browser/chromeos. This results in an ugly mutual dependency
between sync.gyp:sync and chrome.gyp:browser. This hasn't been a huge
problem so far because sync is built into browser. It will, however, be
a problem once sync is pulled into its own component.

This patch does the following:
- Removes the dependency from sync/ onto chrome/browser.
- Reimplements GetSessionNameSynchronously to use command line primitives.
- Adds new CrOs-only unit tests to exercise this code.

BUG=164726,126732,136928
TEST=GetSessionNameTest.*; Open a new tab on a Chromebook and Chromebox and make sure the title of the "Other devices" section looks right.

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

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

3 files changed, 64 insertions, 9 deletions

diff --git a/sync/util/DEPS b/sync/util/DEPS
index 23fe0bd..3bef302 100644
--- a/sync/util/DEPS
+++ b/sync/util/DEPS
@@ -5,9 +5,8 @@ include_rules = [
   "+sync/protocol",
   "+sync/test/fake_encryptor.h",
 
-  # TODO(kochi): Remove this hack after "Chromebox" hack in get_session_name.cc
-  # is gone.
-  "+chrome/browser/chromeos/system",
+  # TODO(rsimha): Remove this after http://crbug.com/126732 is fixed.
+  "+chromeos/chromeos_switches.h",
 
   # TODO(zea): remove this once we don't need the cryptographer to get the set
   # of encrypted types.
diff --git a/sync/util/get_session_name.cc b/sync/util/get_session_name.cc
index 1896ebc..d5cf629 100644
--- a/sync/util/get_session_name.cc
+++ b/sync/util/get_session_name.cc
@@ -12,7 +12,8 @@
 #include "base/task_runner.h"
 
 #if defined(OS_CHROMEOS)
-#include "chrome/browser/chromeos/system/statistics_provider.h"
+#include "base/command_line.h"
+#include "chromeos/chromeos_switches.h"
 #elif defined(OS_LINUX)
 #include "base/linux_util.h"
 #elif defined(OS_IOS)
@@ -32,13 +33,27 @@ namespace {
 std::string GetSessionNameSynchronously() {
   std::string session_name;
 #if defined(OS_CHROMEOS)
-  // TODO(kochi): This is very ad hoc and fragile. http://crbug.com/126732.
+  // The approach below is similar to that used by the CrOs implementation of
+  // StatisticsProvider::GetMachineStatistic(CHROMEOS_RELEASE_BOARD).
+  // See chrome/browser/chromeos/system/statistics_provider.{h|cc}.
+  //
+  // We cannot use StatisticsProvider here because of the mutual dependency
+  // it creates between sync.gyp:sync and chrome.gyp:browser.
+  //
+  // Even though this code is ad hoc and fragile, it remains the only means of
+  // determining the Chrome OS hardware platform so we can display the right
+  // device name in the "Other devices" section of the new tab page.
+  // TODO(rsimha): Change this once a better alternative is available.
+  // See http://crbug.com/126732.
   std::string board;
-  const char kMachineInfoBoard[] = "CHROMEOS_RELEASE_BOARD";
-  chromeos::system::StatisticsProvider* provider =
-      chromeos::system::StatisticsProvider::GetInstance();
-  if (!provider->GetMachineStatistic(kMachineInfoBoard, &board))
+  const CommandLine* command_line = CommandLine::ForCurrentProcess();
+  if (command_line->HasSwitch(chromeos::switches::kChromeOSReleaseBoard)) {
+    board = command_line->
+        GetSwitchValueASCII(chromeos::switches::kChromeOSReleaseBoard);
+  } else {
     LOG(ERROR) << "Failed to get board information";
+  }
+
   // Currently, only "stumpy" type of board is considered Chromebox, and
   // anything else is Chromebook.  On these devices, session_name should look
   // like "stumpy-signed-mp-v2keys" etc. The information can be checked on
diff --git a/sync/util/get_session_name_unittest.cc b/sync/util/get_session_name_unittest.cc
index b4922f6..0d0ffe4 100644
--- a/sync/util/get_session_name_unittest.cc
+++ b/sync/util/get_session_name_unittest.cc
@@ -9,6 +9,11 @@
 #include "sync/util/get_session_name.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+#if defined(OS_CHROMEOS)
+#include "base/command_line.h"
+#include "chromeos/chromeos_switches.h"
+#endif  // OS_CHROMEOS
+
 namespace syncer {
 
 namespace {
@@ -32,6 +37,42 @@ TEST_F(GetSessionNameTest, GetSessionNameSynchronously) {
   EXPECT_FALSE(session_name.empty());
 }
 
+#if defined(OS_CHROMEOS)
+
+// Call GetSessionNameSynchronouslyForTesting on ChromeOS where the board type
+// is "lumpy-signed-mp-v2keys" and make sure the return value is "Chromebook".
+TEST_F(GetSessionNameTest, GetSessionNameSynchronouslyChromebook) {
+  // This test cannot be run on a real CrOs device, since it will already have a
+  // board type, and we cannot override it.
+  // TODO(rsimha): Rewrite this test once http://crbug.com/126732 is fixed.
+  CommandLine* command_line = CommandLine::ForCurrentProcess();
+  if (command_line->HasSwitch(chromeos::switches::kChromeOSReleaseBoard))
+    return;
+
+  command_line->AppendSwitchASCII(chromeos::switches::kChromeOSReleaseBoard,
+                                  "lumpy-signed-mp-v2keys");
+  const std::string& session_name = GetSessionNameSynchronouslyForTesting();
+  EXPECT_EQ("Chromebook", session_name);
+}
+
+// Call GetSessionNameSynchronouslyForTesting on ChromeOS where the board type
+// is "stumpy-signed-mp-v2keys" and make sure the return value is "Chromebox".
+TEST_F(GetSessionNameTest, GetSessionNameSynchronouslyChromebox) {
+  // This test cannot be run on a real CrOs device, since it will already have a
+  // board type, and we cannot override it.
+  // TODO(rsimha): Rewrite this test once http://crbug.com/126732 is fixed.
+  CommandLine* command_line = CommandLine::ForCurrentProcess();
+  if (command_line->HasSwitch(chromeos::switches::kChromeOSReleaseBoard))
+    return;
+
+  command_line->AppendSwitchASCII(chromeos::switches::kChromeOSReleaseBoard,
+                                  "stumpy-signed-mp-v2keys");
+  const std::string& session_name = GetSessionNameSynchronouslyForTesting();
+  EXPECT_EQ("Chromebox", session_name);
+}
+
+#endif  // OS_CHROMEOS
+
 // Calls GetSessionName and runs the message loop until it comes back
 // with a session name.  Makes sure the returned session name is equal
 // to the return value of GetSessionNameSynchronouslyForTesting().