Add BytesRead metric to CPM

chrome/browser/performance_monitor/performance_monitor.cc

Index: chrome/browser/performance_monitor/performance_monitor.cc
diff --git a/chrome/browser/performance_monitor/performance_monitor.cc b/chrome/browser/performance_monitor/performance_monitor.cc
index 38f7192ca207bead318ace9cd48800521db71a54..99a1faeb5970873152d12964eee234b14a68faf4 100644
--- a/chrome/browser/performance_monitor/performance_monitor.cc
+++ b/chrome/browser/performance_monitor/performance_monitor.cc
@@ -34,6 +34,7 @@
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_types.h"
 #include "content/public/browser/web_contents.h"
+#include "net/url_request/url_request.h"
 
 using content::BrowserThread;
 using extensions::Extension;
@@ -61,6 +62,12 @@ bool StringToTime(std::string time, base::Time* output) {
 
 namespace performance_monitor {
 
+bool PerformanceMonitor::initialized_ = false;
+
+PerformanceMonitor::PerformanceDataForIOThread::PerformanceDataForIOThread()
+    : network_bytes_read(0) {
+}
+
 PerformanceMonitor::PerformanceMonitor() : database_(NULL) {
 }
 
@@ -94,6 +101,14 @@ void PerformanceMonitor::Start() {
 void PerformanceMonitor::InitOnBackgroundThread() {
   CHECK(!BrowserThread::CurrentlyOn(BrowserThread::UI));
   database_ = Database::Create(database_path_);
+
+  // Initialize the io thread's performance data to the value in the database;
+  // if there isn't a recording in the database, the value stays at 0.
+  MetricInfo info;
+  if (database_->GetRecentStatsForActivityAndMetric(METRIC_NETWORK_BYTES_READ,
+                                                    &info)) {
+    performance_data_for_io_thread_.network_bytes_read = info.value;

[battre, 2012/08/22 19:23:02]

you access the variable on the UI thread. Sorry that I did not notice this
earlier.

[Devlin, 2012/08/22 20:34:55]

Well, I've been wrong on threading every step of the way here, but I still
*think* this one's okay (I look forward to the reason I'm wrong!) :)

This happens (only) during the initialization of PerformanceMonitor, which is,
in itself, synchronous (it has thread jumping, but it does so synchronously).
Only at the *end* of the initialization is the initialized_ flag set to true.
OnRawBytesRead() checks this initialized flag to make sure that
PerformanceMonitor is initialized before calling BytesReadOnIOThread(). Thus, we
only call BytesReadOnIOThread() if we have conclusively already done the access
on this thread.

initialized_ isn't a thread-safety concern because it is a boolean - it is
either on or off. If it is even partially on , then the instruction was reached
and it is, in effect, fully on. Otherwise it is off, and should be off.

[battre, 2012/08/22 20:50:24]

Maybe you can move the check whether initialzed_ is true to
BytesReadOnIOThread(). That way the access is thread safe by the API of the
class and does not depend on correct calling conventions.

[Devlin, 2012/08/23 16:41:02]

Done.

+  }
 }
 
 void PerformanceMonitor::FinishInit() {
@@ -244,6 +259,8 @@ void PerformanceMonitor::NotifyInitialized() {
       chrome::NOTIFICATION_PERFORMANCE_MONITOR_INITIALIZED,
       content::Source<PerformanceMonitor>(this),
       content::NotificationService::NoDetails());
+
+  initialized_ = true;
 }
 
 void PerformanceMonitor::GatherStatisticsOnBackgroundThread() {
@@ -362,6 +379,45 @@ void PerformanceMonitor::UpdateLiveProfilesHelper(
 
 void PerformanceMonitor::DoTimedCollections() {
   UpdateLiveProfiles();
+
+  BrowserThread::PostBlockingPoolSequencedTask(
+      Database::kDatabaseSequenceToken,
+      FROM_HERE,
+      base::Bind(&PerformanceMonitor::GatherStatisticsOnBackgroundThread,
+                 base::Unretained(this)));
+
+  BrowserThread::PostTask(
+      BrowserThread::IO,
+      FROM_HERE,
+      base::Bind(&PerformanceMonitor::CallInsertIOData,
+                 base::Unretained(this)));
+}
+
+void PerformanceMonitor::CallInsertIOData() {
+  CHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+
+  BrowserThread::PostBlockingPoolSequencedTask(
+      Database::kDatabaseSequenceToken,
+      FROM_HERE,
+      base::Bind(&PerformanceMonitor::InsertIOData,
+                 base::Unretained(this),
+                 performance_data_for_io_thread_));
+}
+
+void PerformanceMonitor::InsertIOData(
+    PerformanceDataForIOThread performance_data_for_io_thread) {
+  CHECK(!BrowserThread::CurrentlyOn(BrowserThread::UI));
+  database_->AddMetric(
+      METRIC_NETWORK_BYTES_READ,
+      base::Uint64ToString(performance_data_for_io_thread.network_bytes_read));
+}
+
+void PerformanceMonitor::BytesReadOnIOThread(const net::URLRequest& request,
+                                             const int bytes_read) {
+  CHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+
+  if (!request.url().SchemeIsFile())
+    performance_data_for_io_thread_.network_bytes_read += bytes_read;
 }
 
 void PerformanceMonitor::Observe(int type,