Move statistics_provider to chromeos/system

chromeos/system/statistics_provider.cc

Index: chromeos/system/statistics_provider.cc
diff --git a/chrome/browser/chromeos/system/statistics_provider.cc b/chromeos/system/statistics_provider.cc
similarity index 47%
rename from chrome/browser/chromeos/system/statistics_provider.cc
rename to chromeos/system/statistics_provider.cc
index e5fd3dce5620a10065a1c4aa16e807fdf6f238e2..4cb5f20dd85bf2b28e8d31997319887cec77e2c3 100644
--- a/chrome/browser/chromeos/system/statistics_provider.cc
+++ b/chromeos/system/statistics_provider.cc
@@ -2,28 +2,28 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/chromeos/system/statistics_provider.h"
+#include "chromeos/system/statistics_provider.h"
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/files/file_path.h"
+#include "base/location.h"
 #include "base/logging.h"
-#include "base/memory/singleton.h"
 #include "base/path_service.h"
 #include "base/synchronization/waitable_event.h"
 #include "base/sys_info.h"
+#include "base/task_runner.h"
+#include "base/task_runner_util.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/time/time.h"
 #include "chromeos/app_mode/kiosk_oem_manifest_parser.h"
 #include "chromeos/chromeos_constants.h"
 #include "chromeos/chromeos_switches.h"
 #include "chromeos/system/name_value_pairs_parser.h"
-#include "content/public/browser/browser_thread.h"
-
-using content::BrowserThread;
 
 namespace chromeos {
 namespace system {
+
 namespace {
 
 // Path to the tool used to get system info, and delimiters for the output
@@ -35,7 +35,6 @@ const char kCrosSystemCommentDelim[] = "#";
 const char kCrosSystemUnknownValue[] = "(error)";
 
 const char kHardwareClassCrosSystemKey[] = "hwid";
-const char kHardwareClassKey[] = "hardware_class";
 const char kUnknownHardwareClass[] = "unknown";
 
 // File to get machine hardware info from, and key/value delimiters of
@@ -67,144 +66,148 @@ const CommandLine::CharType kOemManifestFilePath[] =
 
 // Key values for GetMachineStatistic()/GetMachineFlag() calls.
 const char kDevSwitchBootMode[] = "devsw_boot";
-const char kHardwareClass[] = "hardware_class";
+const char kHardwareClassKey[] = "hardware_class";
 const char kOffersCouponCodeKey[] = "ubind_attribute";
 const char kOffersGroupCodeKey[] = "gbind_attribute";
-const char kOemCanExitEnterpriseEnrollmentKey[] =
-    "oem_can_exit_enrollment";
-const char kOemDeviceRequisitionKey[] =
-    "oem_device_requisition";
-const char kOemIsEnterpriseManagedKey[] =
-    "oem_enterprise_managed";
-const char kOemKeyboardDrivenOobeKey[] =
-    "oem_keyboard_driven_oobe";
+const char kOemCanExitEnterpriseEnrollmentKey[] = "oem_can_exit_enrollment";
+const char kOemDeviceRequisitionKey[] = "oem_device_requisition";
+const char kOemIsEnterpriseManagedKey[] = "oem_enterprise_managed";
+const char kOemKeyboardDrivenOobeKey[] = "oem_keyboard_driven_oobe";
 
 // The StatisticsProvider implementation used in production.
 class StatisticsProviderImpl : public StatisticsProvider {
  public:
+  explicit StatisticsProviderImpl(
+      const scoped_refptr<base::TaskRunner>& file_task_runner);
+  virtual ~StatisticsProviderImpl();
+
   // StatisticsProvider implementation:
-  virtual void Init() OVERRIDE;
-  virtual void StartLoadingMachineStatistics() OVERRIDE;
+  virtual void StartLoadingMachineStatistics(bool load_oem_manifest) OVERRIDE;
   virtual bool GetMachineStatistic(const std::string& name,
                                    std::string* result) OVERRIDE;
-  virtual bool GetMachineFlag(const std::string& name,
-                              bool* result) OVERRIDE;
-  virtual void LoadOemManifest() OVERRIDE;
-
-  static StatisticsProviderImpl* GetInstance();
+  virtual bool GetMachineFlag(const std::string& name, bool* result) OVERRIDE;
 
  protected:
-  StatisticsProviderImpl();
-  void LoadOemManifestFromFile(const base::FilePath& file);
-
- private:
   typedef std::map<std::string, bool> MachineFlags;
-  friend struct DefaultSingletonTraits<StatisticsProviderImpl>;
 
-  // Loads the machine statistcs by examining the system.
-  void LoadMachineStatistics();
+  // Waits up to |kTimeoutSecs| for statistics to be loaded. Returns true if
+  // they were loaded successfully.
+  bool WaitForStatisticsLoaded();
+
+  // Loads the machine statistics off of disk. Runs on the file thread.
+  void LoadMachineStatistics(bool load_oem_manifest);
+
+  // Loads the OEM statistics off of disk. Runs on the file thread.
+  void LoadOemManifestFromFile(const base::FilePath& file);
 
-  bool initialized_;
   bool load_statistics_started_;
+  scoped_refptr<base::TaskRunner> file_task_runner_;
+  // |on_statistics_loaded_| protects |machine_info_| and |machine_flags_|.
+  base::WaitableEvent on_statistics_loaded_;
   NameValuePairsParser::NameValueMap machine_info_;
   MachineFlags machine_flags_;
-  base::WaitableEvent on_statistics_loaded_;
+
+ private:
+  base::WeakPtrFactory<StatisticsProviderImpl> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(StatisticsProviderImpl);
 };
 
-void StatisticsProviderImpl::Init() {
-  DCHECK(!initialized_);
-  initialized_ = true;
+bool StatisticsProviderImpl::WaitForStatisticsLoaded() {
+  CHECK(load_statistics_started_);
+  if (on_statistics_loaded_.IsSignaled())
+    return true;
+
+  // Block if the statistics are not loaded yet. Normally this shouldn't
+  // happen excpet during OOBE.
+  base::Time start_time = base::Time::Now();
+  base::ThreadRestrictions::ScopedAllowWait allow_wait;
+  on_statistics_loaded_.TimedWait(base::TimeDelta::FromSeconds(kTimeoutSecs));
+
+  base::TimeDelta dtime = base::Time::Now() - start_time;
+  if (on_statistics_loaded_.IsSignaled()) {
+    LOG(ERROR) << "Statistics loaded after waiting "
+               << dtime.InMilliseconds() << "ms. ";
+    return true;
+  }
+
+  LOG(ERROR) << "Statistics not loaded after waiting "
+             << dtime.InMilliseconds() << "ms. ";
+  return false;
 }
 
 bool StatisticsProviderImpl::GetMachineStatistic(
     const std::string& name, std::string* result) {
-  CHECK(initialized_);
-  CHECK(load_statistics_started_)
-      << "GetMachineStatistic called before load started: " << name;
-
-  VLOG(1) << "Statistic is requested for " << name;
-  // Block if the statistics are not loaded yet. Per LOG(WARNING) below,
-  // the statistics are loaded before requested as of now. For regular
-  // sessions (i.e. not OOBE), statistics are first requested when the
-  // user is logging in so we have plenty of time to load the data
-  // beforehand.
-  //
-  // If you see the warning appeared for regular sessions, it probably
-  // means that there is new client code that uses the statistics in the
-  // very early stage of the browser startup. The statistic name should be
-  // helpful to identify the caller.
-  if (!on_statistics_loaded_.IsSignaled()) {
-    // http://crbug.com/125385
-    base::Time start_time = base::Time::Now();
-    base::ThreadRestrictions::ScopedAllowWait allow_wait;
-    on_statistics_loaded_.TimedWait(base::TimeDelta::FromSeconds(kTimeoutSecs));
-    base::TimeDelta dtime = base::Time::Now() - start_time;
-    if (!on_statistics_loaded_.IsSignaled()) {
-      LOG(ERROR) << "Statistics weren't loaded after waiting "
-                 << dtime.InMilliseconds() << "ms. "
-                 << "Requested statistic: " << name;
-      return false;
-    } else {
-      LOG(ERROR) << "Statistic loaded after waiting "
-                 << dtime.InMilliseconds() << "ms. "
-                 << "Requested statistic: " << name;
-    }
+  VLOG(1) << "Machine Statistic requested: " << name;
+  if (!WaitForStatisticsLoaded()) {
+    LOG(ERROR) << "GetMachineStatistic called before load started: " << name;
+    return false;
   }
 
   NameValuePairsParser::NameValueMap::iterator iter = machine_info_.find(name);
-  if (iter != machine_info_.end()) {
-    *result = iter->second;
-    return true;
+  if (iter == machine_info_.end()) {
+    if (base::SysInfo::IsRunningOnChromeOS())
+      LOG(ERROR) << "Requested statistic not found: " << name;
+    return false;
   }
-  return false;
+  *result = iter->second;
+  return true;
 }
 
-bool StatisticsProviderImpl::GetMachineFlag(
-    const std::string& name, bool* result) {
-  MachineFlags::const_iterator iter = machine_flags_.find(name);
-  if (iter != machine_flags_.end()) {
-    *result = iter->second;
-    return true;
+bool StatisticsProviderImpl::GetMachineFlag(const std::string& name,
+                                            bool* result) {
+  VLOG(1) << "Machine Flag requested: " << name;
+  if (!WaitForStatisticsLoaded()) {
+    LOG(ERROR) << "GetMachineFlag called before load started: " << name;
+    return false;
   }
 
-  return false;
+  MachineFlags::const_iterator iter = machine_flags_.find(name);
+  if (iter == machine_flags_.end()) {
+    if (base::SysInfo::IsRunningOnChromeOS())
+      LOG(ERROR) << "Requested machine flag not found: " << name;
+    return false;
+  }
+  *result = iter->second;
+  return true;
 }
 
-void StatisticsProviderImpl::LoadOemManifest() {
-  LoadOemManifestFromFile(base::FilePath(kOemManifestFilePath));
+StatisticsProviderImpl::StatisticsProviderImpl(
+    const scoped_refptr<base::TaskRunner>& file_task_runner)
+    : load_statistics_started_(false),
+      file_task_runner_(file_task_runner),
+      on_statistics_loaded_(true  /* manual_reset */,
+                            false /* initially_signaled */),
+      weak_ptr_factory_(this) {
 }
 
-// manual_reset needs to be true, as we want to keep the signaled state.
-StatisticsProviderImpl::StatisticsProviderImpl()
-    : initialized_(false),
-      load_statistics_started_(false),
-      on_statistics_loaded_(true  /* manual_reset */,
-                            false /* initially_signaled */) {
+StatisticsProviderImpl::~StatisticsProviderImpl() {
 }
 
-void StatisticsProviderImpl::StartLoadingMachineStatistics() {
-  DCHECK(initialized_);
-  DCHECK(!load_statistics_started_);
+void StatisticsProviderImpl::StartLoadingMachineStatistics(
+    bool load_oem_manifest) {
+  CHECK(!load_statistics_started_);

[satorux1, 2013/10/04 08:07:56]

DCHECK?

[stevenjb, 2013/10/04 18:48:19]

We don't run DEBUG builds on devices often enough. At one point this was
actually failing in ToT, so I am intentionally making this a CHECK.

   load_statistics_started_ = true;
 
-  VLOG(1) << "Started loading statistics";
-  BrowserThread::PostBlockingPoolTask(
+  VLOG(1) << "Started loading statistics. Load OEM Manifest: "
+          << load_oem_manifest;
+
+  file_task_runner_->PostTask(
       FROM_HERE,
       base::Bind(&StatisticsProviderImpl::LoadMachineStatistics,
-                 base::Unretained(this)));
+                 weak_ptr_factory_.GetWeakPtr(),

[satorux1, 2013/10/04 08:07:56]

you cannot use a weak ptr to post a task to another thread. I'd suggest to use
PostTaskAndReplyWithResult like:

PostTaskAndReplyWithResult(
  // This returns the loaded data.
  base::Bind(&FreeFunction),
  // This receives the loaded data, and copies it to members.
  base::Bind(&StatisticsProviderImpl::OnDone,
             weak_ptr_factory_.GetWeakPtr()));

[satorux1, 2013/10/04 08:13:00]

I think it will DCHECK-fail with a Debug build.

[stevenjb, 2013/10/04 18:48:19]

Ugh, right. I had it that way, but on_statistics_loaded_.Signal() needs to
happen on the File thread. 
I think I need to switch this back to a Singleton<> and use Unretained,
otherwise the code gets unnecessarily complicated. Thanks for catching this.

+                 load_oem_manifest));
 }
 
-void StatisticsProviderImpl::LoadMachineStatistics() {
-  NameValuePairsParser parser(&machine_info_);
-
+void StatisticsProviderImpl::LoadMachineStatistics(bool load_oem_manifest) {
   // Parse all of the key/value pairs from the crossystem tool.
-  if (!parser.ParseNameValuePairsFromTool(
-          arraysize(kCrosSystemTool), kCrosSystemTool, kCrosSystemEq,
-          kCrosSystemDelim, kCrosSystemCommentDelim)) {
-    LOG(WARNING) << "There were errors parsing the output of "
-                 << kCrosSystemTool << ".";
+  NameValuePairsParser parser(&machine_info_);
+  if (!parser.ParseNameValuePairsFromTool(arraysize(kCrosSystemTool),
+                                          kCrosSystemTool,
+                                          kCrosSystemEq,
+                                          kCrosSystemDelim,
+                                          kCrosSystemCommentDelim)) {
+    LOG(ERROR) << "Errors parsing output from: " << kCrosSystemTool;
   }
 
   // Ensure that the hardware class key is present with the expected
@@ -223,17 +226,21 @@ void StatisticsProviderImpl::LoadMachineStatistics() {
                                    kEchoCouponDelim);
   parser.GetNameValuePairsFromFile(base::FilePath(kVpdFile), kVpdEq, kVpdDelim);
 
+  if (load_oem_manifest)
+    LoadOemManifestFromFile(base::FilePath(kOemManifestFilePath));
+
   // Finished loading the statistics.
   on_statistics_loaded_.Signal();
-  VLOG(1) << "Finished loading statistics";
+  VLOG(1) << "Finished loading statistics.";
 }
 
 void StatisticsProviderImpl::LoadOemManifestFromFile(
     const base::FilePath& file) {
   KioskOemManifestParser::Manifest oem_manifest;
-  if (!KioskOemManifestParser::Load(file, &oem_manifest))
+  if (!KioskOemManifestParser::Load(file, &oem_manifest)) {
+    LOG(WARNING) << "Unable to load OEM Manifest file: " << file.value();
     return;
-
+  }
   machine_info_[kOemDeviceRequisitionKey] =
       oem_manifest.device_requisition;
   machine_flags_[kOemIsEnterpriseManagedKey] =
@@ -242,70 +249,79 @@ void StatisticsProviderImpl::LoadOemManifestFromFile(
       oem_manifest.can_exit_enrollment;
   machine_flags_[kOemKeyboardDrivenOobeKey] =
       oem_manifest.keyboard_driven_oobe;
-}
 
-StatisticsProviderImpl* StatisticsProviderImpl::GetInstance() {
-  return Singleton<StatisticsProviderImpl,
-                   DefaultSingletonTraits<StatisticsProviderImpl> >::get();
+  VLOG(1) << "Loaded OEM Manifest statistics from " << file.value();
 }
 
 // The stub StatisticsProvider implementation used on Linux desktop.
 class StatisticsProviderStubImpl : public StatisticsProviderImpl {
  public:
-  // StatisticsProvider implementation:
-  virtual void Init() OVERRIDE {}
-
-  virtual void StartLoadingMachineStatistics() OVERRIDE {}
-
-  virtual bool GetMachineStatistic(const std::string& name,
-                                   std::string* result) OVERRIDE {
-    if (name == "CHROMEOS_RELEASE_BOARD") {
-      // Note: syncer::GetSessionNameSynchronously() also uses the mechanism
-      // below to determine the CrOs release board. However, it cannot include
-      // statistics_provider.h and use this method because of the mutual
-      // dependency that creates between sync.gyp:sync and chrome.gyp:browser.
-      // TODO(rsimha): Update syncer::GetSessionNameSynchronously() if this code
-      // is ever moved into base/. See http://crbug.com/126732.
-      const CommandLine* command_line = CommandLine::ForCurrentProcess();
-      if (command_line->HasSwitch(chromeos::switches::kChromeOSReleaseBoard)) {
-        *result = command_line->
-            GetSwitchValueASCII(chromeos::switches::kChromeOSReleaseBoard);
-        return true;
-      }
-    }
-    return false;
+  explicit StatisticsProviderStubImpl(
+      const scoped_refptr<base::TaskRunner>& file_task_runner)
+      : StatisticsProviderImpl(file_task_runner),
+        weak_ptr_factory_stub_(this) {
   }
-
-  virtual void LoadOemManifest() OVERRIDE {
-    CommandLine* command_line = CommandLine::ForCurrentProcess();
-    if (!command_line->HasSwitch(switches::kAppOemManifestFile))
-      return;
-
-    LoadOemManifestFromFile(
-        command_line->GetSwitchValuePath(switches::kAppOemManifestFile));
+  virtual ~StatisticsProviderStubImpl() {
   }
 
-  static StatisticsProviderStubImpl* GetInstance() {
-    return Singleton<StatisticsProviderStubImpl,
-        DefaultSingletonTraits<StatisticsProviderStubImpl> >::get();
+  // StatisticsProvider implementation:
+  virtual void StartLoadingMachineStatistics(bool load_oem_manifest) OVERRIDE {
+    CHECK(!load_statistics_started_);

[satorux1, 2013/10/04 08:07:56]

ditto

+    load_statistics_started_ = true;
+
+    VLOG(1) << "Started loading STUB statistics. Load OEM Manifest: "
+            << load_oem_manifest;
+
+    file_task_runner_->PostTask(
+        FROM_HERE,
+        base::Bind(&StatisticsProviderStubImpl::LoadStubMachineStatistics,
+                   weak_ptr_factory_stub_.GetWeakPtr(),

[satorux1, 2013/10/04 08:07:56]

likewise, you cannot use weak ptr for posting task to another thread.

+                   load_oem_manifest));
   }
 
  private:
-  friend struct DefaultSingletonTraits<StatisticsProviderStubImpl>;
-
-  StatisticsProviderStubImpl() {
+  void LoadStubMachineStatistics(bool load_oem_manifest) {
+    if (load_oem_manifest) {
+      // If kAppOemManifestFile switch is specified, load OEM Manifest file.
+      CommandLine* command_line = CommandLine::ForCurrentProcess();
+      if (command_line->HasSwitch(switches::kAppOemManifestFile)) {
+        LoadOemManifestFromFile(
+            command_line->GetSwitchValuePath(switches::kAppOemManifestFile));
+      }
+    }
+    // Finished loading the statistics.
+    on_statistics_loaded_.Signal();
+    VLOG(1) << "Finished loading STUB statistics";
   }
 
+  base::WeakPtrFactory<StatisticsProviderStubImpl> weak_ptr_factory_stub_;
+
   DISALLOW_COPY_AND_ASSIGN(StatisticsProviderStubImpl);
 };
 
-StatisticsProvider* StatisticsProvider::GetInstance() {
+static StatisticsProvider* g_statistics_provider = NULL;
+
+// static
+void StatisticsProvider::Initialize(
+    const scoped_refptr<base::TaskRunner>& file_task_runner) {
+  CHECK(!g_statistics_provider);
   if (base::SysInfo::IsRunningOnChromeOS()) {
-    return StatisticsProviderImpl::GetInstance();
+    g_statistics_provider = new StatisticsProviderImpl(file_task_runner);
   } else {
-    return StatisticsProviderStubImpl::GetInstance();
+    g_statistics_provider = new StatisticsProviderStubImpl(file_task_runner);
   }
 }
 
+// static
+void StatisticsProvider::Shutdown() {
+  delete g_statistics_provider;
+  g_statistics_provider = NULL;
+}
+
+StatisticsProvider* StatisticsProvider::GetInstance() {
+  CHECK(g_statistics_provider);
+  return g_statistics_provider;
+}
+
 }  // namespace system
 }  // namespace chromeos