Removing bad dependencies from autocomplete_provider_unittest

components/omnibox/browser/autocomplete_provider_unittest.cc

 // Copyright (c) 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.
 
 #include "components/omnibox/browser/autocomplete_provider.h"
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/location.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/message_loop/message_loop.h"
 #include "base/single_thread_task_runner.h"
 #include "base/strings/string16.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/thread_task_runner_handle.h"
-#include "chrome/browser/autocomplete/chrome_autocomplete_provider_client.h"
-#include "chrome/browser/autocomplete/chrome_autocomplete_scheme_classifier.h"
-#include "chrome/browser/chrome_notification_types.h"
-#include "chrome/browser/search_engines/template_url_service_factory.h"
-#include "chrome/test/base/testing_browser_process.h"
-#include "chrome/test/base/testing_profile.h"
 #include "components/metrics/proto/omnibox_event.pb.h"
 #include "components/omnibox/browser/autocomplete_controller.h"
 #include "components/omnibox/browser/autocomplete_input.h"
 #include "components/omnibox/browser/autocomplete_match.h"
 #include "components/omnibox/browser/autocomplete_provider_listener.h"
 #include "components/omnibox/browser/keyword_provider.h"
+#include "components/omnibox/browser/mock_autocomplete_provider_client.h"
 #include "components/omnibox/browser/search_provider.h"
 #include "components/search_engines/search_engines_switches.h"
 #include "components/search_engines/template_url.h"
 #include "components/search_engines/template_url_service.h"
-#include "content/public/browser/notification_observer.h"
-#include "content/public/browser/notification_registrar.h"
-#include "content/public/browser/notification_source.h"
-#include "content/public/test/test_browser_thread_bundle.h"
+#include "components/search_engines/template_url_service_client.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 static std::ostream& operator<<(std::ostream& os,
                                 const AutocompleteResult::const_iterator& it) {
   return os << static_cast<const AutocompleteMatch*>(&(*it));
 }
 
 namespace {

[Peter Kasting, 2015/11/19 19:04:41]

Nit: Blank line below

[Abhishek, 2015/11/20 09:37:20]

Done.

 const size_t kResultsPerProvider = 3;
 const char kTestTemplateURLKeyword[] = "t";
-}
+
+class TestingSchemeClassifier : public AutocompleteSchemeClassifier {
+ public:
+  metrics::OmniboxInputType::Type GetInputTypeForScheme(
+      const std::string& scheme) const override {
+    if (net::URLRequest::IsHandledProtocol(scheme))
+      return metrics::OmniboxInputType::URL;
+    return metrics::OmniboxInputType::INVALID;

[Peter Kasting, 2015/11/19 19:04:41]

Nit: Simpler:

    return net::URLRequest::IsHandledProtocol(scheme) ?
        metrics::OmniboxInputType::URL : metrics::OmniboxInputType::INVALID;

[Abhishek, 2015/11/20 09:37:21]

Done.

+  }
+};

[Peter Kasting, 2015/11/19 19:04:41]

Nit: DISALLOW_COPY_AND_ASSIGN

[Abhishek, 2015/11/20 09:37:20]

Done.

+
+// AutocompleteProviderClient implementation that calls the specified closure
+// when the result is ready.
+class AutocompleteProviderClientWithClosure
+    : public MockAutocompleteProviderClient {
+ public:
+  void set_closure(const base::Closure& closure) { closure_ = closure; }
+
+ private:
+  void OnAutocompleteControllerResultReady(
+      AutocompleteController* controller) override {
+    if (closure_.is_null())
+      return;

[Peter Kasting, 2015/11/19 19:04:41]

Don't we still need to call QuitWhenIdle() if the closure is null?

[Abhishek, 2015/11/20 09:37:20]

I think we can't do that, because tests are getting crashed if I add it.

[Peter Kasting, 2015/11/20 19:52:18]

Can you investigate to the point of understanding why?  If it turns out there's
a good reason that's happening, we should put a comment here explaining why we
shouldn't do this; or if it turns out the crashes are due to a bug, we should
fix that.

[Abhishek, 2015/11/21 12:11:20]

AutocompleteProviderTest.AllowExactKeywordMatch
AutocompleteProviderTest.ExtraQueryParams

These are the cases when closure_ is null.

If we call QuitWhenIdle when closure_ is null then in release build it's not
crashing, but in debug build it crashes.

I think I'll add TODO for me and create a bug for it.
WDYT?

[Peter Kasting, 2015/11/21 21:08:42]

Crashes how?  DCHECK failure?  I think you should continue investigating this at
least until you completely understand the crash before checking this patch in.

[Abhishek, 2015/11/23 15:22:07]

Actually its crashing in both the cases(either closure_ is null or not null)
into debug build.
Basically its a CHECK failed terminate inside the MessageLoop while calling
QuitWhenIdle().

Here are the crash logs from my local:
--
[32077:32077:1123/200225:11232244126621:FATAL:message_loop.cc(302)] Check
failed: false. Must be inside Run to call Quit
#0 0x00000182c70e base::debug::StackTrace::StackTrace()
#1 0x0000017473bf logging::LogMessage::~LogMessage()
#2 0x00000175024c base::MessageLoop::QuitWhenIdle()
#3 0x000000cd5b69 (anonymous
namespace)::AutocompleteProviderClientWithClosure::OnAutocompleteControllerResultReady()
#4 0x00000223d24f AutocompleteController::NotifyChanged()
#5 0x00000223b9e6 AutocompleteController::UpdateResult()
#6 0x00000223afd9 AutocompleteController::Start()
#7 0x000000cd2686 AutocompleteProviderTest::RunExactKeymatchTest()
--
Other function BT is also same.
No.
When I google it I found similar callstack related to MessageLoop at
https://crbug.com/541737. But I'm not sure how much its applicable here.

Could you please help me to understand this debug build terminate issue?

[droger, 2015/11/23 16:11:15]

I don't know much about the specifics of this test, but the change that is done
in this CL is preserving the existing behavior.

Before this CL, this code was run once the test registered to the
NOTIFICATION_AUTOCOMPLETE_CONTROLLER_RESULT_READY notification, and was not run
at all when the notification was not observed.

The new code calls set_closure() instead of registering for the notification,
but the behavior is otherwise similar.
You can see that set_closure() is called at the same place where the
notification was registered (line 338 below).

Removing the early return causes the code to be called at some time when it was
not run before.

+
+    if (controller->done()) {
+      closure_.Run();
+      base::MessageLoop::current()->QuitWhenIdle();
+    }
+  }
+
+  base::Closure closure_;
+};

[Peter Kasting, 2015/11/19 19:04:41]

Nit: DISALLOW_COPY_AND_ASSIGN

[Abhishek, 2015/11/20 09:37:21]

Done.

+
+}  // namespace
 
 // Autocomplete provider that provides known results. Note that this is
 // refcounted so that it can also be a task on the message loop.
 class TestProvider : public AutocompleteProvider {
  public:
-  TestProvider(int relevance, const base::string16& prefix,
-               Profile* profile,
-               const base::string16 match_keyword)
+  TestProvider(int relevance,
+               const base::string16& prefix,
+               const base::string16 match_keyword,
+               AutocompleteProviderClient* client)
       : AutocompleteProvider(AutocompleteProvider::TYPE_SEARCH),
         listener_(NULL),
-        profile_(profile),
         relevance_(relevance),
         prefix_(prefix),
-        match_keyword_(match_keyword) {
-  }
+        match_keyword_(match_keyword),
+        client_(client) {}
 
   void Start(const AutocompleteInput& input, bool minimal_changes) override;
 
   void set_listener(AutocompleteProviderListener* listener) {
     listener_ = listener;
   }
 
  private:
   ~TestProvider() override {}
 
   void Run();
 
   void AddResults(int start_at, int num);
   void AddResultsWithSearchTermsArgs(
       int start_at,
       int num,
       AutocompleteMatch::Type type,
       const TemplateURLRef::SearchTermsArgs& search_terms_args);
 
   AutocompleteProviderListener* listener_;
-  Profile* profile_;
   int relevance_;
   const base::string16 prefix_;
   const base::string16 match_keyword_;
+  AutocompleteProviderClient* client_;
 };

[Peter Kasting, 2015/11/19 19:04:41]

Nit: DISALLOW_COPY_AND_ASSIGN

[Abhishek, 2015/11/20 09:37:21]

Done.

 
 void TestProvider::Start(const AutocompleteInput& input, bool minimal_changes) {
   if (minimal_changes)
     return;
 
   matches_.clear();
 
   if (input.from_omnibox_focus())
     return;
 
@@ skipping 18 matching lines @@
 
 void TestProvider::Run() {
   DCHECK_GT(kResultsPerProvider, 0U);
   AddResults(1, kResultsPerProvider);
   done_ = true;
   DCHECK(listener_);
   listener_->OnProviderUpdate(true);
 }
 
 void TestProvider::AddResults(int start_at, int num) {
-  AddResultsWithSearchTermsArgs(start_at,
-                                num,
-                                AutocompleteMatchType::URL_WHAT_YOU_TYPED,
-                                TemplateURLRef::SearchTermsArgs(
-                                    base::string16()));
+  AddResultsWithSearchTermsArgs(
+      start_at, num, AutocompleteMatchType::URL_WHAT_YOU_TYPED,
+      TemplateURLRef::SearchTermsArgs(base::string16()));
 }
 
 void TestProvider::AddResultsWithSearchTermsArgs(
     int start_at,
     int num,
     AutocompleteMatch::Type type,
     const TemplateURLRef::SearchTermsArgs& search_terms_args) {
   for (int i = start_at; i < num; i++) {
     AutocompleteMatch match(this, relevance_ - i, false, type);
 
     match.fill_into_edit = prefix_ + base::UTF8ToUTF16(base::IntToString(i));
     match.destination_url = GURL(base::UTF16ToUTF8(match.fill_into_edit));
     match.allowed_to_be_default_match = true;
 
     match.contents = match.fill_into_edit;
     match.contents_class.push_back(
         ACMatchClassification(0, ACMatchClassification::NONE));
     match.description = match.fill_into_edit;
     match.description_class.push_back(
         ACMatchClassification(0, ACMatchClassification::NONE));
     match.search_terms_args.reset(
         new TemplateURLRef::SearchTermsArgs(search_terms_args));
     if (!match_keyword_.empty()) {
       match.keyword = match_keyword_;
-      TemplateURLService* service =
-          TemplateURLServiceFactory::GetForProfile(profile_);
+      TemplateURLService* service = client_->GetTemplateURLService();
       ASSERT_TRUE(match.GetTemplateURL(service, false) != NULL);
     }
 
     matches_.push_back(match);
   }
 }
 
-class AutocompleteProviderTest : public testing::Test,
-                                 public content::NotificationObserver {
+class AutocompleteProviderTest : public testing::Test {
+ public:
+  ~AutocompleteProviderTest() override;
+  AutocompleteProviderTest();

[Peter Kasting, 2015/11/19 19:04:41]

Nit: Constructor before destructor

[Abhishek, 2015/11/20 09:37:20]

Done.

+
  protected:
   struct KeywordTestData {
     const base::string16 fill_into_edit;
     const base::string16 keyword;
     const base::string16 expected_associated_keyword;
   };
 
   struct AssistedQueryStatsTestData {
     const AutocompleteMatch::Type match_type;
     const std::string expected_aqs;
   };
 
  protected:

[Peter Kasting, 2015/11/19 19:04:41]

Nit: Remove this duplicate line

[Abhishek, 2015/11/20 09:37:21]

Done.

-   // Registers a test TemplateURL under the given keyword.
+  // Registers a test TemplateURL under the given keyword.
   void RegisterTemplateURL(const base::string16 keyword,
                            const std::string& template_url);
 
   // Resets |controller_| with two TestProviders.  |provider1_ptr| and
   // |provider2_ptr| are updated to point to the new providers if non-NULL.
   void ResetControllerWithTestProviders(bool same_destinations,
                                         TestProvider** provider1_ptr,
                                         TestProvider** provider2_ptr);
 
   // Runs a query on the input "a", and makes sure both providers' input is
@@ skipping 32 matching lines @@
     return controller_->search_provider_->field_trial_triggered_in_session();
   }
   void set_current_page_classification(
       metrics::OmniboxEventProto::PageClassification classification) {
     controller_->input_.current_page_classification_ = classification;
   }
 
   AutocompleteResult result_;
 
  private:
-  // content::NotificationObserver:
-  void Observe(int type,
-               const content::NotificationSource& source,
-               const content::NotificationDetails& details) override;
+  base::MessageLoop message_loop_;
+  scoped_ptr<AutocompleteController> controller_;
+  // Owned by |controller_|.
+  AutocompleteProviderClientWithClosure* client_;
+  // Sanity check to ensure that |client_| ownership has been passed to

[Peter Kasting, 2015/11/19 19:04:40]

Nit: Sanity check -> Used

[Abhishek, 2015/11/20 09:37:20]

Done.

+  // |controller_| exactly once.
+  bool client_owned_;
+};

[Peter Kasting, 2015/11/19 19:04:41]

Nit: DISALLOW_COPY_AND_ASSIGN

[Abhishek, 2015/11/20 09:37:20]

Done.

 
-  content::TestBrowserThreadBundle thread_bundle_;
-  content::NotificationRegistrar registrar_;
-  TestingProfile profile_;
-  scoped_ptr<AutocompleteController> controller_;
-};
+AutocompleteProviderTest::AutocompleteProviderTest()
+    : client_(new AutocompleteProviderClientWithClosure), client_owned_(false) {
+  scoped_ptr<TemplateURLService> template_url_service(
+      new TemplateURLService(nullptr, 0));
+  client_->set_template_url_service(template_url_service.Pass());
+}
+
+AutocompleteProviderTest::~AutocompleteProviderTest() {
+  DCHECK(client_owned_);

[Peter Kasting, 2015/11/19 19:04:41]

Avoid using DCHECK in tests; failed DCHECKs crash the test and leave the bot in
a bad state.  Find ways to test this using ASSERT_ macros instead.  (Several
places)

[Abhishek, 2015/11/20 09:37:20]

As per the gtest.h we can't use ASSERT_* directly from destructor. So I'm doing
this inside TearDown().
Please correct me if I'm wrong here!

[Peter Kasting, 2015/11/20 19:52:18]

Avoiding ASSERT in destructors is a good move.  I believe you can safely use
EXPECT in a destructor, though, which is probably the right thing to do anyway
here, since failing this check shouldn't instantly halt the test.

[Abhishek, 2015/11/21 12:11:20]

Done.

+}
 
 void AutocompleteProviderTest::RegisterTemplateURL(
     const base::string16 keyword,
     const std::string& template_url) {
-  if (TemplateURLServiceFactory::GetForProfile(&profile_) == NULL) {
-    TemplateURLServiceFactory::GetInstance()->SetTestingFactoryAndUse(
-        &profile_, &TemplateURLServiceFactory::BuildInstanceFor);
-  }
   TemplateURLData data;
   data.SetURL(template_url);
   data.SetShortName(keyword);
   data.SetKeyword(keyword);
   TemplateURL* default_t_url = new TemplateURL(data);
-  TemplateURLService* turl_model =
-      TemplateURLServiceFactory::GetForProfile(&profile_);
+  TemplateURLService* turl_model = client_->GetTemplateURLService();
   turl_model->Add(default_t_url);
   turl_model->SetUserSelectedDefaultSearchProvider(default_t_url);
   turl_model->Load();
   TemplateURLID default_provider_id = default_t_url->id();
   ASSERT_NE(0, default_provider_id);
 }
 
 void AutocompleteProviderTest::ResetControllerWithTestProviders(
     bool same_destinations,
     TestProvider** provider1_ptr,
     TestProvider** provider2_ptr) {
   // TODO: Move it outside this method, after refactoring the existing
   // unit tests.  Specifically:
   //   (1) Make sure that AutocompleteMatch.keyword is set iff there is
   //       a corresponding call to RegisterTemplateURL; otherwise the
   //       controller flow will crash; this practically means that
   //       RunTests/ResetControllerXXX/RegisterTemplateURL should
   //       be coordinated with each other.
   //   (2) Inject test arguments rather than rely on the hardcoded values, e.g.
   //       don't rely on kResultsPerProvided and default relevance ordering
   //       (B > A).
   RegisterTemplateURL(base::ASCIIToUTF16(kTestTemplateURLKeyword),
                       "http://aqs/{searchTerms}/{google:assistedQueryStats}");
 
   AutocompleteController::Providers providers;
 
   // Construct two new providers, with either the same or different prefixes.
-  TestProvider* provider1 = new TestProvider(
-      kResultsPerProvider,
-      base::ASCIIToUTF16("http://a"),
-      &profile_,
-      base::ASCIIToUTF16(kTestTemplateURLKeyword));
+  TestProvider* provider1 =
+      new TestProvider(kResultsPerProvider, base::ASCIIToUTF16("http://a"),
+                       base::ASCIIToUTF16(kTestTemplateURLKeyword), client_);
   providers.push_back(provider1);
 
-  TestProvider* provider2 = new TestProvider(
-      kResultsPerProvider * 2,
-      same_destinations ? base::ASCIIToUTF16("http://a")
-                        : base::ASCIIToUTF16("http://b"),
-      &profile_,
-      base::string16());
+  TestProvider* provider2 =
+      new TestProvider(kResultsPerProvider * 2,
+                       same_destinations ? base::ASCIIToUTF16("http://a")
+                                         : base::ASCIIToUTF16("http://b"),
+                       base::string16(), client_);

[Peter Kasting, 2015/11/19 19:04:41]

Nit: Shorter:

  TestProvider* provider2 = new TestProvider(
      kResultsPerProvider * 2,
      base::ASCIIToUTF16(same_destinations ? "http://a" : "http://b"),
      base::string16(), client_);

[Abhishek, 2015/11/20 09:37:21]

Done.

   providers.push_back(provider2);
 
+  DCHECK(!client_owned_);
   // Reset the controller to contain our new providers.
-  controller_.reset(new AutocompleteController(
-
-      make_scoped_ptr(new ChromeAutocompleteProviderClient(&profile_)), NULL,
-      0));
+  controller_.reset(
+      new AutocompleteController(make_scoped_ptr(client_), NULL, 0));
+  client_owned_ = true;

[Peter Kasting, 2015/11/19 19:04:41]

Nit: Three times you write a block of code to do:

  DCHECK(!client_owned_);
  controller_.reset(new AutocompleteController(make_scoped_ptr(client_),
                    nullptr, type));
  client_owned_ = true;

Where the only difference is |type|.  Factor this out into a helper function.

(Note use of nullptr for NULL as well)

[Abhishek, 2015/11/20 09:37:21]

Done.

   // We're going to swap the providers vector, but the old vector should be
   // empty so no elements need to be freed at this point.
   EXPECT_TRUE(controller_->providers_.empty());
   controller_->providers_.swap(providers);
   provider1->set_listener(controller_.get());
   provider2->set_listener(controller_.get());
 
-  // The providers don't complete synchronously, so listen for "result updated"
-  // notifications.
-  registrar_.Add(this,
-                 chrome::NOTIFICATION_AUTOCOMPLETE_CONTROLLER_RESULT_READY,
-                 content::Source<AutocompleteController>(controller_.get()));
+  client_->set_closure(base::Bind(&AutocompleteProviderTest::CopyResults,
+                                  base::Unretained(this)));
 
   if (provider1_ptr)
     *provider1_ptr = provider1;
   if (provider2_ptr)
     *provider2_ptr = provider2;
 }
 
-void AutocompleteProviderTest::
-    ResetControllerWithKeywordAndSearchProviders() {
-  TemplateURLServiceFactory::GetInstance()->SetTestingFactoryAndUse(
-      &profile_, &TemplateURLServiceFactory::BuildInstanceFor);
-
+void AutocompleteProviderTest::ResetControllerWithKeywordAndSearchProviders() {
   // Reset the default TemplateURL.
   TemplateURLData data;
   data.SetShortName(base::ASCIIToUTF16("default"));
   data.SetKeyword(base::ASCIIToUTF16("default"));
   data.SetURL("http://defaultturl/{searchTerms}");
   TemplateURL* default_t_url = new TemplateURL(data);
-  TemplateURLService* turl_model =
-      TemplateURLServiceFactory::GetForProfile(&profile_);
+  TemplateURLService* turl_model = client_->GetTemplateURLService();
   turl_model->Add(default_t_url);
   turl_model->SetUserSelectedDefaultSearchProvider(default_t_url);
   TemplateURLID default_provider_id = default_t_url->id();
   ASSERT_NE(0, default_provider_id);
 
   // Create another TemplateURL for KeywordProvider.
   TemplateURLData data2;
   data2.SetShortName(base::ASCIIToUTF16("k"));
   data2.SetKeyword(base::ASCIIToUTF16("k"));
   data2.SetURL("http://keyword/{searchTerms}");
   TemplateURL* keyword_t_url = new TemplateURL(data2);
   turl_model->Add(keyword_t_url);
   ASSERT_NE(0, keyword_t_url->id());
 
+  DCHECK(!client_owned_);
   controller_.reset(new AutocompleteController(
-
-      make_scoped_ptr(new ChromeAutocompleteProviderClient(&profile_)), NULL,
+      make_scoped_ptr(client_), NULL,
       AutocompleteProvider::TYPE_KEYWORD | AutocompleteProvider::TYPE_SEARCH));
+  client_owned_ = true;
 }
 
 void AutocompleteProviderTest::ResetControllerWithKeywordProvider() {
-  TemplateURLServiceFactory::GetInstance()->SetTestingFactoryAndUse(
-      &profile_, &TemplateURLServiceFactory::BuildInstanceFor);
-
-  TemplateURLService* turl_model =
-      TemplateURLServiceFactory::GetForProfile(&profile_);
+  TemplateURLService* turl_model = client_->GetTemplateURLService();
 
   // Create a TemplateURL for KeywordProvider.
   TemplateURLData data;
   data.SetShortName(base::ASCIIToUTF16("foo.com"));
   data.SetKeyword(base::ASCIIToUTF16("foo.com"));
   data.SetURL("http://foo.com/{searchTerms}");
   TemplateURL* keyword_t_url = new TemplateURL(data);
   turl_model->Add(keyword_t_url);
   ASSERT_NE(0, keyword_t_url->id());
 
   // Make a TemplateURL for KeywordProvider that a shorter version of the
   // first.
   data.SetShortName(base::ASCIIToUTF16("f"));
   data.SetKeyword(base::ASCIIToUTF16("f"));
   data.SetURL("http://f.com/{searchTerms}");
   keyword_t_url = new TemplateURL(data);
   turl_model->Add(keyword_t_url);
   ASSERT_NE(0, keyword_t_url->id());
 
   // Create another TemplateURL for KeywordProvider.
   data.SetShortName(base::ASCIIToUTF16("bar.com"));
   data.SetKeyword(base::ASCIIToUTF16("bar.com"));
   data.SetURL("http://bar.com/{searchTerms}");
   keyword_t_url = new TemplateURL(data);
   turl_model->Add(keyword_t_url);
   ASSERT_NE(0, keyword_t_url->id());
 
+  DCHECK(!client_owned_);
   controller_.reset(new AutocompleteController(
-      make_scoped_ptr(new ChromeAutocompleteProviderClient(&profile_)), NULL,
-      AutocompleteProvider::TYPE_KEYWORD));
+      make_scoped_ptr(client_), NULL, AutocompleteProvider::TYPE_KEYWORD));
+  client_owned_ = true;
 }
 
 void AutocompleteProviderTest::RunTest() {
   RunQuery(base::ASCIIToUTF16("a"));
 }
 
 void AutocompleteProviderTest::RunKeywordTest(const base::string16& input,
                                               const KeywordTestData* match_data,
                                               size_t size) {
   ACMatches matches;
   for (size_t i = 0; i < size; ++i) {
     AutocompleteMatch match;
     match.relevance = 1000;  // Arbitrary non-zero value.
     match.allowed_to_be_default_match = true;
     match.fill_into_edit = match_data[i].fill_into_edit;
     match.transition = ui::PAGE_TRANSITION_KEYWORD;
     match.keyword = match_data[i].keyword;
     matches.push_back(match);
   }
 
   controller_->input_ = AutocompleteInput(
       input, base::string16::npos, std::string(), GURL(),
       metrics::OmniboxEventProto::INSTANT_NTP_WITH_OMNIBOX_AS_STARTING_FOCUS,
-      false, true, true, true, false,
-      ChromeAutocompleteSchemeClassifier(&profile_));
+      false, true, true, true, false, TestingSchemeClassifier());
   AutocompleteResult result;
   result.AppendMatches(controller_->input_, matches);
   controller_->UpdateAssociatedKeywords(&result);
-
   for (size_t j = 0; j < result.size(); ++j) {
     EXPECT_EQ(match_data[j].expected_associated_keyword,
               result.match_at(j)->associated_keyword.get() ?
                   result.match_at(j)->associated_keyword->keyword :
                   base::string16());
   }
 }
 
 void AutocompleteProviderTest::RunAssistedQueryStatsTest(
     const AssistedQueryStatsTestData* aqs_test_data,
@@ skipping 18 matching lines @@
 
   // Verify data.
   for (size_t i = 0; i < size; ++i) {
     EXPECT_EQ(aqs_test_data[i].expected_aqs,
               result_.match_at(i)->search_terms_args->assisted_query_stats);
   }
 }
 
 void AutocompleteProviderTest::RunQuery(const base::string16 query) {
   result_.Reset();
-  controller_->Start(AutocompleteInput(
-      query, base::string16::npos, std::string(), GURL(),
-      metrics::OmniboxEventProto::INVALID_SPEC, true, false, true, true, false,
-      ChromeAutocompleteSchemeClassifier(&profile_)));
+  controller_->Start(
+      AutocompleteInput(query, base::string16::npos, std::string(), GURL(),
+                        metrics::OmniboxEventProto::INVALID_SPEC, true, false,
+                        true, true, false, TestingSchemeClassifier()));
 
   if (!controller_->done())
     // The message loop will terminate when all autocomplete input has been
     // collected.
     base::MessageLoop::current()->Run();
 }
 
 void AutocompleteProviderTest::RunExactKeymatchTest(
     bool allow_exact_keyword_match) {
   // Send the controller input which exactly matches the keyword provider we
   // created in ResetControllerWithKeywordAndSearchProviders().  The default
   // match should thus be a search-other-engine match iff
   // |allow_exact_keyword_match| is true.  Regardless, the match should
   // be from SearchProvider.  (It provides all verbatim search matches,
   // keyword or not.)
   controller_->Start(AutocompleteInput(
       base::ASCIIToUTF16("k test"), base::string16::npos, std::string(), GURL(),
       metrics::OmniboxEventProto::INVALID_SPEC, true, false,
-      allow_exact_keyword_match, false, false,
-      ChromeAutocompleteSchemeClassifier(&profile_)));
+      allow_exact_keyword_match, false, false, TestingSchemeClassifier()));
   EXPECT_TRUE(controller_->done());
   EXPECT_EQ(AutocompleteProvider::TYPE_SEARCH,
       controller_->result().default_match()->provider->type());
   EXPECT_EQ(allow_exact_keyword_match ?
       AutocompleteMatchType::SEARCH_OTHER_ENGINE :
       AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED,
       controller_->result().default_match()->type);
 }
 
 void AutocompleteProviderTest::CopyResults() {
   result_.CopyFrom(controller_->result());
 }
 
 GURL AutocompleteProviderTest::GetDestinationURL(
     AutocompleteMatch match,
     base::TimeDelta query_formulation_time) const {
   controller_->UpdateMatchDestinationURLWithQueryFormulationTime(
       query_formulation_time, &match);
   return match.destination_url;
 }
 
-void AutocompleteProviderTest::Observe(
-    int type,
-    const content::NotificationSource& source,
-    const content::NotificationDetails& details) {
-  if (controller_->done()) {
-    CopyResults();
-    base::MessageLoop::current()->QuitWhenIdle();
-  }
-}
-
 // Tests that the default selection is set properly when updating results.
 TEST_F(AutocompleteProviderTest, Query) {
   TestProvider* provider1 = NULL;
   TestProvider* provider2 = NULL;
   ResetControllerWithTestProviders(false, &provider1, &provider2);
   RunTest();
 
   // Make sure the default match gets set to the highest relevance match.  The
   // highest relevance matches should come from the second provider.
   EXPECT_EQ(kResultsPerProvider * 2, result_.size());
@@ skipping 104 matching lines @@
 // Test that exact match keywords trump keywords associated with
 // the match.
 TEST_F(AutocompleteProviderTest, ExactMatchKeywords) {
   ResetControllerWithKeywordProvider();
 
   {
     KeywordTestData keyword_match[] = {
       { base::ASCIIToUTF16("foo.com"), base::string16(),
         base::ASCIIToUTF16("foo.com") }
     };
-

[Peter Kasting, 2015/11/19 19:04:41]

Nit: Don't remove this

[Abhishek, 2015/11/20 09:37:21]

Done.

     SCOPED_TRACE("keyword match as usual");
     RunKeywordTest(base::ASCIIToUTF16("fo"), keyword_match,
                    arraysize(keyword_match));
   }
 
   // The same result set with an input of "f" (versus "fo") should get
   // a different associated keyword because "f" is an exact match for
   // a keyword and that should trump the keyword normally associated with
   // this match.
   {
@@ skipping 98 matching lines @@
   EXPECT_FALSE(search_provider_field_trial_triggered_in_session());
   url = GetDestinationURL(match, base::TimeDelta::FromMilliseconds(2456));
   EXPECT_EQ("//aqs=chrome.0.69i57j69i58j5l2j0l3j69i59.2456j0j4&", url.path());
 
   // Test page classification and field trial triggered set.
   set_search_provider_field_trial_triggered_in_session(true);
   EXPECT_TRUE(search_provider_field_trial_triggered_in_session());
   url = GetDestinationURL(match, base::TimeDelta::FromMilliseconds(2456));
   EXPECT_EQ("//aqs=chrome.0.69i57j69i58j5l2j0l3j69i59.2456j1j4&", url.path());
 }