Added asynchronous notification of readiness to the StatisticsProvider, and

chrome/browser/chromeos/system/statistics_provider.cc

Index: chrome/browser/chromeos/system/statistics_provider.cc
diff --git a/chrome/browser/chromeos/system/statistics_provider.cc b/chrome/browser/chromeos/system/statistics_provider.cc
index 28118635c75ba22ebcadc78211511a041f5f2bff..fcfd52b261a562351153b54c150e42a7990645c2 100644
--- a/chrome/browser/chromeos/system/statistics_provider.cc
+++ b/chrome/browser/chromeos/system/statistics_provider.cc
@@ -10,6 +10,9 @@
 #include "base/file_util.h"
 #include "base/logging.h"
 #include "base/memory/singleton.h"
+#include "base/memory/weak_ptr.h"
+#include "base/metrics/histogram.h"
+#include "base/observer_list.h"
 #include "base/synchronization/waitable_event.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/time.h"
@@ -72,6 +75,9 @@ class StatisticsProviderImpl : public StatisticsProvider {
   virtual bool GetMachineStatistic(const std::string& name,
                                    std::string* result) OVERRIDE;
 
+  virtual void AddObserver(Observer* observer) OVERRIDE;
+  virtual void RemoveObserver(Observer* observer) OVERRIDE;
+
   static StatisticsProviderImpl* GetInstance();
 
  private:
@@ -82,19 +88,27 @@ class StatisticsProviderImpl : public StatisticsProvider {
   // Starts loading the machine statistcs.
   void StartLoadingMachineStatistics();
 
-  // Loads the machine statistcs by examining the system.
-  void LoadMachineStatistics();
+  // Loads the machine statistics by examining the system.
+  // This is executed on the blocking pool, and replies by posting a task to
+  // the UI thread using |weak_this|.
+  void LoadMachineStatistics(base::WeakPtr<StatisticsProviderImpl> weak_this,
+                             base::Time start_time);
+
+  // Posted to UI from LoadMachineStatistics() once the info has been read.
+  void OnStatisticsLoaded(base::Time start_time);
 
   NameValuePairsParser::NameValueMap machine_info_;
   base::WaitableEvent on_statistics_loaded_;
+  base::WeakPtrFactory<StatisticsProviderImpl> weak_factory_;
+  ObserverList<Observer, true> observer_list_;
 
   DISALLOW_COPY_AND_ASSIGN(StatisticsProviderImpl);
 };
 
-bool StatisticsProviderImpl::GetMachineStatistic(
-    const std::string& name, std::string* result) {
+bool StatisticsProviderImpl::GetMachineStatistic(const std::string& name,
+                                                 std::string* result) {
   VLOG(1) << "Statistic is requested for " << name;
-  // Block if the statistics are not loaded yet. Per LOG(WARNING) below,
+  // Block if the statistics are not loaded yet. Per DLOG(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
@@ -105,15 +119,16 @@ bool StatisticsProviderImpl::GetMachineStatistic(
   // very early stage of the browser startup. The statistic name should be
   // helpful to identify the caller.
   if (!on_statistics_loaded_.IsSignaled()) {
-    LOG(WARNING) << "Waiting to load statistics. Requested statistic: "
-                 << name;
+    DLOG(WARNING) << "Waiting to load statistics. Requested statistic: "
+                  << name;
     // http://crbug.com/125385
     base::ThreadRestrictions::ScopedAllowWait allow_wait;
     on_statistics_loaded_.TimedWait(base::TimeDelta::FromSeconds(kTimeoutSecs));
 
     if (!on_statistics_loaded_.IsSignaled()) {
-      LOG(ERROR) << "Statistics weren't loaded after waiting! "
-                 << "Requested statistic: " << name;
+      DLOG(ERROR) << "Statistics weren't loaded after waiting! "
+                  << "Requested statistic: " << name;
+      UMA_HISTOGRAM_BOOLEAN("Cros.StatisticsProviderTimedOut", true);
       return false;
     }
   }
@@ -126,10 +141,24 @@ bool StatisticsProviderImpl::GetMachineStatistic(
   return false;
 }
 
+void StatisticsProviderImpl::AddObserver(Observer* observer) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  if (on_statistics_loaded_.IsSignaled()) {
+    observer->OnMachineStatisticsReady();
+  } else {
+    observer_list_.AddObserver(observer);
+  }
+}
+
+void StatisticsProviderImpl::RemoveObserver(Observer* observer) {
+  observer_list_.RemoveObserver(observer);
+}
+
 // manual_reset needs to be true, as we want to keep the signaled state.
 StatisticsProviderImpl::StatisticsProviderImpl()
     : on_statistics_loaded_(true  /* manual_reset */,
-                            false /* initially_signaled */) {
+                            false /* initially_signaled */),
+      ALLOW_THIS_IN_INITIALIZER_LIST(weak_factory_(this)) {
   StartLoadingMachineStatistics();
 }
 
@@ -138,18 +167,23 @@ void StatisticsProviderImpl::StartLoadingMachineStatistics() {
   BrowserThread::PostBlockingPoolTask(
       FROM_HERE,
       base::Bind(&StatisticsProviderImpl::LoadMachineStatistics,
-                 base::Unretained(this)));
+                 base::Unretained(this),
+                 weak_factory_.GetWeakPtr(),
+                 base::Time::Now()));
 }
 
-void StatisticsProviderImpl::LoadMachineStatistics() {
+void StatisticsProviderImpl::LoadMachineStatistics(
+    base::WeakPtr<StatisticsProviderImpl> weak_this,
+    base::Time start_time) {
   NameValuePairsParser parser(&machine_info_);
 
   // 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 << ".";
+    DLOG(WARNING) << "There were errors parsing the output of "
+                  << kCrosSystemTool << ".";
+    UMA_HISTOGRAM_BOOLEAN("Cros.CrosSystemParsingFailed", true);
   }
 
   // Ensure that the hardware class key is present with the expected
@@ -175,6 +209,12 @@ void StatisticsProviderImpl::LoadMachineStatistics() {
   on_statistics_loaded_.Signal();
   VLOG(1) << "Finished loading statistics";
 
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&StatisticsProviderImpl::OnStatisticsLoaded,
+                 weak_this,
+                 start_time));
+
 #if defined(GOOGLE_CHROME_BUILD)
   // TODO(kochi): This is for providing a channel information to
   // chrome::VersionInfo::GetChannel()/GetVersionStringModifier(),
@@ -199,6 +239,13 @@ void StatisticsProviderImpl::LoadMachineStatistics() {
 #endif
 }
 
+void StatisticsProviderImpl::OnStatisticsLoaded(base::Time start_time) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  base::TimeDelta elapsed = base::Time::Now() - start_time;
+  UMA_HISTOGRAM_LONG_TIMES("Cros.StatisticsProviderReadyDelay", elapsed);
+  FOR_EACH_OBSERVER(Observer, observer_list_, OnMachineStatisticsReady());
+}
+
 StatisticsProviderImpl* StatisticsProviderImpl::GetInstance() {
   return Singleton<StatisticsProviderImpl,
                    DefaultSingletonTraits<StatisticsProviderImpl> >::get();
@@ -213,6 +260,13 @@ class StatisticsProviderStubImpl : public StatisticsProvider {
     return false;
   }
 
+  virtual void AddObserver(Observer* observer) OVERRIDE {
+    DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+    observer->OnMachineStatisticsReady();
+  }
+
+  virtual void RemoveObserver(Observer* observer) OVERRIDE {}
+
   static StatisticsProviderStubImpl* GetInstance() {
     return Singleton<StatisticsProviderStubImpl,
         DefaultSingletonTraits<StatisticsProviderStubImpl> >::get();