Make profile statistics tasks inspectable by tests

chrome/browser/profiles/profile_statistics_browsertest.cc

Index: chrome/browser/profiles/profile_statistics_browsertest.cc
diff --git a/chrome/browser/profiles/profile_statistics_browsertest.cc b/chrome/browser/profiles/profile_statistics_browsertest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..da4f1479f885d5e254c2e40e794eacf49832cf08
--- /dev/null
+++ b/chrome/browser/profiles/profile_statistics_browsertest.cc
@@ -0,0 +1,279 @@
+// Copyright (c) 2016 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.
+
+#include <stddef.h>
+#include <stdint.h>
+
+#include <algorithm>
+#include <set>
+#include <string>
+#include <vector>
+
+#include "base/bind.h"
+#include "base/callback_forward.h"
+#include "base/macros.h"
+#include "base/strings/stringprintf.h"
+#include "build/build_config.h"
+#include "chrome/browser/password_manager/password_store_factory.h"
+#include "chrome/browser/profiles/profile_manager.h"
+#include "chrome/browser/profiles/profile_statistics.h"
+#include "chrome/browser/profiles/profile_statistics_aggregator.h"
+#include "chrome/browser/profiles/profile_statistics_constants.h"
+#include "chrome/browser/profiles/profile_statistics_factory.h"
+#include "chrome/browser/profiles/profile_statistics_types.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "components/password_manager/core/browser/password_manager_test_utils.h"
+#include "components/password_manager/core/browser/test_password_store.h"
+#include "content/public/test/test_utils.h"
+
+namespace {
+
+std::set<std::string> stats_categories() {
+  std::set<std::string> categories;
+  categories.insert(profiles::kProfileStatisticsBrowsingHistory);
+  categories.insert(profiles::kProfileStatisticsPasswords);
+  categories.insert(profiles::kProfileStatisticsBookmarks);
+  categories.insert(profiles::kProfileStatisticsSettings);
+  EXPECT_EQ(4u, categories.size());
+  return categories;
+}
+
+bool IsProfileCategoryStatEqual(const profiles::ProfileCategoryStat& a,
+                                const profiles::ProfileCategoryStat& b) {
+  return a.category == b.category && a.count == b.count &&
+         a.success == b.success;
+}
+
+std::string ProfileCategoryStatToString(
+    const profiles::ProfileCategoryStat& a) {
+  return base::StringPrintf("category = %s, count = %d, success = %s",
+      a.category.c_str(), a.count, a.success ? "true" : "false");
+}
+
+::testing::AssertionResult AssertionProfileCategoryStatEqual(
+    const char* actual_expression,
+    const char* expected_expression,
+    const profiles::ProfileCategoryStat& actual_value,
+    const profiles::ProfileCategoryStat& expected_value) {
+  if (IsProfileCategoryStatEqual(actual_value, expected_value)) {
+    return ::testing::AssertionSuccess();
+  } else {
+    return ::testing::AssertionFailure()
+        << "Value of: " << actual_expression
+        << "\n  Actual: " << ProfileCategoryStatToString(actual_value)
+        << "\nExpected: " << expected_expression
+        << "\nWhich is: " << ProfileCategoryStatToString(expected_value);
+  }
+}
+
+::testing::AssertionResult AssertionProfileCategoryStatsEqual(
+    const char* actual_expression,
+    const char* expected_expression,
+    const profiles::ProfileCategoryStats& actual_value,
+    const profiles::ProfileCategoryStats& expected_value) {
+  if (std::is_permutation(actual_value.cbegin(),
+                           actual_value.cend(),
+                           expected_value.cbegin(),
+                           expected_value.cend(),
+                           IsProfileCategoryStatEqual)) {
+    return ::testing::AssertionSuccess();
+  } else {
+    ::testing::AssertionResult result = testing::AssertionFailure();
+    result << "ProfileCategoryStats are not equal.";
+
+    result << "\n  Actual: " << actual_expression << "\nWhich is:";
+    for (const auto& value : actual_value)
+      result << "\n    " << ProfileCategoryStatToString(value);
+
+    result << "\nExpected: " << expected_expression << "\nWhich is:";
+    for (const auto& value : expected_value)
+      result << "\n    " << ProfileCategoryStatToString(value);
+
+    return result;
+  }
+}
+
+class ProfileStatisticsAggregatorState {
+ public:
+  explicit ProfileStatisticsAggregatorState(const base::Closure& quit_closure)
+      : ProfileStatisticsAggregatorState(quit_closure,
+                                         stats_categories().size()) {}
+
+  ProfileStatisticsAggregatorState(const base::Closure& quit_closure,
+                                   size_t required_stat_count) {
+    stats_categories_ = stats_categories();
+    num_of_stats_categories_ = stats_categories_.size();
+    SetQuitClosureAndRequiredStatCount(quit_closure, required_stat_count);
+  }
+
+  void SetQuitClosureAndRequiredStatCount(const base::Closure& quit_closure,
+                                          size_t required_stat_count) {
+    EXPECT_FALSE(quit_closure.is_null());
+    quit_closure_ = quit_closure;
+    EXPECT_GE(num_of_stats_categories_, required_stat_count);
+    required_stat_count_ = required_stat_count;
+  }
+
+  profiles::ProfileCategoryStats GetStats() const { return stats_; }
+  int GetNumOfFails() const { return num_of_fails_; }
+
+  void StatsCallback(profiles::ProfileCategoryStats stats_return) {
+    size_t newCount = stats_return.size();
+    // If newCount is 1, then a new GatherStatistics task has started. Discard
+    // the old statistics by setting oldCount to 0 in this case.
+    size_t oldCount = newCount == 1u ? 0u : stats_.size();
+
+    // Only one new statistic arrives at a time.
+    EXPECT_EQ(oldCount + 1u, newCount);
+    for (size_t i = 0u; i < oldCount; i++) {
+      // Exisiting statistics must be the same.
+      EXPECT_PRED_FORMAT2(AssertionProfileCategoryStatEqual,
+                          stats_[i], stats_return[i]);
+    }
+
+    num_of_fails_ = 0;
+    for (size_t i = 0u; i < newCount; i++) {
+      // The category must be a valid category.
+      EXPECT_EQ(1u, stats_categories_.count(stats_return[i].category));
+      // The categories in |stats_return| must all different.
+      for (size_t j = 0u; j < i; j++)
+        EXPECT_NE(stats_return[i].category, stats_return[j].category);
+      // Count the number of statistics failures.
+      if (!stats_return[i].success)
+        num_of_fails_++;
+    }
+    stats_ = stats_return;
+
+    EXPECT_GE(num_of_stats_categories_, newCount);
+    if (required_stat_count_ <= newCount)
+      quit_closure_.Run();
+  }
+
+ private:
+  std::set<std::string> stats_categories_;
+  base::Closure quit_closure_;
+  size_t num_of_stats_categories_;
+  size_t required_stat_count_;
+
+  profiles::ProfileCategoryStats stats_;
+  int num_of_fails_ = 0;
+};
+
+}  // namespace
+
+class ProfileStatisticsBrowserTest : public InProcessBrowserTest {
+ public:
+  void SetUpOnMainThread() override {
+    // Use TestPasswordStore to remove a possible race. Normally the
+    // PasswordStore does its database manipulation on the DB thread, which
+    // creates a possible race during navigation. Specifically the
+    // PasswordManager will ignore any forms in a page if the load from the
+    // PasswordStore has not completed.
+    PasswordStoreFactory::GetInstance()->SetTestingFactory(
+        browser()->profile(),
+        password_manager::BuildPasswordStore<
+            content::BrowserContext, password_manager::TestPasswordStore>);
+  }
+};
+
+using ProfileStatisticsBrowserDeathTest = ProfileStatisticsBrowserTest;
+
+IN_PROC_BROWSER_TEST_F(ProfileStatisticsBrowserTest, GatherStatistics) {
+  Profile* profile = ProfileManager::GetActiveUserProfile();
+  ASSERT_TRUE(profile);
+  ProfileStatistics* profile_stat =
+      ProfileStatisticsFactory::GetForProfile(profile);
+
+  scoped_refptr<content::MessageLoopRunner> loop =
+      new content::MessageLoopRunner;
+  ProfileStatisticsAggregatorState state(loop->QuitClosure());
+  EXPECT_FALSE(profile_stat->HasAggregator());
+  profile_stat->GatherStatistics(
+      base::Bind(&ProfileStatisticsAggregatorState::StatsCallback,
+                 base::Unretained(&state)));
+  ASSERT_TRUE(profile_stat->HasAggregator());
+  EXPECT_EQ(1u, profile_stat->GetAggregator()->GetCallbackCount());
+  loop->Run();
+
+  EXPECT_EQ(0, state.GetNumOfFails());
+
+  profiles::ProfileCategoryStats stats = state.GetStats();
+  for (const auto& stat : stats) {
+    if (stat.category != profiles::kProfileStatisticsSettings)
+      EXPECT_EQ(0, stat.count);
+  }
+
+  EXPECT_PRED_FORMAT2(AssertionProfileCategoryStatsEqual, stats,
+      ProfileStatistics::GetProfileStatisticsFromCache(profile->GetPath()));
+}
+
+IN_PROC_BROWSER_TEST_F(ProfileStatisticsBrowserTest,
+                       GatherStatisticsTwoCallbacks) {
+  Profile* profile = ProfileManager::GetActiveUserProfile();
+  ASSERT_TRUE(profile);
+  ProfileStatistics* profile_stat =
+      ProfileStatisticsFactory::GetForProfile(profile);
+
+  scoped_refptr<content::MessageLoopRunner> loop1a =

[lwchkg, 2016/03/16 17:26:03]

Using content::MessageLoopRunner instead of base::RunLoop caused this test to
fail. Symptom: after loop1a->Run(), all 4 stats are returned, instead of just 1.

Avi: Do you know why does it happen?

[Avi (use Gerrit), 2016/03/16 17:39:04]

Offhand, no, I've never used so many at once.

BTW, why the scoped_refptrs rather than just making them inline local variables?

[lwchkg, 2016/03/16 18:36:41]

content::MessageLoopRunner is a RefCounted, so we need to use a smart pointer
that do refcounting to track it. It's use is documented in
https://code.google.com/p/chromium/codesearch#chromium/src/content/public/tes...

+      new content::MessageLoopRunner;
+  scoped_refptr<content::MessageLoopRunner> loop1b =
+      new content::MessageLoopRunner;
+  scoped_refptr<content::MessageLoopRunner> loop2 =
+      new content::MessageLoopRunner;
+  ProfileStatisticsAggregatorState state1(loop1a->QuitClosure(), 1u);
+  ProfileStatisticsAggregatorState state2(loop2->QuitClosure());
+
+  EXPECT_FALSE(profile_stat->HasAggregator());
+  profile_stat->GatherStatistics(
+      base::Bind(&ProfileStatisticsAggregatorState::StatsCallback,
+                 base::Unretained(&state1)));
+  ASSERT_TRUE(profile_stat->HasAggregator());
+  EXPECT_EQ(1u, profile_stat->GetAggregator()->GetCallbackCount());
+  loop1a->Run();
+
+  state1.SetQuitClosureAndRequiredStatCount(loop1b->QuitClosure(),
+                                            stats_categories().size());
+
+  profile_stat->GatherStatistics(
+      base::Bind(&ProfileStatisticsAggregatorState::StatsCallback,
+                 base::Unretained(&state2)));
+  ASSERT_TRUE(profile_stat->HasAggregator());
+  EXPECT_EQ(2u, profile_stat->GetAggregator()->GetCallbackCount());
+  loop1b->Run();
+  loop2->Run();
+
+  EXPECT_EQ(0, state1.GetNumOfFails());
+
+  EXPECT_PRED_FORMAT2(AssertionProfileCategoryStatsEqual,
+      state1.GetStats(), state2.GetStats());
+}
+
+IN_PROC_BROWSER_TEST_F(ProfileStatisticsBrowserTest, CloseBrowser) {
+  Profile* profile = ProfileManager::GetActiveUserProfile();
+  ASSERT_TRUE(profile);
+  ProfileStatistics* profile_stat =
+      ProfileStatisticsFactory::GetForProfile(profile);
+
+  EXPECT_FALSE(profile_stat->HasAggregator());
+  CloseBrowserSynchronously(browser());
+  // The statistics task should be either running or finished.
+  if (profile_stat->HasAggregator()) {
+    EXPECT_EQ(0u, profile_stat->GetAggregator()->GetCallbackCount());
+  } else {
+    // Some of the statistics (e.g. settings) do always succeed. If all the
+    // statistics "failed", it means nothing is stored in profile attributes
+    // storage, and the statistics task was not run.
+    profiles::ProfileCategoryStats stats =
+        ProfileStatistics::GetProfileStatisticsFromCache(profile->GetPath());
+    bool has_stats = false;
+    for (const profiles::ProfileCategoryStat& stat : stats) {
+      if (stat.success) {
+        has_stats = true;
+        break;
+      }
+    }
+    EXPECT_TRUE(has_stats);
+  }
+}