Decouple loading of channel info and the rest of machine statistics.

chrome/browser/chromeos/system/statistics_provider.cc

 // 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.
 
 #include "chrome/browser/chromeos/system/statistics_provider.h"
 
 #include "base/bind.h"
 #include "base/chromeos/chromeos_version.h"
 #include "base/command_line.h"
 #include "base/file_path.h"
@@ skipping 53 matching lines @@
 // Timeout that we should wait for statistics to get loaded
 const int kTimeoutSecs = 3;
 
 }  // namespace
 
 // The StatisticsProvider implementation used in production.
 class StatisticsProviderImpl : public StatisticsProvider {
  public:
   // StatisticsProvider implementation:
   virtual void Init() OVERRIDE;
+  virtual void StartLoadingMachineStatistics() OVERRIDE;
   virtual bool GetMachineStatistic(const std::string& name,
                                    std::string* result) OVERRIDE;
 
   static StatisticsProviderImpl* GetInstance();
 
  private:
   friend struct DefaultSingletonTraits<StatisticsProviderImpl>;
 
   StatisticsProviderImpl();
 
   // Loads the machine info file, which is necessary to get the Chrome channel.
   // Treat MachineOSInfoFile specially, as distribution channel information
   // (stable, beta, dev, canary) is required at earlier stage than everything
   // else. Rather than posting a delayed task, read and parse the machine OS
   // info file immediately.
   void LoadMachineOSInfoFile();
 
-  // Starts loading the machine statistcs.
-  void StartLoadingMachineStatistics();
-
   // Loads the machine statistcs by examining the system.
   void LoadMachineStatistics();
 
   bool initialized_;
+  bool load_statistics_started_;
   NameValuePairsParser::NameValueMap machine_info_;
   base::WaitableEvent on_statistics_loaded_;
 
   DISALLOW_COPY_AND_ASSIGN(StatisticsProviderImpl);
 };
 
 void StatisticsProviderImpl::Init() {
   DCHECK(!initialized_);
   initialized_ = true;
 
-  // Load the machine info file immediately to get the channel info and delay
-  // loading the remaining statistics.
+  // Load the machine info file immediately to get the channel info.
   LoadMachineOSInfoFile();
-  StartLoadingMachineStatistics();
 }
 
 bool StatisticsProviderImpl::GetMachineStatistic(
     const std::string& name, std::string* result) {
   DCHECK(initialized_);
+  DCHECK(load_statistics_started_);
 
   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
@@ skipping 17 matching lines @@
   if (iter != machine_info_.end()) {
     *result = iter->second;
     return true;
   }
   return false;
 }
 
 // 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 */) {
 }
 
 void StatisticsProviderImpl::LoadMachineOSInfoFile() {
   NameValuePairsParser parser(&machine_info_);
   if (parser.GetNameValuePairsFromFile(FilePath(kMachineOSInfoFile),
                                        kMachineOSInfoEq,
                                        kMachineOSInfoDelim)) {
 #if defined(GOOGLE_CHROME_BUILD)
     const char kChromeOSReleaseTrack[] = "CHROMEOS_RELEASE_TRACK";
     NameValuePairsParser::NameValueMap::iterator iter =
         machine_info_.find(kChromeOSReleaseTrack);
     if (iter != machine_info_.end())
       chrome::VersionInfo::SetChannel(iter->second);
 #endif
   }
 }
 
 void StatisticsProviderImpl::StartLoadingMachineStatistics() {

[Alexei Svitkine (slow), 2013/01/07 19:39:00]

Actually, can you also add DCHECK(initialized_); here?

[hshi1, 2013/01/07 19:41:16]

Done.

+  DCHECK(!load_statistics_started_);
+  load_statistics_started_ = true;
+
   VLOG(1) << "Started loading statistics";
   BrowserThread::PostBlockingPoolTask(
       FROM_HERE,
       base::Bind(&StatisticsProviderImpl::LoadMachineStatistics,
                  base::Unretained(this)));
 }
 
 void StatisticsProviderImpl::LoadMachineStatistics() {
   NameValuePairsParser parser(&machine_info_);
 
@@ skipping 28 matching lines @@
 
 StatisticsProviderImpl* StatisticsProviderImpl::GetInstance() {
   return Singleton<StatisticsProviderImpl,
                    DefaultSingletonTraits<StatisticsProviderImpl> >::get();
 }
 
 // The stub StatisticsProvider implementation used on Linux desktop.
 class StatisticsProviderStubImpl : public StatisticsProvider {
  public:
   // StatisticsProvider implementation:
-  virtual void Init() OVERRIDE {
-  }
+  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.
@@ skipping 24 matching lines @@
 StatisticsProvider* StatisticsProvider::GetInstance() {
   if (base::chromeos::IsRunningOnChromeOS()) {
     return StatisticsProviderImpl::GetInstance();
   } else {
     return StatisticsProviderStubImpl::GetInstance();
   }
 }
 
 }  // namespace system
 }  // namespace chromeos