[UMA] Use proper C++ objects to serialize tracked_objects across process boundaries.

chrome/browser/metrics/tracking_synchronizer.cc

Index: chrome/browser/metrics/tracking_synchronizer.cc
diff --git a/chrome/browser/metrics/tracking_synchronizer.cc b/chrome/browser/metrics/tracking_synchronizer.cc
index e64b9240883e4be90e228456d9941eb24f49bf0b..b0e717da6a36e9234f71df9d0a3d0a97487b3d15 100644
--- a/chrome/browser/metrics/tracking_synchronizer.cc
+++ b/chrome/browser/metrics/tracking_synchronizer.cc
@@ -9,31 +9,53 @@
 #include "base/process_util.h"
 #include "base/threading/thread.h"
 #include "base/tracked_objects.h"
+#include "chrome/browser/metrics/tracking_synchronizer_observer.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/profiler_controller.h"
-#include "content/public/common/process_type.h"
+#include "content/public/common/serialized_profiler_data.h"
 
 using base::TimeTicks;
 using content::BrowserThread;
 
+namespace {
+
+// Negative numbers are never used as sequence numbers.  We explicitly pick a
+// negative number that is "so negative" that even when we add one (as is done
+// when we generated the next sequence number) that it will still be negative.
+// We have code that handles wrapping around on an overflow into negative
+// territory.
+const int kNeverUsableSequenceNumber = -2;
+
+// TODO(isherman): Is there any reason not to use base/singleton.h and instead
+// roll our own version?

[Ilya Sherman, 2012/03/14 06:03:45]

Reviewers, thoughts?  Just less code this way?

+// This singleton instance should be started during the single threaded
+// portion of main(). It initializes globals to provide support for all future
+// calls. This object is created on the UI thread, and it is destroyed after
+// all the other threads have gone away. As a result, it is ok to call it
+// from the UI thread, or for about:profiler.
+static chrome_browser_metrics::TrackingSynchronizer* g_tracking_synchronizer =
+    NULL;
+
+}  // anonymous namespace
+
 namespace chrome_browser_metrics {
 
 // The "RequestContext" structure describes an individual request received
 // from the UI. All methods are accessible on UI thread.
-class RequestContext {
+class TrackingSynchronizer::RequestContext {
  public:
   // A map from sequence_number_ to the actual RequestContexts.
   typedef std::map<int, RequestContext*> RequestContextMap;
 
-  ~RequestContext() {}
-
-  RequestContext(const base::WeakPtr<ProfilerUI>& callback_object,
-                 int sequence_number)
+  RequestContext(
+      const base::WeakPtr<TrackingSynchronizerObserver>& callback_object,
+      int sequence_number)
       : callback_object_(callback_object),
         sequence_number_(sequence_number),
         received_process_group_count_(0),
         processes_pending_(0) {
   }
+  ~RequestContext() {}
 
   void SetReceivedProcessGroupCount(bool done) {
     DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
@@ -63,9 +85,8 @@ class RequestContext {
   void DeleteIfAllDone() {
     DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
-    if (processes_pending_ <= 0 && received_process_group_count_) {
+    if (processes_pending_ <= 0 && received_process_group_count_)
       RequestContext::Unregister(sequence_number_);
-    }
   }
 
 
@@ -73,7 +94,7 @@ class RequestContext {
   // |sequence_number|.
   static RequestContext* Register(
       int sequence_number,
-      const base::WeakPtr<ProfilerUI>& callback_object) {
+      const base::WeakPtr<TrackingSynchronizerObserver>& callback_object) {
     DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
     RequestContext* request = new RequestContext(
@@ -93,9 +114,8 @@ class RequestContext {
     if (it == outstanding_requests_.Get().end())
       return NULL;
 
-    RequestContext* request = NULL;
-    request = it->second;
-    DCHECK(sequence_number == request->sequence_number_);
+    RequestContext* request = it->second;
+    DCHECK_EQ(sequence_number, request->sequence_number_);
     return request;
   }
 
@@ -111,11 +131,13 @@ class RequestContext {
       return;
 
     RequestContext* request = it->second;
-    DCHECK(sequence_number == request->sequence_number_);
+    DCHECK_EQ(sequence_number, request->sequence_number_);
     bool received_process_group_count = request->received_process_group_count_;
     int unresponsive_processes = request->processes_pending_;
 
-    delete it->second;
+    request->callback_object_->FinishedReceivingProfilerData();
+
+    delete request;
     outstanding_requests_.Get().erase(it);
 
     UMA_HISTOGRAM_BOOLEAN("Profiling.ReceivedProcessGroupCount",
@@ -136,7 +158,7 @@ class RequestContext {
   }
 
   // Requests are made to asynchronously send data to the |callback_object_|.
-  base::WeakPtr<ProfilerUI> callback_object_;
+  base::WeakPtr<TrackingSynchronizerObserver> callback_object_;
 
   // The sequence number used by the most recent update request to contact all
   // processes.
@@ -154,22 +176,17 @@ class RequestContext {
   static base::LazyInstance<RequestContextMap> outstanding_requests_;
 };
 
-// Negative numbers are never used as sequence numbers.  We explicitly pick a
-// negative number that is "so negative" that even when we add one (as is done
-// when we generated the next sequence number) that it will still be negative.
-// We have code that handles wrapping around on an overflow into negative
-// territory.
-static const int kNeverUsableSequenceNumber = -2;
+// static
+base::LazyInstance<TrackingSynchronizer::RequestContext::RequestContextMap>
+    TrackingSynchronizer::RequestContext::outstanding_requests_ =
+        LAZY_INSTANCE_INITIALIZER;
 
 // TrackingSynchronizer methods and members.
-//
-// static
-TrackingSynchronizer* TrackingSynchronizer::tracking_synchronizer_ = NULL;
 
 TrackingSynchronizer::TrackingSynchronizer()
     : last_used_sequence_number_(kNeverUsableSequenceNumber) {
-  DCHECK(tracking_synchronizer_ == NULL);
-  tracking_synchronizer_ = this;
+  DCHECK(!g_tracking_synchronizer);
+  g_tracking_synchronizer = this;
   content::ProfilerController::GetInstance()->Register(this);
 }
 
@@ -179,21 +196,20 @@ TrackingSynchronizer::~TrackingSynchronizer() {
   // Just in case we have any pending tasks, clear them out.
   RequestContext::OnShutdown();
 
-  tracking_synchronizer_ = NULL;
+  g_tracking_synchronizer = NULL;
 }
 
 // static
 void TrackingSynchronizer::FetchProfilerDataAsynchronously(
-    const base::WeakPtr<ProfilerUI>& callback_object) {
+    const base::WeakPtr<TrackingSynchronizerObserver>& callback_object) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
-  TrackingSynchronizer* current_synchronizer = CurrentSynchronizer();
-  if (current_synchronizer == NULL) {
+  if (!g_tracking_synchronizer) {
     // System teardown is happening.
     return;
   }
 
-  int sequence_number = current_synchronizer->RegisterAndNotifyAllProcesses(
+  int sequence_number = g_tracking_synchronizer->RegisterAndNotifyAllProcesses(
       callback_object);
 
   // Post a task that would be called after waiting for wait_time.  This acts
@@ -219,24 +235,19 @@ void TrackingSynchronizer::OnPendingProcesses(int sequence_number,
 
 void TrackingSynchronizer::OnProfilerDataCollected(
     int sequence_number,
-    base::DictionaryValue* profiler_data) {
+    const content::SerializedProfilerData& profiler_data) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-
-  RequestContext* request = RequestContext::GetRequestContext(sequence_number);
-  if (!request)
-    return;
-
   DecrementPendingProcessesAndSendData(sequence_number, profiler_data);
 }
 
 int TrackingSynchronizer::RegisterAndNotifyAllProcesses(
-    const base::WeakPtr<ProfilerUI>& callback_object) {
+    const base::WeakPtr<TrackingSynchronizerObserver>& callback_object) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   int sequence_number = GetNextAvailableSequenceNumber();
 
   RequestContext* request =
-    RequestContext::Register(sequence_number, callback_object);
+      RequestContext::Register(sequence_number, callback_object);
 
   // Increment pending process count for sending browser's profiler data.
   request->IncrementProcessesPending();
@@ -245,33 +256,24 @@ int TrackingSynchronizer::RegisterAndNotifyAllProcesses(
   content::ProfilerController::GetInstance()->GetProfilerData(sequence_number);
 
   // Send profiler_data from browser process.
-  base::DictionaryValue* value = tracked_objects::ThreadData::ToValue(false);
-  const std::string process_type =
-      content::GetProcessTypeNameInEnglish(content::PROCESS_TYPE_BROWSER);
-  value->SetString("process_type", process_type);
-  value->SetInteger("process_id", base::GetCurrentProcId());
-  DecrementPendingProcessesAndSendData(sequence_number, value);
+  content::SerializedProfilerData profiler_data(content::PROCESS_TYPE_BROWSER);
+  tracked_objects::ThreadData::ToSerializedProcessData(false, &profiler_data);
+  DecrementPendingProcessesAndSendData(sequence_number, profiler_data);
 
   return sequence_number;
 }
 
 void TrackingSynchronizer::DecrementPendingProcessesAndSendData(
     int sequence_number,
-    base::DictionaryValue* value) {
+    const content::SerializedProfilerData& profiler_data) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   RequestContext* request = RequestContext::GetRequestContext(sequence_number);
-  if (!request) {
-    delete value;
+  if (!request)
     return;
-  }
 
-  if (value && request->callback_object_) {
-    // Transfers ownership of |value| to |callback_object_|.
-    request->callback_object_->ReceivedData(value);
-  } else {
-    delete value;
-  }
+  if (request->callback_object_)
+    request->callback_object_->ReceivedProfilerData(profiler_data);
 
   // Delete request if we have heard back from all child processes.
   request->DecrementProcessesPending();
@@ -289,15 +291,4 @@ int TrackingSynchronizer::GetNextAvailableSequenceNumber() {
   return last_used_sequence_number_;
 }
 
-// static
-TrackingSynchronizer* TrackingSynchronizer::CurrentSynchronizer() {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  DCHECK(tracking_synchronizer_ != NULL);
-  return tracking_synchronizer_;
-}
-
-// static
-base::LazyInstance<RequestContext::RequestContextMap>
-    RequestContext::outstanding_requests_ = LAZY_INSTANCE_INITIALIZER;
-
 }  // namespace chrome_browser_metrics