Fix a use-after-free error in WaitOrCountBookmarks

chrome/browser/profiles/profile_statistics_unittest.cc

 // Copyright 2015 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 <map>
 #include <set>
 #include <utility>
 #include <vector>
 
 #include "base/files/file_path.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/run_loop.h"
+#include "chrome/browser/bookmarks/bookmark_model_factory.h"
+#include "chrome/browser/bookmarks/chrome_bookmark_client.h"
+#include "chrome/browser/bookmarks/managed_bookmark_service_factory.h"
 #include "chrome/browser/profiles/profile_statistics.h"
 #include "chrome/browser/profiles/profile_statistics_common.h"
+#include "chrome/browser/profiles/profile_statistics_factory.h"
+#include "chrome/common/pref_names.h"
 #include "chrome/test/base/testing_browser_process.h"
 #include "chrome/test/base/testing_profile.h"
 #include "chrome/test/base/testing_profile_manager.h"
+#include "components/bookmarks/browser/bookmark_model.h"
+#include "components/prefs/pref_service.h"
+#include "content/public/browser/browser_thread.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace {
+
+scoped_ptr<KeyedService> BuildBookmarkModelWithoutLoad(
+    content::BrowserContext* context) {
+  Profile* profile = Profile::FromBrowserContext(context);
+  scoped_ptr<bookmarks::BookmarkModel> bookmark_model(
+      new bookmarks::BookmarkModel(make_scoped_ptr(new ChromeBookmarkClient(
+          profile, ManagedBookmarkServiceFactory::GetForProfile(profile)))));
+  return std::move(bookmark_model);
+}
+
+void LoadBookmarkModel(Profile* profile,
+                       bookmarks::BookmarkModel* bookmark_model) {
+  bookmark_model->Load(profile->GetPrefs(),
+                       profile->GetPrefs()->GetString(prefs::kAcceptLanguages),
+                       profile->GetPath(),
+                       profile->GetIOTaskRunner(),
+                       content::BrowserThread::GetMessageLoopProxyForThread(
+                           content::BrowserThread::UI));
+}
+
+bookmarks::BookmarkModel* CreateBookmarkModelWithoutLoad(Profile* profile) {
+  return static_cast<bookmarks::BookmarkModel*>(
+      BookmarkModelFactory::GetInstance()->SetTestingFactoryAndUse(
+          profile, BuildBookmarkModelWithoutLoad));
+}
+
 void VerifyStatisticsCache(const base::FilePath& profile_path,
     const std::map<std::string, int>& expected,
     const std::vector<std::string>& categories_to_check) {
   const profiles::ProfileCategoryStats actual =
       ProfileStatistics::GetProfileStatisticsFromAttributesStorage(
           profile_path);
 
   EXPECT_EQ(categories_to_check.size(), actual.size());
 
   std::set<std::string> checked;
@@ skipping 49 matching lines @@
   // Now no keys are set.
   VerifyStatisticsCache(profile_path, expected, categories_to_check);
   // Insert items and test after each insert.
   for (const auto& item : insertions) {
     ProfileStatistics::SetProfileStatisticsToAttributesStorage(
         profile_path, item.first, item.second);
     expected[item.first] = item.second;
     VerifyStatisticsCache(profile_path, expected, categories_to_check);
   }
 }
+
+TEST_F(ProfileStatisticsTest, WaitOrCountBookmarks) {
+  TestingProfile* profile = manager()->CreateTestingProfile("Test 1");
+  ASSERT_TRUE(profile);
+
+  bookmarks::BookmarkModel* bookmark_model =
+      CreateBookmarkModelWithoutLoad(profile);
+  ASSERT_TRUE(bookmark_model);
+
+  ProfileStatistics* profile_stat =
+      ProfileStatisticsFactory::GetForProfile(profile);
+
+  // Run ProfileStatisticsAggregator::WaitOrCountBookmarks twice.
+  for (int i = 0; i < 2; i++) {
+    profile_stat->GatherStatistics(profiles::ProfileStatisticsCallback());
+    // Wait until ProfileStatisticsAggregator::WaitOrCountBookmarks is run.
+    base::RunLoop run_loop;
+    run_loop.RunUntilIdle();
+  }
+
+  // Load the bookmark model to allow the bookmark counting task to finish. This

[Mike Lerman, 2016/04/04 15:38:58]

Rather than saying "this checks for a bug..." you can say "This verifies that
multiple pending GatherStatistics() calls only results in one callback for
gathering bookmarks", or similar. Then, you can verify that :)

[lwchkg, 2016/04/04 19:29:03]

Verification is hard... but done.

+  // checks for a bug in ProfileStatisticsAggregator::WaitOrCountBookmarks. See
+  // https://crbug.com/596693.
+  LoadBookmarkModel(profile, bookmark_model);
+}