Sync performance tests: Cleanup TODOs and print in dashboard friendly format

chrome/test/live_sync/performance/autofill_sync_perf_test.cc

Index: chrome/test/live_sync/performance/autofill_sync_perf_test.cc
diff --git a/chrome/test/live_sync/performance/autofill_sync_perf_test.cc b/chrome/test/live_sync/performance/autofill_sync_perf_test.cc
index a7a18c602ba3c796ddecc0a09cade2cd5604c5d4..5348cfddd081b3b3584fbdd9de39e9347cfc131a 100644
--- a/chrome/test/live_sync/performance/autofill_sync_perf_test.cc
+++ b/chrome/test/live_sync/performance/autofill_sync_perf_test.cc
@@ -9,15 +9,11 @@
 #include "chrome/test/live_sync/performance/sync_timing_helper.h"
 
 static const int kNumProfiles = 150;
-
-// TODO(braffert): Consider the range / resolution of these test points.

[Raghu Simha, 2011/07/26 06:19:45]

Perhaps replace this with a TODO for moving kBenchmarkPoints and
kNumBenchmarkPoints into a performance test base class once it's possible to
create one?

[braffert, 2011/07/26 17:46:59]

Done.

 static const int kNumBenchmarkPoints = 18;
 static const int kBenchmarkPoints[] = {1, 10, 20, 30, 40, 50, 75, 100, 125,
                                        150, 175, 200, 225, 250, 300, 350, 400,
                                        500};
 
-// TODO(braffert): Move this class into its own .h/.cc files.  What should the
-// class files be named as opposed to the file containing the tests themselves?

[Raghu Simha, 2011/07/26 06:19:45]

Curious: You're deleting the TODO to move the class into its own .h/.cc file,
but the class hasn't actually moved. Do you still intend to do so? (I'm OK with
them being left as they are, instead of tripling the number of files)

[braffert, 2011/07/26 17:46:59]

That's what I was thinking as well - currently there would be no benefit from
separating each datatype into three files.  Though it may be a good idea to move
all the datatype specific convenience methods into a single performance helper
class, but this is best left for a later time.  

 class AutofillSyncPerfTest
     : public TwoClientLiveAutofillSyncTest {
  public:
@@ -127,8 +123,7 @@ IN_PROC_BROWSER_TEST_F(AutofillSyncPerfTest, Add) {
   ASSERT_EQ(kNumProfiles, GetProfileCount(0));
   ASSERT_TRUE(AllProfilesMatch());
 
-  // TODO(braffert): Compare timings against some target value.

[Raghu Simha, 2011/07/26 06:19:45]

Don't you still have to compare the timings? I don't see PrintResult doing so.

[braffert, 2011/07/26 17:46:59]

The tracking and comparison is done automatically by the perf dashboard.  This
just prints the timings in a format that matches the dashboard's expectations.

-  VLOG(0) << std::endl << "dt: " << dt.InSecondsF() << " s";
+  SyncTimingHelper::PrintResult("autofill", "add", dt);
 }
 
 // TCM ID - 7549835.
@@ -144,8 +139,7 @@ IN_PROC_BROWSER_TEST_F(AutofillSyncPerfTest, Update) {
   ASSERT_EQ(kNumProfiles, GetProfileCount(0));
   ASSERT_TRUE(AllProfilesMatch());
 
-  // TODO(braffert): Compare timings against some target value.
-  VLOG(0) << std::endl << "dt: " << dt.InSecondsF() << " s";
+  SyncTimingHelper::PrintResult("autofill", "update", dt);
 }
 
 // TCM ID - 7553678.
@@ -161,8 +155,7 @@ IN_PROC_BROWSER_TEST_F(AutofillSyncPerfTest, Delete) {
   ASSERT_EQ(0, GetProfileCount(0));
   ASSERT_TRUE(AllProfilesMatch());
 
-  // TODO(braffert): Compare timings against some target value.
-  VLOG(0) << std::endl << "dt: " << dt.InSecondsF() << " s";
+  SyncTimingHelper::PrintResult("autofill", "delete", dt);
 }
 
 IN_PROC_BROWSER_TEST_F(AutofillSyncPerfTest, DISABLED_Benchmark) {