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

base/tracked_objects_unittest.cc

Index: base/tracked_objects_unittest.cc
diff --git a/base/tracked_objects_unittest.cc b/base/tracked_objects_unittest.cc
index 837a449ccaf06427317f38ed014266793f948872..6014b00f7c01b2a8287104b5f68b1650a9a9d3b8 100644
--- a/base/tracked_objects_unittest.cc
+++ b/base/tracked_objects_unittest.cc
@@ -6,11 +6,18 @@
 
 #include "base/tracked_objects.h"
 
-#include "base/json/json_writer.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/time.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+namespace {
+
+std::string GetProcessIdString() {
+  return base::IntToString(base::GetCurrentProcId());

[jar (doing other things), 2012/04/04 17:55:35]

nit: I'm not sure which is better.... but you could put this in as a slot in the
TrackedObjectTest class, that is initialized during the constructor.

I can't make up my mind... so leaving it as is should be fine... but I thought
I'd mention the alternative.

+}
+
+}  // anonymous namespace
+
 namespace tracked_objects {
 
 class TrackedObjectsTest : public testing::Test {
@@ -170,7 +177,11 @@ TEST_F(TrackedObjectsTest, ParentChildTest) {
   EXPECT_EQ(parent_child_set.begin()->first,
             parent_child_set.begin()->second);
 
-  scoped_ptr<base::Value> value(ThreadData::ToValue(false));
+  SerializedProcessData process_data;
+  ThreadData::ToSerializedProcessData(false, &process_data);
+  scoped_ptr<base::DictionaryValue> value(new base::DictionaryValue);

[jar (doing other things), 2012/04/04 17:55:35]

I think the scoped pointer was only useful because we used to get a return of a
pointer.  Now that you're creating the value locally, you can just use a local
instance (instead of a scoped pointer).

[Ilya Sherman, 2012/04/05 02:51:04]

This test code is actually completely out of sync with the rest of the CL,
because I didn't want to spend time updating it if we decided to go in a
different direction.  There is no longer any way to generate the DictionaryValue
serialization within base/ -- I'll need to fix up the tests, which I'll do in a
follow-up patch set.  (Note: patch set, not CL :)

+  process_data.ToValue(value.get());
+
   std::string json;
   base::JSONWriter::Write(value.get(), &json);
   std::string birth_only_result = "{"
@@ -245,7 +256,8 @@ TEST_F(TrackedObjectsTest, DeathDataTest) {
   EXPECT_EQ(data->queue_duration_sample(), queue_ms);
   EXPECT_EQ(data->count(), 2);
 
-  scoped_ptr<base::DictionaryValue> dictionary(data->ToValue());
+  scoped_ptr<base::DictionaryValue> dictionary(new base::DictionaryValue);

[jar (doing other things), 2012/04/04 17:55:35]

Suggest using just an instance (not scoped pointer).  Here and below.

+  SerializedDeathData(*data).ToValue(dictionary.get());
   int integer;
   EXPECT_TRUE(dictionary->GetInteger("run_ms", &integer));
   EXPECT_EQ(integer, 2 * run_ms);
@@ -258,7 +270,8 @@ TEST_F(TrackedObjectsTest, DeathDataTest) {
   EXPECT_TRUE(dictionary->GetInteger("count", &integer));
   EXPECT_EQ(integer, 2);
 
-  scoped_ptr<base::Value> value(data->ToValue());
+  scoped_ptr<base::DictionaryValue> value(new base::DictionaryValue);
+  SerializedDeathData(*data).ToValue(value.get());
   std::string json;
   base::JSONWriter::Write(value.get(), &json);
   std::string birth_only_result = "{"
@@ -287,16 +300,21 @@ TEST_F(TrackedObjectsTest, DeactivatedBirthOnlyToValueWorkerThread) {
   // We should now see a NULL birth record.
   EXPECT_EQ(birth, reinterpret_cast<Births*>(NULL));
 
-  scoped_ptr<base::Value> value(ThreadData::ToValue(false));
+  SerializedProcessData process_data;
+  ThreadData::ToSerializedProcessData(false, &process_data);
+  scoped_ptr<base::DictionaryValue> value(new base::DictionaryValue);
+  process_data.ToValue(value.get());
+
   std::string json;
   base::JSONWriter::Write(value.get(), &json);
   std::string birth_only_result = "{"
       "\"descendants\":["
       "],"
       "\"list\":["
-      "]"
+      "],"
+      "\"process_id\":" + GetProcessIdString() +
     "}";
-  EXPECT_EQ(json, birth_only_result);
+  EXPECT_EQ(birth_only_result, json);
 }
 
 TEST_F(TrackedObjectsTest, DeactivatedBirthOnlyToValueMainThread) {
@@ -315,16 +333,21 @@ TEST_F(TrackedObjectsTest, DeactivatedBirthOnlyToValueMainThread) {
   // We expect to not get a birth record.
   EXPECT_EQ(birth, reinterpret_cast<Births*>(NULL));
 
-  scoped_ptr<base::Value> value(ThreadData::ToValue(false));
+  SerializedProcessData process_data;
+  ThreadData::ToSerializedProcessData(false, &process_data);
+  scoped_ptr<base::DictionaryValue> value(new base::DictionaryValue);
+  process_data.ToValue(value.get());
+
   std::string json;
   base::JSONWriter::Write(value.get(), &json);
   std::string birth_only_result = "{"
       "\"descendants\":["
       "],"
       "\"list\":["
-      "]"
+      "],"
+      "\"process_id\":" + GetProcessIdString() +
     "}";
-  EXPECT_EQ(json, birth_only_result);
+  EXPECT_EQ(birth_only_result, json);
 }
 
 TEST_F(TrackedObjectsTest, BirthOnlyToValueWorkerThread) {
@@ -340,7 +363,11 @@ TEST_F(TrackedObjectsTest, BirthOnlyToValueWorkerThread) {
   Births* birth = ThreadData::TallyABirthIfActive(location);
   EXPECT_NE(birth, reinterpret_cast<Births*>(NULL));
 
-  scoped_ptr<base::Value> value(ThreadData::ToValue(false));
+  SerializedProcessData process_data;
+  ThreadData::ToSerializedProcessData(false, &process_data);
+  scoped_ptr<base::DictionaryValue> value(new base::DictionaryValue);
+  process_data.ToValue(value.get());
+
   std::string json;
   base::JSONWriter::Write(value.get(), &json);
   std::string birth_only_result = "{"
@@ -365,9 +392,10 @@ TEST_F(TrackedObjectsTest, BirthOnlyToValueWorkerThread) {
             "\"line_number\":173"
           "}"
         "}"
-      "]"
+      "],"
+      "\"process_id\":" + GetProcessIdString() +
     "}";
-  EXPECT_EQ(json, birth_only_result);
+  EXPECT_EQ(birth_only_result, json);
 }
 
 TEST_F(TrackedObjectsTest, BirthOnlyToValueMainThread) {
@@ -385,7 +413,11 @@ TEST_F(TrackedObjectsTest, BirthOnlyToValueMainThread) {
   Births* birth = ThreadData::TallyABirthIfActive(location);
   EXPECT_NE(birth, reinterpret_cast<Births*>(NULL));
 
-  scoped_ptr<base::Value> value(ThreadData::ToValue(false));
+  SerializedProcessData process_data;
+  ThreadData::ToSerializedProcessData(false, &process_data);
+  scoped_ptr<base::DictionaryValue> value(new base::DictionaryValue);
+  process_data.ToValue(value.get());
+
   std::string json;
   base::JSONWriter::Write(value.get(), &json);
   std::string birth_only_result = "{"
@@ -410,9 +442,10 @@ TEST_F(TrackedObjectsTest, BirthOnlyToValueMainThread) {
             "\"line_number\":173"
           "}"
         "}"
-      "]"
+      "],"
+      "\"process_id\":" + GetProcessIdString() +
     "}";
-  EXPECT_EQ(json, birth_only_result);
+  EXPECT_EQ(birth_only_result, json);
 }
 
 TEST_F(TrackedObjectsTest, LifeCycleToValueMainThread) {
@@ -443,7 +476,11 @@ TEST_F(TrackedObjectsTest, LifeCycleToValueMainThread) {
   ThreadData::TallyRunOnNamedThreadIfTracking(pending_task,
       kStartOfRun, kEndOfRun);
 
-  scoped_ptr<base::Value> value(ThreadData::ToValue(false));
+  SerializedProcessData process_data;
+  ThreadData::ToSerializedProcessData(false, &process_data);
+  scoped_ptr<base::DictionaryValue> value(new base::DictionaryValue);
+  process_data.ToValue(value.get());
+
   std::string json;
   base::JSONWriter::Write(value.get(), &json);
   std::string one_line_result = "{"
@@ -468,7 +505,8 @@ TEST_F(TrackedObjectsTest, LifeCycleToValueMainThread) {
             "\"line_number\":236"
           "}"
         "}"
-      "]"
+      "],"
+      "\"process_id\":" + GetProcessIdString() +
     "}";
   EXPECT_EQ(one_line_result, json);
 }
@@ -509,7 +547,11 @@ TEST_F(TrackedObjectsTest, LifeCycleMidDeactivatedToValueMainThread) {
   ThreadData::TallyRunOnNamedThreadIfTracking(pending_task,
       kStartOfRun, kEndOfRun);
 
-  scoped_ptr<base::Value> value(ThreadData::ToValue(false));
+  SerializedProcessData process_data;
+  ThreadData::ToSerializedProcessData(false, &process_data);
+  scoped_ptr<base::DictionaryValue> value(new base::DictionaryValue);
+  process_data.ToValue(value.get());
+
   std::string json;
   base::JSONWriter::Write(value.get(), &json);
   std::string one_line_result = "{"
@@ -534,7 +576,8 @@ TEST_F(TrackedObjectsTest, LifeCycleMidDeactivatedToValueMainThread) {
             "\"line_number\":236"
           "}"
         "}"
-      "]"
+      "],"
+      "\"process_id\":" + GetProcessIdString() +
     "}";
   EXPECT_EQ(one_line_result, json);
 }
@@ -568,14 +611,19 @@ TEST_F(TrackedObjectsTest, LifeCyclePreDeactivatedToValueMainThread) {
   ThreadData::TallyRunOnNamedThreadIfTracking(pending_task,
       kStartOfRun, kEndOfRun);
 
-  scoped_ptr<base::Value> value(ThreadData::ToValue(false));
+  SerializedProcessData process_data;
+  ThreadData::ToSerializedProcessData(false, &process_data);
+  scoped_ptr<base::DictionaryValue> value(new base::DictionaryValue);
+  process_data.ToValue(value.get());
+
   std::string json;
   base::JSONWriter::Write(value.get(), &json);
   std::string one_line_result = "{"
       "\"descendants\":["
       "],"
       "\"list\":["
-      "]"
+      "],"
+      "\"process_id\":" + GetProcessIdString() +
     "}";
   EXPECT_EQ(one_line_result, json);
 }
@@ -604,7 +652,11 @@ TEST_F(TrackedObjectsTest, LifeCycleToValueWorkerThread) {
       kStartOfRun, kEndOfRun);
 
   // Call for the ToValue, but tell it to not the maxes after scanning.
-  scoped_ptr<base::Value> value(ThreadData::ToValue(false));
+  SerializedProcessData process_data;
+  ThreadData::ToSerializedProcessData(false, &process_data);
+  scoped_ptr<base::DictionaryValue> value(new base::DictionaryValue);
+  process_data.ToValue(value.get());
+
   std::string json;
   base::JSONWriter::Write(value.get(), &json);
   std::string one_line_result = "{"
@@ -629,22 +681,31 @@ TEST_F(TrackedObjectsTest, LifeCycleToValueWorkerThread) {
             "\"line_number\":236"
           "}"
         "}"
-      "]"
+      "],"
+      "\"process_id\":" + GetProcessIdString() +
     "}";
   EXPECT_EQ(one_line_result, json);
 
   // Call for the ToValue, but tell it to reset the maxes after scanning.
   // We'll still get the same values, but the data will be reset (which we'll
   // see in a moment).
-  value.reset(ThreadData::ToValue(true));
-  base::JSONWriter::Write(value.get(), &json);
+  SerializedProcessData process_data_pre_reset;
+  ThreadData::ToSerializedProcessData(true, &process_data_pre_reset);
+  value.reset(new base::DictionaryValue);
+  process_data_pre_reset.ToValue(value.get());
+
+  base::JSONWriter::Write(value.get(), false, &json);
   // Result should be unchanged.
   EXPECT_EQ(one_line_result, json);
 
   // Call for the ToValue, and now we'll see the result of the last translation,
   // as the max will have been pushed back to zero.
-  value.reset(ThreadData::ToValue(false));
-  base::JSONWriter::Write(value.get(), &json);
+  SerializedProcessData process_data_post_reset;
+  ThreadData::ToSerializedProcessData(true, &process_data_post_reset);
+  value.reset(new base::DictionaryValue);
+  process_data_post_reset.ToValue(value.get());
+
+  base::JSONWriter::Write(value.get(), false, &json);
   std::string one_line_result_with_zeros = "{"
       "\"descendants\":["
       "],"
@@ -667,7 +728,8 @@ TEST_F(TrackedObjectsTest, LifeCycleToValueWorkerThread) {
             "\"line_number\":236"
           "}"
         "}"
-      "]"
+      "],"
+      "\"process_id\":" + GetProcessIdString() +
     "}";
   EXPECT_EQ(one_line_result_with_zeros, json);
 }
@@ -708,7 +770,11 @@ TEST_F(TrackedObjectsTest, TwoLives) {
   ThreadData::TallyRunOnNamedThreadIfTracking(pending_task2,
       kStartOfRun, kEndOfRun);
 
-  scoped_ptr<base::Value> value(ThreadData::ToValue(false));
+  SerializedProcessData process_data;
+  ThreadData::ToSerializedProcessData(false, &process_data);
+  scoped_ptr<base::DictionaryValue> value(new base::DictionaryValue);
+  process_data.ToValue(value.get());
+
   std::string json;
   base::JSONWriter::Write(value.get(), &json);
   std::string one_line_result = "{"
@@ -733,7 +799,8 @@ TEST_F(TrackedObjectsTest, TwoLives) {
             "\"line_number\":222"
           "}"
         "}"
-      "]"
+      "],"
+      "\"process_id\":" + GetProcessIdString() +
     "}";
   EXPECT_EQ(one_line_result, json);
 }
@@ -770,7 +837,11 @@ TEST_F(TrackedObjectsTest, DifferentLives) {
   base::TrackingInfo pending_task2(second_location, kDelayedStartTime);
   pending_task2.time_posted = kTimePosted;  // Overwrite implied Now().
 
-  scoped_ptr<base::Value> value(ThreadData::ToValue(false));
+  SerializedProcessData process_data;
+  ThreadData::ToSerializedProcessData(false, &process_data);
+  scoped_ptr<base::DictionaryValue> value(new base::DictionaryValue);
+  process_data.ToValue(value.get());
+
   std::string json;
   base::JSONWriter::Write(value.get(), &json);
   std::string one_line_result = "{"
@@ -813,7 +884,8 @@ TEST_F(TrackedObjectsTest, DifferentLives) {
             "\"line_number\":999"
           "}"
         "}"
-      "]"
+      "],"
+      "\"process_id\":" + GetProcessIdString() +
     "}";
   EXPECT_EQ(one_line_result, json);
 }