Measuring data use of different ModelTypes in Sync Service.

sync/engine/syncer_unittest.cc

 // Copyright 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 //
 // Syncer unit tests. Unfortunately a lot of these tests
 // are outdated and need to be reworked and updated.
 
 #include <algorithm>
 #include <limits>
 #include <list>
 #include <map>
 #include <set>
 #include <string>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback.h"
 #include "base/compiler_specific.h"
 #include "base/location.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop/message_loop.h"
 #include "base/strings/string_number_conversions.h"
+#include "base/test/histogram_tester.h"
 #include "base/time/time.h"
 #include "build/build_config.h"
 #include "sync/engine/backoff_delay_provider.h"
 #include "sync/engine/get_commit_ids.h"
 #include "sync/engine/net/server_connection_manager.h"
 #include "sync/engine/sync_scheduler_impl.h"
 #include "sync/engine/syncer.h"
 #include "sync/engine/syncer_proto_util.h"
 #include "sync/internal_api/public/base/cancelation_signal.h"
 #include "sync/internal_api/public/base/model_type.h"
@@ skipping 626 matching lines @@
   EXPECT_TRUE(SyncShareNudge());
   {
     // It should have been committed.
     syncable::ReadTransaction rtrans(FROM_HERE, directory());
     Entry entryA(&rtrans, syncable::GET_BY_ID, ids_.FromNumber(1));
     ASSERT_TRUE(entryA.good());
     EXPECT_FALSE(entryA.GetIsUnsynced());
   }
 }
 
+// This test has three steps. In the first step, a BOOKMARK update is received.
+// In the next step, syncing BOOKMARKS is disabled, so no BOOKMARK is sent or
+// received. In the last step, a BOOKMARK update is committed.
+TEST_F(SyncerTest, DataUseHistogramsTest) {
+  base::HistogramTester histogram_tester;
+  sync_pb::EntitySpecifics bookmark_data;
+  AddDefaultFieldValue(BOOKMARKS, &bookmark_data);
+
+  mock_server_->AddUpdateDirectory(1, 0, "A", 10, 10,
+                                   foreign_cache_guid(), "-1");
+  int download_bytes_bookmark = 0;
+  vector<int> progress_bookmark = {0, 0, 0};

[Nicolas Zea, 2015/08/19 22:01:51]

nit: std:: here and below

[amohammadkhan, 2015/08/20 00:20:13]

Done.

+  vector<int> progress_all = {0, 0, 0};

[sclittle, 2015/08/19 22:41:35]

Instead of using these progress vectors and comparing them, could you just use a
different histogram_tester for each of the blocks below? i.e. maybe scope the
histogram_tester within those blocks. You might be able to cut out the for loops
in each of the blocks if there would only be 1 sample then.

[amohammadkhan, 2015/08/20 00:20:13]

By comparing these values I am showing the progress and we will need the for
loops anyways. (Because of the progress makers of other types) If you see any
other drawbacks besides the loops please let me know.

+  std::vector<base::Bucket> samples;
+  EXPECT_TRUE(SyncShareNudge());
+  {
+    histogram_tester.ExpectTotalCount("DataUse.Sync.Upload.Count", 0);
+    histogram_tester.ExpectTotalCount("DataUse.Sync.Upload.Bytes", 0);
+    histogram_tester.ExpectTotalCount("DataUse.Sync.Download.Count", 1);
+    histogram_tester.ExpectUniqueSample("DataUse.Sync.Download.Count",
+                                        BOOKMARKS, 1);
+    samples = histogram_tester.GetAllSamples("DataUse.Sync.Download.Bytes");
+    EXPECT_TRUE(samples.size() == 1);

[Nicolas Zea, 2015/08/19 22:01:51]

nit: use EXPECT_EQ, EXPECT_GE, EXPECT_GT where possible here and below

[amohammadkhan, 2015/08/20 00:20:13]

Done.

+    EXPECT_TRUE(samples.at(0).min == BOOKMARKS);
+    EXPECT_TRUE(samples.at(0).count >= 0);
+    download_bytes_bookmark = samples.at(0).count;
+
+    samples = histogram_tester.GetAllSamples(
+        "DataUse.Sync.ProgressMarker.Bytes");
+
+    for(std::vector<base::Bucket>::iterator it = samples.begin();

[sclittle, 2015/08/19 22:41:35]

nit: use range-based for, e.g. for (const base::Bucket& bucket : samples)

[amohammadkhan, 2015/08/20 00:20:13]

Done.

+        it != samples.end(); ++it) {
+      if (it->min == BOOKMARKS)
+        progress_bookmark.at(0) += it->count;

[Nicolas Zea, 2015/08/19 22:01:51]

there should only be one bucket with value BOOKMARKS right? I think it's cleaner
to do an assignment then, rather than += (here and below)

[amohammadkhan, 2015/08/20 00:20:13]

Yes it should be one here. But I was thinking maybe it is better to count it if
it is more than one for some reason. But in below, it could be more than one.

+      progress_all.at(0) += it->count;
+    }
+    EXPECT_TRUE(progress_bookmark.at(0) > 0);
+    EXPECT_TRUE(progress_all.at(0) > 0);

[Nicolas Zea, 2015/08/19 22:01:51]

should progress_all == progress_bookmark? Maybe that's a better condition (here
and below)

[amohammadkhan, 2015/08/20 00:20:13]

They are not equal because of having progress markers for other types.
progress_all is larger than progress_bookmark.

+
+    WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
+    MutableEntry A(&wtrans, GET_BY_ID, ids_.FromNumber(1));
+    A.PutIsUnsynced(true);
+    A.PutSpecifics(bookmark_data);
+    A.PutNonUniqueName("bookmark");
+  }
+
+  // Now sync without enabling bookmarks.
+  mock_server_->ExpectGetUpdatesRequestTypes(
+      Difference(context_->GetEnabledTypes(), ModelTypeSet(BOOKMARKS)));
+  ResetSession();
+  syncer_->NormalSyncShare(
+      Difference(context_->GetEnabledTypes(), ModelTypeSet(BOOKMARKS)),
+      &nudge_tracker_, session_.get());
+
+  {
+    // Nothing should have been committed as bookmarks is throttled.
+    histogram_tester.ExpectTotalCount("DataUse.Sync.Upload.Count", 0);
+    histogram_tester.ExpectTotalCount("DataUse.Sync.Upload.Bytes", 0);
+    histogram_tester.ExpectTotalCount("DataUse.Sync.Download.Count", 1);
+    histogram_tester.ExpectUniqueSample("DataUse.Sync.Download.Count",
+                                        BOOKMARKS, 1);
+
+    samples = histogram_tester.GetAllSamples("DataUse.Sync.Download.Bytes");
+    EXPECT_TRUE(samples.size() == 1);
+    EXPECT_TRUE(samples.at(0).min == BOOKMARKS);
+    EXPECT_TRUE(samples.at(0).count == download_bytes_bookmark);
+
+    samples = histogram_tester.GetAllSamples(
+        "DataUse.Sync.ProgressMarker.Bytes");
+    for(std::vector<base::Bucket>::iterator it = samples.begin();
+        it != samples.end(); ++it) {
+      if (it->min == BOOKMARKS)
+        progress_bookmark.at(1) += it->count;
+      progress_all.at(1) += it->count;
+    }
+    EXPECT_TRUE(progress_bookmark.at(1) == progress_bookmark.at(0));
+    EXPECT_TRUE(progress_all.at(1) > progress_all.at(0));
+  }
+
+  // Sync again with bookmarks enabled.
+  mock_server_->ExpectGetUpdatesRequestTypes(context_->GetEnabledTypes());
+  EXPECT_TRUE(SyncShareNudge());
+  {
+    // It should have been committed.
+    histogram_tester.ExpectTotalCount("DataUse.Sync.Upload.Count", 1);
+    histogram_tester.ExpectUniqueSample("DataUse.Sync.Upload.Count",
+                                        BOOKMARKS,1);
+    samples = histogram_tester.GetAllSamples("DataUse.Sync.Upload.Bytes");
+    EXPECT_TRUE(samples.size() == 1);
+    EXPECT_TRUE(samples.at(0).min == BOOKMARKS);
+    EXPECT_TRUE(samples.at(0).count >= 0);
+
+    samples = histogram_tester.GetAllSamples("DataUse.Sync.Download.Bytes");
+    EXPECT_TRUE(samples.size() == 1);
+    EXPECT_TRUE(samples.at(0).min == BOOKMARKS);
+    EXPECT_TRUE(samples.at(0).count == download_bytes_bookmark);
+
+    histogram_tester.ExpectTotalCount("DataUse.Sync.Download.Count", 1);
+
+    samples = histogram_tester.GetAllSamples(
+        "DataUse.Sync.ProgressMarker.Bytes");
+    for(std::vector<base::Bucket>::iterator it = samples.begin();
+        it != samples.end(); ++it) {
+      if (it->min == BOOKMARKS)
+        progress_bookmark.at(2) += it->count;
+      progress_all.at(2) += it->count;
+    }
+    EXPECT_TRUE(progress_bookmark.at(2) > progress_bookmark.at(1));
+    EXPECT_TRUE(progress_all.at(2) > progress_all.at(1));
+  }
+}
+
 // We use a macro so we can preserve the error location.
 #define VERIFY_ENTRY(id, is_unapplied, is_unsynced, prev_initialized, \
                      parent_id, version, server_version, id_fac, rtrans) \
   do { \
     Entry entryA(rtrans, syncable::GET_BY_ID, id_fac.FromNumber(id)); \
     ASSERT_TRUE(entryA.good()); \
     /* We don't use EXPECT_EQ here because when the left side param is false,
     gcc 4.6 warns about converting 'false' to pointer type for argument 1. */ \
     EXPECT_TRUE(is_unsynced == entryA.GetIsUnsynced()); \
     EXPECT_TRUE(is_unapplied == entryA.GetIsUnappliedUpdate()); \
@@ skipping 4808 matching lines @@
     EXPECT_EQ("xyz", final_monitor_records["xyz"].extension_id);
     EXPECT_EQ(2049U, final_monitor_records["ABC"].bookmark_write_count);
     EXPECT_EQ(4U,    final_monitor_records["xyz"].bookmark_write_count);
   } else {
     EXPECT_TRUE(final_monitor_records.empty())
         << "Should not restore records after successful bookmark commit.";
   }
 }
 
 }  // namespace syncer