[omnibox] Make a couple tests use a common mock

Short-cuts provider and history URL provider unit-tests each have an
AnonFakeAutocompleteProviderClient, which shares a little code with
the common FakeAutocompleteProviderClient. This CL has those two
classes derive from the common one, and move the common bits down.
It's a follow-up to 776643.

Bug: 780835
Change-Id: Ida2fc850df0c36a35a3a1898296e85aec0a28809
Reviewed-on: https://chromium-review.googlesource.com/782079
Commit-Queue: Kevin Bailey <krb@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520839}

components/omnibox/browser/fake_autocomplete_provider_client.cc

@@ -4,6 +4,7 @@
 
 #include "components/omnibox/browser/fake_autocomplete_provider_client.h"
 
+#include "base/files/file_path.h"
 #include "base/memory/ptr_util.h"
 #include "base/run_loop.h"
 #include "components/bookmarks/browser/bookmark_model.h"
@@ -12,42 +13,46 @@
 #include "components/history/core/test/history_service_test_util.h"
 #include "components/omnibox/browser/in_memory_url_index.h"
 #include "components/omnibox/browser/in_memory_url_index_test_util.h"
+#include "components/omnibox/browser/shortcuts_backend.h"
+#include "components/search_engines/template_url_service.h"
 
-FakeAutocompleteProviderClient::FakeAutocompleteProviderClient()
+FakeAutocompleteProviderClient::FakeAutocompleteProviderClient(
+    bool create_history_db)
     : is_tab_open_with_url_(false) {
+  set_template_url_service(std::make_unique<TemplateURLService>(nullptr, 0));
+
   bookmark_model_ = bookmarks::TestBookmarkClient::CreateModel();
 
   CHECK(history_dir_.CreateUniqueTempDir());
   history_service_ =
-      history::CreateHistoryService(history_dir_.GetPath(), true);
+      history::CreateHistoryService(history_dir_.GetPath(), create_history_db);
 
   in_memory_url_index_.reset(
       new InMemoryURLIndex(bookmark_model_.get(), history_service_.get(),
                            nullptr, history_dir_.GetPath(), SchemeSet()));
   in_memory_url_index_->Init();
+
+  shortcuts_backend_ = base::MakeRefCounted<ShortcutsBackend>(
+      GetTemplateURLService(), std::make_unique<SearchTermsData>(),
+      GetHistoryService(), base::FilePath(), true);
+  shortcuts_backend_->Init();
 }
 
 FakeAutocompleteProviderClient::~FakeAutocompleteProviderClient() {
   // The InMemoryURLIndex must be explicitly shut down or it will DCHECK() in
   // its destructor.
   GetInMemoryURLIndex()->Shutdown();
   set_in_memory_url_index(nullptr);
-  // History index rebuild task is created from main thread during SetUp,
-  // performed on DB thread and must be deleted on main thread.
-  // Run main loop to process delete task, to prevent leaks.
-  base::RunLoop().RunUntilIdle();
+  // Note that RunUntilIdle() must still be called after this, from
+  // whichever task model is being used, probably ScopedTaskEnvironment,
+  // or there will be memory leaks.
 }
 
 const AutocompleteSchemeClassifier&
 FakeAutocompleteProviderClient::GetSchemeClassifier() const {
   return scheme_classifier_;
 }
 
-const SearchTermsData& FakeAutocompleteProviderClient::GetSearchTermsData()
-    const {
-  return search_terms_data_;
-}
-
 history::HistoryService* FakeAutocompleteProviderClient::GetHistoryService() {
   return history_service_.get();
 }
@@ -60,6 +65,21 @@ InMemoryURLIndex* FakeAutocompleteProviderClient::GetInMemoryURLIndex() {
   return in_memory_url_index_.get();
 }
 
+const SearchTermsData& FakeAutocompleteProviderClient::GetSearchTermsData()
+    const {
+  return search_terms_data_;
+}
+
+scoped_refptr<ShortcutsBackend>
+FakeAutocompleteProviderClient::GetShortcutsBackend() {
+  return shortcuts_backend_;
+}
+
+scoped_refptr<ShortcutsBackend>
+FakeAutocompleteProviderClient::GetShortcutsBackendIfExists() {
+  return shortcuts_backend_;
+}
+
 bool FakeAutocompleteProviderClient::IsTabOpenWithURL(const GURL& url) {
   return is_tab_open_with_url_;
 }

components/omnibox/browser/fake_autocomplete_provider_client.h

@@ -12,6 +12,7 @@
 #include "components/omnibox/browser/in_memory_url_index.h"
 #include "components/omnibox/browser/mock_autocomplete_provider_client.h"
 #include "components/omnibox/browser/test_scheme_classifier.h"
+#include "components/search_engines/search_terms_data.h"
 
 namespace bookmarks {
 class BookmarkModel;
@@ -22,18 +23,27 @@ class HistoryService;
 }  // namespace history
 
 class InMemoryURLIndex;
+class ShortcutsBackend;
 
 // Fully operational AutocompleteProviderClient for usage in tests.
+// Note: The history index rebuild task is created from main thread, usually
+// during SetUp(), performed on DB thread and must be deleted on main thread.
+// Run main loop to process delete task, to prevent leaks.
+// Note that these tests have switched to using a ScopedTaskEnvironment,
+// so clearing that task queue is done through
+// scoped_task_environment_.RunUntilIdle().
 class FakeAutocompleteProviderClient : public MockAutocompleteProviderClient {
  public:
-  FakeAutocompleteProviderClient();
+  explicit FakeAutocompleteProviderClient(bool create_history_db = true);
   ~FakeAutocompleteProviderClient() override;
 
   const AutocompleteSchemeClassifier& GetSchemeClassifier() const override;
-  const SearchTermsData& GetSearchTermsData() const override;
   history::HistoryService* GetHistoryService() override;
   bookmarks::BookmarkModel* GetBookmarkModel() override;
   InMemoryURLIndex* GetInMemoryURLIndex() override;
+  const SearchTermsData& GetSearchTermsData() const override;
+  scoped_refptr<ShortcutsBackend> GetShortcutsBackend() override;
+  scoped_refptr<ShortcutsBackend> GetShortcutsBackendIfExists() override;
 
   void set_in_memory_url_index(std::unique_ptr<InMemoryURLIndex> index) {
     in_memory_url_index_ = std::move(index);
@@ -52,6 +62,7 @@ class FakeAutocompleteProviderClient : public MockAutocompleteProviderClient {
   std::unique_ptr<InMemoryURLIndex> in_memory_url_index_;
   std::unique_ptr<history::HistoryService> history_service_;
   bool is_tab_open_with_url_;
+  scoped_refptr<ShortcutsBackend> shortcuts_backend_;
 
   DISALLOW_COPY_AND_ASSIGN(FakeAutocompleteProviderClient);
 };

components/omnibox/browser/history_provider_unittest.cc

@@ -57,6 +57,7 @@ void HistoryProviderTest::SetUp() {
 void HistoryProviderTest::TearDown() {
   provider_ = nullptr;
   client_.reset();
+  scoped_task_environment_.RunUntilIdle();
 }
 
 }  // namespace

components/omnibox/browser/history_quick_provider_performance_unittest.cc

@@ -117,6 +117,7 @@ void HQPPerfTestOnePopularURL::SetUp() {
 void HQPPerfTestOnePopularURL::TearDown() {
   provider_ = nullptr;
   client_.reset();
+  scoped_task_environment_.RunUntilIdle();
 }
 
 void HQPPerfTestOnePopularURL::PrepareData() {

components/omnibox/browser/history_quick_provider_unittest.cc

@@ -219,6 +219,7 @@ void HistoryQuickProviderTest::SetUp() {
 void HistoryQuickProviderTest::TearDown() {
   provider_ = nullptr;
   client_.reset();
+  scoped_task_environment_.RunUntilIdle();
 }
 
 std::vector<HistoryQuickProviderTest::TestURLInfo>

components/omnibox/browser/history_url_provider_unittest.cc

@@ -10,7 +10,6 @@
 #include <memory>
 #include <utility>
 
-#include "base/files/scoped_temp_dir.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/run_loop.h"
@@ -26,12 +25,10 @@
 #include "components/omnibox/browser/autocomplete_provider.h"
 #include "components/omnibox/browser/autocomplete_provider_listener.h"
 #include "components/omnibox/browser/autocomplete_result.h"
+#include "components/omnibox/browser/fake_autocomplete_provider_client.h"
 #include "components/omnibox/browser/history_quick_provider.h"
-#include "components/omnibox/browser/mock_autocomplete_provider_client.h"
-#include "components/omnibox/browser/test_scheme_classifier.h"
 #include "components/prefs/pref_service.h"
 #include "components/search_engines/default_search_manager.h"
-#include "components/search_engines/search_terms_data.h"
 #include "components/search_engines/template_url.h"
 #include "components/search_engines/template_url_service.h"
 #include "components/url_formatter/url_fixer.h"
@@ -171,38 +168,6 @@ struct TestURLInfo {
     {"https://www.wytih/file", "What you typed in history www file", 7, 7, 80},
 };
 
-class AnonFakeAutocompleteProviderClient
-    : public MockAutocompleteProviderClient {
- public:
-  explicit AnonFakeAutocompleteProviderClient(bool create_history_db) {
-    set_template_url_service(base::MakeUnique<TemplateURLService>(nullptr, 0));
-    if (history_dir_.CreateUniqueTempDir()) {
-      history_service_ = history::CreateHistoryService(history_dir_.GetPath(),
-                                                       create_history_db);
-    }
-  }
-
-  const AutocompleteSchemeClassifier& GetSchemeClassifier() const override {
-    return scheme_classifier_;
-  }
-
-  const SearchTermsData& GetSearchTermsData() const override {
-    return search_terms_data_;
-  }
-
-  history::HistoryService* GetHistoryService() override {
-    return history_service_.get();
-  }
-
- private:
-  TestSchemeClassifier scheme_classifier_;
-  SearchTermsData search_terms_data_;
-  base::ScopedTempDir history_dir_;
-  std::unique_ptr<history::HistoryService> history_service_;
-
-  DISALLOW_COPY_AND_ASSIGN(AnonFakeAutocompleteProviderClient);
-};
-
 }  // namespace
 
 class HistoryURLProviderTest : public testing::Test,
@@ -267,7 +232,7 @@ class HistoryURLProviderTest : public testing::Test,
 
   base::test::ScopedTaskEnvironment scoped_task_environment_;
   ACMatches matches_;
-  std::unique_ptr<AnonFakeAutocompleteProviderClient> client_;
+  std::unique_ptr<FakeAutocompleteProviderClient> client_;
   scoped_refptr<HistoryURLProvider> autocomplete_;
   // Should the matches be sorted and duplicates removed?
   bool sort_matches_;
@@ -300,17 +265,18 @@ void HistoryURLProviderTest::OnProviderUpdate(bool updated_matches) {
 }
 
 bool HistoryURLProviderTest::SetUpImpl(bool create_history_db) {
-  client_ =
-      std::make_unique<AnonFakeAutocompleteProviderClient>(create_history_db);
+  client_ = std::make_unique<FakeAutocompleteProviderClient>(create_history_db);
   if (!client_->GetHistoryService())
     return false;
-  autocomplete_ = new HistoryURLProvider(client_.get(), this);
+  autocomplete_ = base::MakeRefCounted<HistoryURLProvider>(client_.get(), this);
   FillData();
   return true;
 }
 
 void HistoryURLProviderTest::TearDown() {
   autocomplete_ = nullptr;
+  client_.reset();
+  scoped_task_environment_.RunUntilIdle();
 }
 
 void HistoryURLProviderTest::FillData() {
@@ -366,7 +332,7 @@ void HistoryURLProviderTest::RunTest(
     std::sort(matches_.begin(), matches_.end(),
               &AutocompleteMatch::MoreRelevant);
   }
-  SCOPED_TRACE(base::ASCIIToUTF16("input = ") + text);
+  SCOPED_TRACE(ASCIIToUTF16("input = ") + text);
   ASSERT_EQ(num_results, matches_.size()) << "Input text: " << text
                                           << "\nTLD: \"" << desired_tld << "\"";
   for (size_t i = 0; i < num_results; ++i) {
@@ -386,7 +352,7 @@ void HistoryURLProviderTest::ExpectFormattedFullMatch(
   ASSERT_FALSE(expected_match_contents_string.empty());
 
   SCOPED_TRACE("input = " + input_text);
-  SCOPED_TRACE(base::ASCIIToUTF16("expected_match_contents = ") +
+  SCOPED_TRACE(ASCIIToUTF16("expected_match_contents = ") +
                expected_match_contents_string);
 
   AutocompleteInput input(ASCIIToUTF16(input_text),
@@ -586,8 +552,8 @@ TEST_F(HistoryURLProviderTest, CullRedirects) {
   redirects_to_a.push_back(GURL(test_cases[2].url));
   redirects_to_a.push_back(GURL(test_cases[0].url));
   client_->GetHistoryService()->AddPage(
-      GURL(test_cases[0].url), base::Time::Now(), nullptr, 0, GURL(),
-      redirects_to_a, ui::PAGE_TRANSITION_TYPED, history::SOURCE_BROWSED, true);
+      GURL(test_cases[0].url), Time::Now(), nullptr, 0, GURL(), redirects_to_a,
+      ui::PAGE_TRANSITION_TYPED, history::SOURCE_BROWSED, true);
 
   // Because all the results are part of a redirect chain with other results,
   // all but the first one (A) should be culled. We should get the default

components/omnibox/browser/mock_autocomplete_provider_client.h

@@ -50,13 +50,6 @@ class MockAutocompleteProviderClient : public AutocompleteProviderClient {
       bool create_if_necessary) const override {
     return contextual_suggestions_service_.get();
   }
-  std::unique_ptr<KeywordExtensionsDelegate> GetKeywordExtensionsDelegate(
-      KeywordProvider* keyword_provider) override {
-    return nullptr;
-  }
-  physical_web::PhysicalWebDataSource* GetPhysicalWebDataSource() override {
-    return nullptr;
-  }
 
   MOCK_CONST_METHOD0(GetSearchTermsData, const SearchTermsData&());
 
@@ -67,6 +60,13 @@ class MockAutocompleteProviderClient : public AutocompleteProviderClient {
   scoped_refptr<ShortcutsBackend> GetShortcutsBackendIfExists() override {
     return nullptr;
   }
+  std::unique_ptr<KeywordExtensionsDelegate> GetKeywordExtensionsDelegate(
+      KeywordProvider* keyword_provider) override {
+    return nullptr;
+  }
+  physical_web::PhysicalWebDataSource* GetPhysicalWebDataSource() override {
+    return nullptr;
+  }
 
   MOCK_CONST_METHOD0(GetAcceptLanguages, std::string());
   MOCK_METHOD0(GetEmbedderRepresentationOfAboutScheme, std::string());
@@ -88,12 +88,12 @@ class MockAutocompleteProviderClient : public AutocompleteProviderClient {
                void(history::KeywordID keyword_id, const base::string16& term));
   MOCK_METHOD1(PrefetchImage, void(const GURL& url));
 
+  bool IsTabOpenWithURL(const GURL& url) override { return false; }
+
   void set_template_url_service(std::unique_ptr<TemplateURLService> service) {
     template_url_service_ = std::move(service);
   }
 
-  bool IsTabOpenWithURL(const GURL& url) override { return false; }
-
  private:
   std::unique_ptr<ContextualSuggestionsService> contextual_suggestions_service_;
   std::unique_ptr<TemplateURLService> template_url_service_;