Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(2759)

Unified Diff: chrome/browser/chromeos/system/statistics_provider.cc

Issue 10078017: Added asynchronous notification of readiness to the StatisticsProvider, and (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Timeout while waiting for the hardware_class; sample UMA stats Created 8 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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 f7b88ff1d3504bf5ea5f538d4e6a5ce5be6fbfdf..7d82cd0ddbd0582672291e8fbb2609e075cb7aa4 100644
--- a/chrome/browser/chromeos/system/statistics_provider.cc
+++ b/chrome/browser/chromeos/system/statistics_provider.cc
@@ -5,11 +5,14 @@
#include "chrome/browser/chromeos/system/statistics_provider.h"
#include "base/bind.h"
+#include "base/bind_helpers.h"
#include "base/chromeos/chromeos_version.h"
#include "base/file_path.h"
#include "base/file_util.h"
#include "base/logging.h"
#include "base/memory/singleton.h"
+#include "base/message_loop.h"
+#include "base/metrics/histogram.h"
#include "base/synchronization/waitable_event.h"
#include "base/time.h"
#include "base/chromeos/chromeos_version.h"
@@ -62,6 +65,15 @@ const char kVpdDelim[] = "\n";
// Timeout that we should wait for statistics to get loaded
const int kTimeoutSecs = 3;
+// Helper to invoke |callbacks| on the UI thread.
+void NotifyOnUI(const std::vector<base::Closure>& callbacks) {
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+ for (std::vector<base::Closure>::const_iterator it = callbacks.begin();
+ it != callbacks.end(); ++it) {
+ it->Run();
+ }
+}
+
} // namespace
// The StatisticsProvider implementation used in production.
@@ -71,6 +83,8 @@ class StatisticsProviderImpl : public StatisticsProvider {
virtual bool GetMachineStatistic(const std::string& name,
std::string* result) OVERRIDE;
+ virtual void WhenReady(const base::Closure& callback) OVERRIDE;
+
static StatisticsProviderImpl* GetInstance();
private:
@@ -86,14 +100,16 @@ class StatisticsProviderImpl : public StatisticsProvider {
NameValuePairsParser::NameValueMap machine_info_;
base::WaitableEvent on_statistics_loaded_;
+ std::vector<base::Closure> callbacks_;
DISALLOW_COPY_AND_ASSIGN(StatisticsProviderImpl);
};
bool StatisticsProviderImpl::GetMachineStatistic(
- const std::string& name, std::string* result) {
+ 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
@@ -104,13 +120,13 @@ 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;
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;
return false;
}
}
@@ -123,6 +139,16 @@ bool StatisticsProviderImpl::GetMachineStatistic(
return false;
}
+void StatisticsProviderImpl::WhenReady(const base::Closure& callback) {
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+ if (on_statistics_loaded_.IsSignaled()) {
+ // Don't assume the caller is re-entrant.
+ BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, callback);
+ } else {
+ callbacks_.push_back(callback);
+ }
+}
+
// manual_reset needs to be true, as we want to keep the signaled state.
StatisticsProviderImpl::StatisticsProviderImpl()
: on_statistics_loaded_(true /* manual_reset */,
@@ -145,8 +171,9 @@ void StatisticsProviderImpl::LoadMachineStatistics() {
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
@@ -172,6 +199,13 @@ void StatisticsProviderImpl::LoadMachineStatistics() {
on_statistics_loaded_.Signal();
VLOG(1) << "Finished loading statistics";
+ // Any callbacks registed before Signal() are in |callbacks_|. Any registed
+ // after Signal() will be immediately posted. NotifyOnUI() posts the callbacks
+ // in |callbacks_| from the UI loop.
+ BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+ base::Bind(NotifyOnUI, callbacks_));
+ callbacks_.clear();
+
#if defined(GOOGLE_CHROME_BUILD)
// TODO(kochi): This is for providing a channel information to
// chrome::VersionInfo::GetChannel()/GetVersionStringModifier(),
@@ -210,6 +244,11 @@ class StatisticsProviderStubImpl : public StatisticsProvider {
return false;
}
+ virtual void WhenReady(const base::Closure& callback) OVERRIDE {
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+ BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, callback);
+ }
+
static StatisticsProviderStubImpl* GetInstance() {
return Singleton<StatisticsProviderStubImpl,
DefaultSingletonTraits<StatisticsProviderStubImpl> >::get();

Powered by Google App Engine
This is Rietveld 408576698