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

chrome/browser/metrics/tracking_synchronizer.h

Index: chrome/browser/metrics/tracking_synchronizer.h
diff --git a/chrome/browser/metrics/tracking_synchronizer.h b/chrome/browser/metrics/tracking_synchronizer.h
index 75fe3c9d29e8f66ede7cb1f3e642eb37e56449d9..cb16a727ddf736582b91d18134266be73e8d0156 100644
--- a/chrome/browser/metrics/tracking_synchronizer.h
+++ b/chrome/browser/metrics/tracking_synchronizer.h
@@ -14,8 +14,6 @@
 #include "base/lazy_instance.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
-#include "base/values.h"
-#include "chrome/browser/ui/webui/profiler_ui.h"
 #include "content/public/browser/profiler_subscriber.h"
 
 // This class maintains state that is used to upload profiler data from the
@@ -35,7 +33,7 @@
 
 namespace chrome_browser_metrics {
 
-class RequestContext;
+class TrackingSynchronizerObserver;
 
 class TrackingSynchronizer
     : public content::ProfilerSubscriber,
@@ -52,7 +50,7 @@ class TrackingSynchronizer
   // the data received from each sub-process.
   // This method is accessible on UI thread.
   static void FetchProfilerDataAsynchronously(
-      const base::WeakPtr<ProfilerUI>& callback_object);
+      const base::WeakPtr<TrackingSynchronizerObserver>& callback_object);
 
   // ------------------------------------------------------
   // ProfilerSubscriber methods for browser child processes
@@ -64,32 +62,28 @@ class TrackingSynchronizer
                                   int pending_processes,
                                   bool end) OVERRIDE;
 
-  // Send profiler_data back to callback_object_ by calling
-  // DecrementPendingProcessesAndSendData which records that we are waiting
-  // for one less profiler data from renderer or browser child process for the
-  // given sequence number. This method is accessible on UI thread.
-  virtual void OnProfilerDataCollected(
-      int sequence_number,
-      base::DictionaryValue* profiler_data) OVERRIDE;
-
  private:
   friend class base::RefCountedThreadSafe<TrackingSynchronizer>;
-  friend class RequestContext;
+
+  class RequestContext;
 
   virtual ~TrackingSynchronizer();
 
-  // Send profiler_data back to callback_object_. It records that we are waiting
+  // Send profiler_data back to callback_object_ by calling
+  // DecrementPendingProcessesAndSendData which records that we are waiting
   // for one less profiler data from renderer or browser child process for the
   // given sequence number. This method is accessible on UI thread.
-  void OnProfilerDataCollectedOnUI(int sequence_number,
-                                   base::DictionaryValue* profiler_data);
+  virtual void OnProfilerDataCollected(
+      int sequence_number,
+      const tracked_objects::SerializedProcessData& profiler_data,
+      content::ProcessType process_type) OVERRIDE;
 
   // Establish a new sequence_number_, and use it to notify all the processes of
   // the need to supply, to the browser, their tracking data. It also registers
   // |callback_object| in |outstanding_requests_| map. Return the
   // sequence_number_ that was used. This method is accessible on UI thread.
   int RegisterAndNotifyAllProcesses(
-      const base::WeakPtr<ProfilerUI>& callback_object);
+      const base::WeakPtr<TrackingSynchronizerObserver>& callback_object);
 
   // It finds the RequestContext for the given |sequence_number| and notifies
   // the RequestContext's |callback_object_| about the |value|. This is called
@@ -98,18 +92,15 @@ class TrackingSynchronizer
   // sequence number. If we have received a response from all renderers and
   // browser processes, then it calls RequestContext's DeleteIfAllDone to delete
   // the entry for sequence_number. This method is accessible on UI thread.
-  void DecrementPendingProcessesAndSendData(int sequence_number,
-                                            base::DictionaryValue* value);
+  void DecrementPendingProcessesAndSendData(
+      int sequence_number,
+      const tracked_objects::SerializedProcessData& profiler_data,
+      content::ProcessType process_type);

[jar (doing other things), 2012/03/21 18:19:55]

nit/comment/suggest/question: Shouldn't process_type be embedded in the profiler
data?

[Ilya Sherman, 2012/03/21 19:23:02]

I also wanted to write it that way, but our visibility rules don't allow it: the
ProcessType enum is defined under content/ whereas the profiler code lives in
base/.  We could do something hacky like casting the enum to an int and back,
but that really seems like we'd just be unwisely circumventing the visibility
rules.

 
   // Get a new sequence number to be sent to processes from browser process.
   // This method is accessible on UI thread.
   int GetNextAvailableSequenceNumber();
 
-  // Return pointer to the singleton instance, which is allocated and
-  // deallocated on the main UI thread (during system startup and teardown).
-  // This method is accessible on UI thread.
-  static TrackingSynchronizer* CurrentSynchronizer();
-
   // We don't track the actual processes that are contacted for an update, only
   // the count of the number of processes, and we can sometimes time-out and
   // give up on a "slow to respond" process.  We use a sequence_number to be
@@ -119,13 +110,6 @@ class TrackingSynchronizer
   // reuse for a long time).
   int last_used_sequence_number_;
 
-  // 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 TrackingSynchronizer* tracking_synchronizer_;
-
   DISALLOW_COPY_AND_ASSIGN(TrackingSynchronizer);
 };