Add dev switch boot mode to device policy status reports.

Also modified NameValuePairsParser to support reading the output from /usr/bin/crossystem.

BUG=chromium-os:22035
TEST=New unit test added to DeviceStatusCollectorTest & new unit_test added to
NamesValuePairsParser unit test.


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

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

8 files changed, 206 insertions, 50 deletions

diff --git a/chrome/browser/chromeos/system/name_value_pairs_parser.cc b/chrome/browser/chromeos/system/name_value_pairs_parser.cc
index 51ee327..45ea536 100644
--- a/chrome/browser/chromeos/system/name_value_pairs_parser.cc
+++ b/chrome/browser/chromeos/system/name_value_pairs_parser.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 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.
 
@@ -11,7 +11,6 @@
 #include "base/process_util.h"
 #include "base/string_tokenizer.h"
 #include "base/string_util.h"
-#include "base/threading/thread_restrictions.h"
 
 namespace chromeos {  // NOLINT
 namespace system {
@@ -19,6 +18,24 @@ namespace system {
 namespace {
 
 const char kQuoteChars[] = "\"";
+const char kTrimChars[] = "\" ";
+
+bool GetToolOutput(int argc, const char* argv[], std::string& output) {
+  DCHECK_GE(argc, 1);
+
+  if (!file_util::PathExists(FilePath(argv[0]))) {
+    LOG(WARNING) << "Tool for statistics not found: " << argv[0];
+    return false;
+  }
+
+  CommandLine command_line(argc, argv);
+  if (!base::GetAppOutput(command_line, &output)) {
+    LOG(WARNING) << "Error executing: " << command_line.GetCommandLineString();
+    return false;
+  }
+
+  return true;
+}
 
 }  // namespace
 
@@ -35,6 +52,14 @@ void NameValuePairsParser::AddNameValuePair(const std::string& key,
 bool NameValuePairsParser::ParseNameValuePairs(const std::string& in_string,
                                                const std::string& eq,
                                                const std::string& delim) {
+  return ParseNameValuePairsWithComments(in_string, eq, delim, "");
+}
+
+bool NameValuePairsParser::ParseNameValuePairsWithComments(
+    const std::string& in_string,
+    const std::string& eq,
+    const std::string& delim,
+    const std::string& comment_delim) {
   // Set up the pair tokenizer.
   StringTokenizer pair_toks(in_string, delim);
   pair_toks.set_quote_chars(kQuoteChars);
@@ -46,15 +71,25 @@ bool NameValuePairsParser::ParseNameValuePairs(const std::string& in_string,
       return false;
     }
     StringTokenizer keyvalue(pair, eq);
-    std::string key,value;
+    std::string key;
+    std::string value;
     if (keyvalue.GetNext()) {
-      TrimString(keyvalue.token(), kQuoteChars, &key);
+      TrimString(keyvalue.token(), kTrimChars, &key);
       if (keyvalue.GetNext()) {
-        TrimString(keyvalue.token(), kQuoteChars, &value);
+        value = keyvalue.token();
         if (keyvalue.GetNext()) {
           LOG(WARNING) << "Multiple key tokens: '" << pair << "'. Aborting.";
           return false;
         }
+        // If value ends with a comment, throw away everything after
+        // comment_delim is encountered.
+        if (!comment_delim.empty()) {
+          StringTokenizer value_with_comment(value, comment_delim);
+          value_with_comment.GetNext();
+          value = value_with_comment.token();
+        }
+
+        TrimString(value, kTrimChars, &value);
       }
     }
     if (key.empty()) {
@@ -69,19 +104,10 @@ bool NameValuePairsParser::ParseNameValuePairs(const std::string& in_string,
 bool NameValuePairsParser::GetSingleValueFromTool(int argc,
                                                   const char* argv[],
                                                   const std::string& key) {
-  DCHECK_GE(argc, 1);
-
-  if (!file_util::PathExists(FilePath(argv[0]))) {
-    LOG(WARNING) << "Tool for statistics not found: " << argv[0];
-    return false;
-  }
-
-  CommandLine command_line(argc, argv);
   std::string output_string;
-  if (!base::GetAppOutput(command_line, &output_string)) {
-    LOG(WARNING) << "Error executing: " << command_line.GetCommandLineString();
+  if (!GetToolOutput(argc, argv, output_string))
     return false;
-  }
+
   TrimWhitespaceASCII(output_string, TRIM_ALL, &output_string);
   AddNameValuePair(key, output_string);
   return true;
@@ -98,5 +124,19 @@ void NameValuePairsParser::GetNameValuePairsFromFile(const FilePath& file_path,
   }
 }
 
+bool NameValuePairsParser::ParseNameValuePairsFromTool(
+    int argc,
+    const char* argv[],
+    const std::string& eq,
+    const std::string& delim,
+    const std::string& comment_delim) {
+  std::string output_string;
+  if (!GetToolOutput(argc, argv, output_string))
+    return false;
+
+  return ParseNameValuePairsWithComments(
+      output_string, eq, delim, comment_delim);
+}
+
 }  // namespace system
 }  // namespace chromeos
diff --git a/chrome/browser/chromeos/system/name_value_pairs_parser.h b/chrome/browser/chromeos/system/name_value_pairs_parser.h
index faf81b8..bcf4f43 100644
--- a/chrome/browser/chromeos/system/name_value_pairs_parser.h
+++ b/chrome/browser/chromeos/system/name_value_pairs_parser.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 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.
 
@@ -8,7 +8,6 @@
 
 #include <map>
 #include <string>
-#include <vector>
 
 #include "base/basictypes.h"
 
@@ -40,13 +39,29 @@ class NameValuePairsParser {
                                  const std::string& eq,
                                  const std::string& delim);
 
-  // This will parse strings with output in the format:
+  // These will parse strings with output in the format:
   // <key><EQ><value><DELIM>[<key><EQ><value>][...]
   // e.g. ParseNameValuePairs("key1=value1 key2=value2", "=", " ")
   bool ParseNameValuePairs(const std::string& in_string,
                            const std::string& eq,
                            const std::string& delim);
 
+  // This version allows for values which end with a comment
+  // beginning with comment_delim.
+  // e.g."key2=value2 # Explanation of value\n"
+  bool ParseNameValuePairsWithComments(const std::string& in_string,
+                                       const std::string& eq,
+                                       const std::string& delim,
+                                       const std::string& comment_delim);
+
+  bool ParseNameValuePairsFromTool(
+      int argc,
+      const char* argv[],
+      const std::string& eq,
+      const std::string& delim,
+      const std::string& comment_delim);
+
+ private:
   NameValueMap* map_;
 
   DISALLOW_COPY_AND_ASSIGN(NameValuePairsParser);
diff --git a/chrome/browser/chromeos/system/name_value_pairs_parser_unittest.cc b/chrome/browser/chromeos/system/name_value_pairs_parser_unittest.cc
index d229126..ddbd908 100644
--- a/chrome/browser/chromeos/system/name_value_pairs_parser_unittest.cc
+++ b/chrome/browser/chromeos/system/name_value_pairs_parser_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 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.
 
@@ -57,5 +57,27 @@ TEST(NameValuePairsParser, TestParseNameValuePairs) {
   EXPECT_EQ("mozc-jp", map["keyboard_layout"]);
 }
 
+TEST(NameValuePairsParser, TestParseNameValuePairsFromTool) {
+  // Sample output is taken from the /usr/bin/crosssytem tool.
+  const char* command[] = { "/bin/echo",
+    "arch                   = x86           # Platform architecture\n" \
+    "cros_debug             = 1             # OS should allow debug\n" \
+    "dbg_reset              = (error)       # Debug reset mode request\n" \
+    "key#with_comment       = some value    # Multiple # comment # delims\n" \
+    "key                    =               # No value."
+  };
+
+  NameValuePairsParser::NameValueMap map;
+  NameValuePairsParser parser(&map);
+  parser.ParseNameValuePairsFromTool(
+      arraysize(command), command, "=", "\n", "#");
+  EXPECT_EQ("x86", map["arch"]);
+  EXPECT_EQ("1", map["cros_debug"]);
+  EXPECT_EQ("(error)", map["dbg_reset"]);
+  EXPECT_EQ("some value", map["key#with_comment"]);
+  EXPECT_EQ("", map["key"]);
+  EXPECT_EQ(5u, map.size());
+}
+
 }  // namespace system
 }  // namespace chromeos
diff --git a/chrome/browser/chromeos/system/statistics_provider.cc b/chrome/browser/chromeos/system/statistics_provider.cc
index 32ef5ca..3b5a950 100644
--- a/chrome/browser/chromeos/system/statistics_provider.cc
+++ b/chrome/browser/chromeos/system/statistics_provider.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 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.
 
@@ -22,26 +22,31 @@ namespace chromeos {
 namespace system {
 namespace {
 
-// The tools used here should be specified as absolute paths. The
-// existence of the tools is checked in GetSingleValueFromTool().
+// Path to the tool used to get system info, and delimiters for the output
+// format of the tool.
+const char* kCrosSystemTool[] = { "/usr/bin/crossystem" };
+const char kCrosSystemEq[] = "=";
+const char kCrosSystemDelim[] = "\n";
+const char kCrosSystemCommentDelim[] = "#";
+const char kCrosSystemUnknownValue[] = "(error)";
 
-// The system command that returns the hardware class.
+const char kHardwareClassCrosSystemKey[] = "hwid";
 const char kHardwareClassKey[] = "hardware_class";
-const char* kHardwareClassTool[] = { "/usr/bin/crossystem", "hwid" };
 const char kUnknownHardwareClass[] = "unknown";
 
-// Command to get machine hardware info and key/value delimiters.
+// File to get machine hardware info from, and key/value delimiters of
+// the file.
 // /tmp/machine-info is generated by platform/init/chromeos_startup.
 const char kMachineHardwareInfoFile[] = "/tmp/machine-info";
 const char kMachineHardwareInfoEq[] = "=";
 const char kMachineHardwareInfoDelim[] = " \n";
 
-// Command to get machine OS info and key/value delimiters.
+// File to get machine OS info from, and key/value delimiters of the file.
 const char kMachineOSInfoFile[] = "/etc/lsb-release";
 const char kMachineOSInfoEq[] = "=";
 const char kMachineOSInfoDelim[] = "\n";
 
-// Command to get VPD info and key/value delimiters.
+// File to get VPD info from, and key/value delimiters of the file.
 const char kVpdFile[] = "/var/log/vpd_2.0.txt";
 const char kVpdEq[] = "=";
 const char kVpdDelim[] = "\n";
@@ -56,7 +61,7 @@ class StatisticsProviderImpl : public StatisticsProvider {
  public:
   // StatisticsProvider implementation:
   virtual bool GetMachineStatistic(const std::string& name,
-                                   std::string* result);
+                                   std::string* result) OVERRIDE;
 
   static StatisticsProviderImpl* GetInstance();
 
@@ -124,12 +129,20 @@ void StatisticsProviderImpl::StartLoadingMachineStatistics() {
 
 void StatisticsProviderImpl::LoadMachineStatistics() {
   NameValuePairsParser parser(&machine_info_);
-  if (!parser.GetSingleValueFromTool(arraysize(kHardwareClassTool),
-                                     kHardwareClassTool,
-                                     kHardwareClassKey)) {
-    // Use kUnknownHardwareClass if the hardware class command fails.
-    parser.AddNameValuePair(kHardwareClassKey, kUnknownHardwareClass);
-  }
+
+  // Parse all of the key/value pairs from the crossystem tool.
+  parser.ParseNameValuePairsFromTool(
+      arraysize(kCrosSystemTool), kCrosSystemTool, kCrosSystemEq,
+      kCrosSystemDelim, kCrosSystemCommentDelim);
+
+  // Ensure that the hardware class key is present with the expected
+  // key name, and if it couldn't be retrieved, that the value is "unknown".
+  std::string hardware_class = machine_info_[kHardwareClassCrosSystemKey];
+  if (hardware_class.empty() || hardware_class == kCrosSystemUnknownValue)
+    machine_info_[kHardwareClassKey] = kUnknownHardwareClass;
+  else
+    machine_info_[kHardwareClassKey] = hardware_class;
+
   parser.GetNameValuePairsFromFile(FilePath(kMachineHardwareInfoFile),
                                    kMachineHardwareInfoEq,
                                    kMachineHardwareInfoDelim);
@@ -167,7 +180,7 @@ class StatisticsProviderStubImpl : public StatisticsProvider {
  public:
   // StatisticsProvider implementation:
   virtual bool GetMachineStatistic(const std::string& name,
-                                   std::string* result) {
+                                   std::string* result) OVERRIDE {
     return false;
   }
 
diff --git a/chrome/browser/policy/browser_policy_connector.cc b/chrome/browser/policy/browser_policy_connector.cc
index 8e2c3ce9..e94f0d6 100644
--- a/chrome/browser/policy/browser_policy_connector.cc
+++ b/chrome/browser/policy/browser_policy_connector.cc
@@ -442,7 +442,9 @@ void BrowserPolicyConnector::CompleteInitialization() {
         kServiceInitializationStartupDelay);
   }
   device_data_store_->set_device_status_collector(
-      new DeviceStatusCollector(g_browser_process->local_state()));
+      new DeviceStatusCollector(
+          g_browser_process->local_state(),
+          chromeos::system::StatisticsProvider::GetInstance()));
 #endif
 }
 
diff --git a/chrome/browser/policy/device_status_collector.cc b/chrome/browser/policy/device_status_collector.cc
index 44722a2..3d82084 100644
--- a/chrome/browser/policy/device_status_collector.cc
+++ b/chrome/browser/policy/device_status_collector.cc
@@ -7,6 +7,7 @@
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/string_number_conversions.h"
+#include "chrome/browser/chromeos/system/statistics_provider.h"
 #include "chrome/browser/policy/proto/device_management_backend.pb.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/prefs/scoped_user_pref_update.h"
@@ -42,11 +43,14 @@ bool GetTimestamp(const ListValue* list, int index, int64* out_value) {
 
 namespace policy {
 
-DeviceStatusCollector::DeviceStatusCollector(PrefService* local_state)
+DeviceStatusCollector::DeviceStatusCollector(
+    PrefService* local_state,
+    chromeos::system::StatisticsProvider* provider)
     : max_stored_active_periods_(kMaxStoredActivePeriods),
       local_state_(local_state),
       last_idle_check_(Time()),
-      last_idle_state_(IDLE_STATE_UNKNOWN) {
+      last_idle_state_(IDLE_STATE_UNKNOWN),
+      statistics_provider_(provider) {
   timer_.Start(FROM_HERE,
                TimeDelta::FromSeconds(
                    DeviceStatusCollector::kPollIntervalSeconds),
@@ -131,6 +135,7 @@ void DeviceStatusCollector::IdleStateCallback(IdleState state) {
 }
 
 void DeviceStatusCollector::GetStatus(em::DeviceStatusReportRequest* request) {
+  // Report device active periods.
   const ListValue* active_periods =
       local_state_->GetList(kPrefDeviceActivePeriods);
   em::TimePeriod* time_period;
@@ -159,6 +164,16 @@ void DeviceStatusCollector::GetStatus(em::DeviceStatusReportRequest* request) {
   request->set_browser_version(version_info.Version());
   request->set_os_version(os_version_);
   request->set_firmware_version(firmware_version_);
+
+  // Report the state of the dev switch at boot.
+  std::string dev_switch_mode;
+  if (statistics_provider_->GetMachineStatistic(
+      "devsw_boot", &dev_switch_mode)) {
+    if (dev_switch_mode == "1")
+      request->set_boot_mode("Dev");
+    else if (dev_switch_mode == "0")
+      request->set_boot_mode("Verified");
+  }
 }
 
 void DeviceStatusCollector::OnOSVersion(VersionLoader::Handle handle,
diff --git a/chrome/browser/policy/device_status_collector.h b/chrome/browser/policy/device_status_collector.h
index 2be0a15..4f1c772 100644
--- a/chrome/browser/policy/device_status_collector.h
+++ b/chrome/browser/policy/device_status_collector.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 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.
 
@@ -11,6 +11,12 @@
 #include "chrome/browser/chromeos/version_loader.h"
 #include "chrome/browser/idle.h"
 
+namespace chromeos {
+namespace system {
+class StatisticsProvider;
+}
+}
+
 namespace enterprise_management {
 class DeviceStatusReportRequest;
 }
@@ -22,7 +28,8 @@ namespace policy {
 // Collects and summarizes the status of an enterprised-managed ChromeOS device.
 class DeviceStatusCollector {
  public:
-  explicit DeviceStatusCollector(PrefService* local_state);
+  DeviceStatusCollector(PrefService* local_state,
+                        chromeos::system::StatisticsProvider* provider);
   virtual ~DeviceStatusCollector();
 
   void GetStatus(enterprise_management::DeviceStatusReportRequest* request);
@@ -73,6 +80,8 @@ class DeviceStatusCollector {
   std::string os_version_;
   std::string firmware_version_;
 
+  chromeos::system::StatisticsProvider* statistics_provider_;
+
   DISALLOW_COPY_AND_ASSIGN(DeviceStatusCollector);
 };
 
diff --git a/chrome/browser/policy/device_status_collector_unittest.cc b/chrome/browser/policy/device_status_collector_unittest.cc
index 8bda4b8..1dde8b6 100644
--- a/chrome/browser/policy/device_status_collector_unittest.cc
+++ b/chrome/browser/policy/device_status_collector_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 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.
 
@@ -7,9 +7,11 @@
 #include "base/message_loop.h"
 #include "base/time.h"
 #include "chrome/browser/idle.h"
+#include "chrome/browser/chromeos/system/mock_statistics_provider.h"
 #include "chrome/browser/policy/proto/device_management_backend.pb.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/test/base/testing_pref_service.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using base::TimeDelta;
@@ -21,8 +23,10 @@ namespace {
 
 class TestingDeviceStatusCollector : public policy::DeviceStatusCollector {
  public:
-  explicit TestingDeviceStatusCollector(PrefService* local_state)
-    :  policy::DeviceStatusCollector(local_state),
+  TestingDeviceStatusCollector(
+      PrefService* local_state,
+      chromeos::system::StatisticsProvider* provider)
+    :  policy::DeviceStatusCollector(local_state, provider),
        local_state_(local_state),
        baseline_time_(Time::Now()) {
   }
@@ -79,13 +83,19 @@ int64 GetActiveMilliseconds(em::DeviceStatusReportRequest& status) {
 
 namespace policy {
 
+using ::testing::_;
+using ::testing::NotNull;
+using ::testing::Return;
+using ::testing::SetArgPointee;
+
 class DeviceStatusCollectorTest : public testing::Test {
  public:
   DeviceStatusCollectorTest()
-    :  message_loop_(new MessageLoop),
-       prefs_(),
-       status_collector_(&prefs_) {
+    :  prefs_(),
+       status_collector_(&prefs_, &statistics_provider_) {
     DeviceStatusCollector::RegisterPrefs(&prefs_);
+    EXPECT_CALL(statistics_provider_, GetMachineStatistic(_, NotNull()))
+        .WillRepeatedly(Return(false));
   }
 
  protected:
@@ -94,9 +104,9 @@ class DeviceStatusCollectorTest : public testing::Test {
     return policy::DeviceStatusCollector::kPollIntervalSeconds * 1000;
   }
 
-  scoped_ptr<MessageLoop> message_loop_;
-
+  MessageLoop message_loop_;
   TestingPrefService prefs_;
+  chromeos::system::MockStatisticsProvider statistics_provider_;
   TestingDeviceStatusCollector status_collector_;
   em::DeviceStatusReportRequest status_;
 };
@@ -180,7 +190,8 @@ TEST_F(DeviceStatusCollectorTest, StateKeptInPref) {
   // Process the list a second time with a different collector.
   // It should be able to count the active periods found by the first
   // collector, because the results are stored in a pref.
-  TestingDeviceStatusCollector second_collector(&prefs_);
+  TestingDeviceStatusCollector second_collector(&prefs_,
+                                                &statistics_provider_);
   second_collector.Simulate(test_states,
                             sizeof(test_states) / sizeof(IdleState));
 
@@ -226,4 +237,33 @@ TEST_F(DeviceStatusCollectorTest, MaxStoredPeriods) {
   EXPECT_EQ(static_cast<int>(max_periods), status_.active_time_size());
 }
 
+TEST_F(DeviceStatusCollectorTest, DevSwitchBootMode) {
+  status_collector_.GetStatus(&status_);
+  EXPECT_EQ(false, status_.has_boot_mode());
+
+  EXPECT_CALL(statistics_provider_,
+              GetMachineStatistic("devsw_boot", NotNull()))
+      .WillOnce(DoAll(SetArgPointee<1>("(error)"), Return(true)));
+  status_collector_.GetStatus(&status_);
+  EXPECT_EQ(false, status_.has_boot_mode());
+
+  EXPECT_CALL(statistics_provider_,
+              GetMachineStatistic("devsw_boot", NotNull()))
+      .WillOnce(DoAll(SetArgPointee<1>(" "), Return(true)));
+  status_collector_.GetStatus(&status_);
+  EXPECT_EQ(false, status_.has_boot_mode());
+
+  EXPECT_CALL(statistics_provider_,
+              GetMachineStatistic("devsw_boot", NotNull()))
+      .WillOnce(DoAll(SetArgPointee<1>("0"), Return(true)));
+  status_collector_.GetStatus(&status_);
+  EXPECT_EQ("Verified", status_.boot_mode());
+
+  EXPECT_CALL(statistics_provider_,
+              GetMachineStatistic("devsw_boot", NotNull()))
+      .WillOnce(DoAll(SetArgPointee<1>("1"), Return(true)));
+  status_collector_.GetStatus(&status_);
+  EXPECT_EQ("Dev", status_.boot_mode());
+}
+
 }  // namespace policy