Fix a use-after-free error in WaitOrCountBookmarks

chrome/browser/profiles/profile_statistics_unittest.cc

Index: chrome/browser/profiles/profile_statistics_unittest.cc
diff --git a/chrome/browser/profiles/profile_statistics_unittest.cc b/chrome/browser/profiles/profile_statistics_unittest.cc
index 9fa316ff224869222a151bfdff1a395e050241d6..947ae95c10448f2ee48966c390dc2f6b09a7e4c9 100644
--- a/chrome/browser/profiles/profile_statistics_unittest.cc
+++ b/chrome/browser/profiles/profile_statistics_unittest.cc
@@ -8,15 +8,51 @@
 #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) {
@@ -86,3 +122,28 @@ TEST_F(ProfileStatisticsTest, ProfileAttributesStorage) {
     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);
+}