[Omnibox] Reland 94417e2806 and a3d1b9e964.

These CLs were not actually the culprits for the OmniboxView browsertest failures on Win7; see crbug.com/514934 for details. BUG=371536,511943 TBR=pkasting Review URL: https://codereview.chromium.org/1257663004 Cr-Commit-Position: refs/heads/master@{#340923}
author: blundell <blundell@chromium.org> 2015-07-29 11:16:20 -0700
committer: Commit bot <commit-bot@chromium.org> 2015-07-29 18:16:58 +0000
commit: 3c100cee848bd29360dad2cba9a7013910dacafb (patch)
tree: ef1449284575d5fa1ca87eaa7c94c9126ec726a3
parent: 587c07234226e832a986b7ed055d8c918415695e (diff)
download: chromium_src-3c100cee848bd29360dad2cba9a7013910dacafb.zip
chromium_src-3c100cee848bd29360dad2cba9a7013910dacafb.tar.gz
chromium_src-3c100cee848bd29360dad2cba9a7013910dacafb.tar.bz2
8 files changed, 303 insertions, 182 deletions
diff --git a/chrome/browser/ui/omnibox/chrome_omnibox_client.cc b/chrome/browser/ui/omnibox/chrome_omnibox_client.cc
index 8263993..0b42119 100644
--- a/chrome/browser/ui/omnibox/chrome_omnibox_client.cc
+++ b/chrome/browser/ui/omnibox/chrome_omnibox_client.cc
@@ -4,13 +4,22 @@
 
 #include "chrome/browser/ui/omnibox/chrome_omnibox_client.h"
 
+#include "base/bind.h"
+#include "base/callback.h"
+#include "base/metrics/histogram.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/app/chrome_command_ids.h"
+#include "chrome/browser/autocomplete/chrome_autocomplete_provider_client.h"
+#include "chrome/browser/bitmap_fetcher/bitmap_fetcher_service_factory.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/command_updater.h"
 #include "chrome/browser/extensions/api/omnibox/omnibox_api.h"
+#include "chrome/browser/net/predictor.h"
 #include "chrome/browser/predictors/autocomplete_action_predictor.h"
 #include "chrome/browser/predictors/autocomplete_action_predictor_factory.h"
+#include "chrome/browser/prerender/prerender_field_trial.h"
+#include "chrome/browser/prerender/prerender_manager.h"
+#include "chrome/browser/prerender/prerender_manager_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/search/search.h"
 #include "chrome/browser/search_engines/template_url_service_factory.h"
@@ -18,22 +27,103 @@
 #include "chrome/browser/ui/omnibox/omnibox_edit_controller.h"
 #include "chrome/browser/ui/search/instant_search_prerenderer.h"
 #include "chrome/browser/ui/search/search_tab_helper.h"
+#include "chrome/common/instant_types.h"
 #include "components/omnibox/browser/autocomplete_match.h"
+#include "components/omnibox/browser/autocomplete_result.h"
+#include "components/omnibox/browser/search_provider.h"
 #include "components/search/search.h"
 #include "components/search_engines/template_url_service.h"
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/web_contents.h"
+#include "extensions/common/constants.h"
 #include "ui/base/window_open_disposition.h"
 #include "url/gurl.h"
 
-ChromeOmniboxClient::ChromeOmniboxClient(
-    OmniboxEditController* controller,
-    Profile* profile)
+namespace {
+
+// Returns the AutocompleteMatch that the InstantController should prefetch, if
+// any.
+//
+// The SearchProvider may mark some suggestions to be prefetched based on
+// instructions from the suggest server. If such a match ranks sufficiently
+// highly or if kAllowPrefetchNonDefaultMatch field trial is enabled, we'll
+// return it.
+//
+// If the kAllowPrefetchNonDefaultMatch field trial is enabled we return the
+// prefetch suggestion even if it is not the default match. Otherwise we only
+// care about matches that are the default or the second entry in the dropdown
+// (which can happen for non-default matches when a top verbatim match is
+// shown); for other matches, we think the likelihood of the user selecting
+// them is low enough that prefetching isn't worth doing.
+const AutocompleteMatch* GetMatchToPrefetch(const AutocompleteResult& result) {
+  if (chrome::ShouldAllowPrefetchNonDefaultMatch()) {
+    const AutocompleteResult::const_iterator prefetch_match = std::find_if(
+        result.begin(), result.end(), SearchProvider::ShouldPrefetch);
+    return prefetch_match != result.end() ? &(*prefetch_match) : NULL;
+  }
+
+  // If the default match should be prefetched, do that.
+  const auto default_match = result.default_match();
+  if ((default_match != result.end()) &&
+      SearchProvider::ShouldPrefetch(*default_match))
+    return &(*default_match);
+
+  // Otherwise, if the top match is a verbatim match and the very next match
+  // is prefetchable, fetch that.
+  if (result.TopMatchIsStandaloneVerbatimMatch() && (result.size() > 1) &&
+      SearchProvider::ShouldPrefetch(result.match_at(1)))
+    return &result.match_at(1);
+
+  return NULL;
+}
+
+// Calls the specified callback when the requested image is downloaded.  This
+// is a separate class instead of being implemented on ChromeOmniboxClient
+// because BitmapFetcherService currently takes ownership of this object.
+// TODO(dschuyler): Make BitmapFetcherService use the more typical non-owning
+// ObserverList pattern and have ChromeOmniboxClient implement the Observer
+// call directly.
+class AnswerImageObserver : public BitmapFetcherService::Observer {
+ public:
+  explicit AnswerImageObserver(const BitmapFetchedCallback& callback)
+      : callback_(callback) {}
+
+  void OnImageChanged(BitmapFetcherService::RequestId request_id,
+                      const SkBitmap& image) override;
+
+ private:
+  const BitmapFetchedCallback callback_;
+
+  DISALLOW_COPY_AND_ASSIGN(AnswerImageObserver);
+};
+
+void AnswerImageObserver::OnImageChanged(
+    BitmapFetcherService::RequestId request_id,
+    const SkBitmap& image) {
+  DCHECK(!image.empty());
+  callback_.Run(image);
+}
+
+}  // namespace
+
+ChromeOmniboxClient::ChromeOmniboxClient(OmniboxEditController* controller,
+                                         Profile* profile)
     : controller_(controller),
-      profile_(profile) {}
+      profile_(profile),
+      request_id_(BitmapFetcherService::REQUEST_ID_INVALID) {}
+
+ChromeOmniboxClient::~ChromeOmniboxClient() {
+  BitmapFetcherService* image_service =
+      BitmapFetcherServiceFactory::GetForBrowserContext(profile_);
+  if (image_service)
+    image_service->CancelRequest(request_id_);
+}
 
-ChromeOmniboxClient::~ChromeOmniboxClient() {}
+scoped_ptr<AutocompleteProviderClient>
+ChromeOmniboxClient::CreateAutocompleteProviderClient() {
+  return make_scoped_ptr(new ChromeAutocompleteProviderClient(profile_));
+}
 
 bool ChromeOmniboxClient::CurrentPageExists() const {
   return (controller_->GetWebContents() != NULL);
@@ -110,6 +200,49 @@ void ChromeOmniboxClient::OnFocusChanged(
       controller_->GetWebContents())->OmniboxFocusChanged(state, reason);
 }
 
+void ChromeOmniboxClient::OnResultChanged(
+    const AutocompleteResult& result,
+    bool default_match_changed,
+    const base::Callback<void(const SkBitmap& bitmap)>& on_bitmap_fetched) {
+  if (chrome::IsInstantExtendedAPIEnabled() &&
+      ((default_match_changed && result.default_match() != result.end()) ||
+       (chrome::ShouldAllowPrefetchNonDefaultMatch() && !result.empty()))) {
+    InstantSuggestion prefetch_suggestion;
+    const AutocompleteMatch* match_to_prefetch = GetMatchToPrefetch(result);
+    if (match_to_prefetch) {
+      prefetch_suggestion.text = match_to_prefetch->contents;
+      prefetch_suggestion.metadata =
+          SearchProvider::GetSuggestMetadata(*match_to_prefetch);
+    }
+    // Send the prefetch suggestion unconditionally to the InstantPage. If
+    // there is no suggestion to prefetch, we need to send a blank query to
+    // clear the prefetched results.
+    SetSuggestionToPrefetch(prefetch_suggestion);
+  }
+
+  const auto match = std::find_if(
+      result.begin(), result.end(),
+      [](const AutocompleteMatch& current)->bool { return current.answer; });
+  if (match != result.end()) {
+    BitmapFetcherService* image_service =
+        BitmapFetcherServiceFactory::GetForBrowserContext(profile_);
+    if (image_service) {
+      image_service->CancelRequest(request_id_);
+      request_id_ = image_service->RequestImage(
+          match->answer->second_line().image_url(),
+          new AnswerImageObserver(
+              base::Bind(&ChromeOmniboxClient::OnBitmapFetched,
+                         base::Unretained(this), on_bitmap_fetched)));
+    }
+  }
+}
+
+void ChromeOmniboxClient::OnCurrentMatchChanged(
+    const AutocompleteMatch& match) {
+  if (!prerender::IsOmniboxEnabled(profile_))
+    DoPreconnect(match);
+}
+
 void ChromeOmniboxClient::OnURLOpenedFromOmnibox(OmniboxLog* log) {
   content::NotificationService::current()->Notify(
       chrome::NOTIFICATION_OMNIBOX_OPENED_URL,
@@ -138,6 +271,23 @@ void ChromeOmniboxClient::DoPrerender(
           container_bounds.size());
 }
 
+void ChromeOmniboxClient::DoPreconnect(const AutocompleteMatch& match) {
+  if (match.destination_url.SchemeIs(extensions::kExtensionScheme))
+    return;
+
+  // Warm up DNS Prefetch cache, or preconnect to a search service.
+  UMA_HISTOGRAM_ENUMERATION("Autocomplete.MatchType", match.type,
+                            AutocompleteMatchType::NUM_TYPES);
+  if (profile_->GetNetworkPredictor()) {
+    profile_->GetNetworkPredictor()->AnticipateOmniboxUrl(
+        match.destination_url,
+        predictors::AutocompleteActionPredictor::IsPreconnectable(match));
+  }
+  // We could prefetch the alternate nav URL, if any, but because there
+  // can be many of these as a user types an initial series of characters,
+  // the OS DNS cache could suffer eviction problems for minimal gain.
+}
+
 void ChromeOmniboxClient::SetSuggestionToPrefetch(
       const InstantSuggestion& suggestion) {
   DCHECK(chrome::IsInstantExtendedAPIEnabled());
@@ -155,3 +305,9 @@ void ChromeOmniboxClient::SetSuggestionToPrefetch(
       prerenderer->Prerender(suggestion);
   }
 }
+
+void ChromeOmniboxClient::OnBitmapFetched(const BitmapFetchedCallback& callback,
+                                          const SkBitmap& bitmap) {
+  request_id_ = BitmapFetcherService::REQUEST_ID_INVALID;
+  callback.Run(bitmap);
+}
diff --git a/chrome/browser/ui/omnibox/chrome_omnibox_client.h b/chrome/browser/ui/omnibox/chrome_omnibox_client.h
index f8e9635..6bc8643 100644
--- a/chrome/browser/ui/omnibox/chrome_omnibox_client.h
+++ b/chrome/browser/ui/omnibox/chrome_omnibox_client.h
@@ -6,7 +6,9 @@
 #define CHROME_BROWSER_UI_OMNIBOX_CHROME_OMNIBOX_CLIENT_H_
 
 #include "base/compiler_specific.h"
+#include "chrome/browser/bitmap_fetcher/bitmap_fetcher_service.h"
 #include "chrome/browser/ui/omnibox/omnibox_client.h"
+#include "chrome/common/instant_types.h"
 
 class OmniboxEditController;
 class Profile;
@@ -17,6 +19,8 @@ class ChromeOmniboxClient : public OmniboxClient {
   ~ChromeOmniboxClient() override;
 
   // OmniboxClient.
+  scoped_ptr<AutocompleteProviderClient>
+      CreateAutocompleteProviderClient() override;
   bool CurrentPageExists() const override;
   const GURL& GetURL() const override;
   bool IsInstantNTP() const override;
@@ -31,13 +35,27 @@ class ChromeOmniboxClient : public OmniboxClient {
   void OnInputStateChanged() override;
   void OnFocusChanged(OmniboxFocusState state,
                       OmniboxFocusChangeReason reason) override;
+  void OnResultChanged(const AutocompleteResult& result,
+                       bool default_match_changed,
+                       const base::Callback<void(const SkBitmap& bitmap)>&
+                           on_bitmap_fetched) override;
+  void OnCurrentMatchChanged(const AutocompleteMatch& match) override;
   void OnURLOpenedFromOmnibox(OmniboxLog* log) override;
   void DoPrerender(const AutocompleteMatch& match) override;
-  void SetSuggestionToPrefetch(const InstantSuggestion& suggestion) override;
+
+  // TODO(blundell): Make private once OmniboxEditModel no longer refers to it.
+  void DoPreconnect(const AutocompleteMatch& match) override;
 
  private:
+  // Sends the current SearchProvider suggestion to the Instant page if any.
+  void SetSuggestionToPrefetch(const InstantSuggestion& suggestion);
+
+  void OnBitmapFetched(const BitmapFetchedCallback& callback,
+                       const SkBitmap& bitmap);
+
   OmniboxEditController* controller_;
   Profile* profile_;
+  BitmapFetcherService::RequestId request_id_;
 
   DISALLOW_COPY_AND_ASSIGN(ChromeOmniboxClient);
 };
diff --git a/chrome/browser/ui/omnibox/omnibox_client.h b/chrome/browser/ui/omnibox/omnibox_client.h
index 86607a1..d55fbb2 100644
--- a/chrome/browser/ui/omnibox/omnibox_client.h
+++ b/chrome/browser/ui/omnibox/omnibox_client.h
@@ -6,10 +6,11 @@
 #define CHROME_BROWSER_UI_OMNIBOX_OMNIBOX_CLIENT_H_
 
 #include "base/basictypes.h"
-#include "chrome/common/instant_types.h"
+#include "components/omnibox/browser/autocomplete_provider_client.h"
 #include "components/omnibox/common/omnibox_focus_state.h"
 #include "ui/base/window_open_disposition.h"
 
+class AutocompleteResult;
 class GURL;
 class SessionID;
 class TemplateURL;
@@ -20,6 +21,8 @@ namespace content {
 class NavigationController;
 }
 
+typedef base::Callback<void(const SkBitmap& bitmap)> BitmapFetchedCallback;
+
 // Interface that allows the omnibox component to interact with its embedder
 // (e.g., getting information about the current page, retrieving objects
 // associated with the current tab, or performing operations that rely on such
@@ -28,6 +31,10 @@ class OmniboxClient {
  public:
   virtual ~OmniboxClient() {}
 
+  // Returns an AutocompleteProviderClient specific to the embedder context.
+  virtual scoped_ptr<AutocompleteProviderClient>
+      CreateAutocompleteProviderClient() = 0;
+
   // Returns whether there is any associated current page.  For example, during
   // startup or shutdown, the omnibox may exist but have no attached page.
   virtual bool CurrentPageExists() const = 0;
@@ -67,6 +74,17 @@ class OmniboxClient {
   virtual void OnFocusChanged(OmniboxFocusState state,
                               OmniboxFocusChangeReason reason) = 0;
 
+  // Called when the autocomplete result has changed. If the embedder supports
+  // fetching of bitmaps for URLs (not all embedders do), |on_bitmap_fetched|
+  // will be called when the bitmap has been fetched.
+  virtual void OnResultChanged(
+      const AutocompleteResult& result,
+      bool default_match_changed,
+      const BitmapFetchedCallback& on_bitmap_fetched) = 0;
+
+  // Called when the current autocomplete match has changed.
+  virtual void OnCurrentMatchChanged(const AutocompleteMatch& match) = 0;
+
   // Called to notify clients that a URL was opened from the omnibox.
   // TODO(blundell): Eliminate this method in favor of having all //chrome-
   // level listeners of the OMNIBOX_OPENED_URL instead observe OmniboxEditModel.
@@ -79,8 +97,10 @@ class OmniboxClient {
   // Performs prerendering for |match|.
   virtual void DoPrerender(const AutocompleteMatch& match) = 0;
 
-  // Sends the current SearchProvider suggestion to the Instant page if any.
-  virtual void SetSuggestionToPrefetch(const InstantSuggestion& suggestion) = 0;
+  // Performs preconnection for |match|.
+  // TODO(blundell): Remove from this interface once OmniboxEditModel no
+  // longer refers to it.
+  virtual void DoPreconnect(const AutocompleteMatch& match) = 0;
 };
 
 #endif  // CHROME_BROWSER_UI_OMNIBOX_OMNIBOX_CLIENT_H_
diff --git a/chrome/browser/ui/omnibox/omnibox_controller.cc b/chrome/browser/ui/omnibox/omnibox_controller.cc
index ad4e0d7..04a55a2 100644
--- a/chrome/browser/ui/omnibox/omnibox_controller.cc
+++ b/chrome/browser/ui/omnibox/omnibox_controller.cc
@@ -5,113 +5,28 @@
 #include "chrome/browser/ui/omnibox/omnibox_controller.h"
 
 #include "base/metrics/histogram.h"
-#include "chrome/browser/autocomplete/chrome_autocomplete_provider_client.h"
-#include "chrome/browser/bitmap_fetcher/bitmap_fetcher_service_factory.h"
-#include "chrome/browser/net/predictor.h"
-#include "chrome/browser/predictors/autocomplete_action_predictor.h"
-#include "chrome/browser/prerender/prerender_field_trial.h"
-#include "chrome/browser/prerender/prerender_manager.h"
-#include "chrome/browser/prerender/prerender_manager_factory.h"
-#include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/search/search.h"
+#include "chrome/browser/ui/omnibox/omnibox_client.h"
 #include "chrome/browser/ui/omnibox/omnibox_edit_controller.h"
 #include "chrome/browser/ui/omnibox/omnibox_edit_model.h"
 #include "chrome/browser/ui/omnibox/omnibox_popup_model.h"
-#include "chrome/common/instant_types.h"
 #include "components/omnibox/browser/autocomplete_classifier.h"
 #include "components/omnibox/browser/autocomplete_match.h"
 #include "components/omnibox/browser/omnibox_popup_view.h"
-#include "components/omnibox/browser/search_provider.h"
 #include "components/search/search.h"
-#include "extensions/common/constants.h"
 #include "ui/gfx/geometry/rect.h"
 
-namespace {
-
-// Returns the AutocompleteMatch that the InstantController should prefetch, if
-// any.
-//
-// The SearchProvider may mark some suggestions to be prefetched based on
-// instructions from the suggest server. If such a match ranks sufficiently
-// highly or if kAllowPrefetchNonDefaultMatch field trial is enabled, we'll
-// return it.
-//
-// If the kAllowPrefetchNonDefaultMatch field trial is enabled we return the
-// prefetch suggestion even if it is not the default match. Otherwise we only
-// care about matches that are the default or the second entry in the dropdown
-// (which can happen for non-default matches when a top verbatim match is
-// shown); for other matches, we think the likelihood of the user selecting
-// them is low enough that prefetching isn't worth doing.
-const AutocompleteMatch* GetMatchToPrefetch(const AutocompleteResult& result) {
-  if (chrome::ShouldAllowPrefetchNonDefaultMatch()) {
-    const AutocompleteResult::const_iterator prefetch_match = std::find_if(
-        result.begin(), result.end(), SearchProvider::ShouldPrefetch);
-    return prefetch_match != result.end() ? &(*prefetch_match) : NULL;
-  }
-
-  // If the default match should be prefetched, do that.
-  const auto default_match = result.default_match();
-  if ((default_match != result.end()) &&
-      SearchProvider::ShouldPrefetch(*default_match))
-    return &(*default_match);
-
-  // Otherwise, if the top match is a verbatim match and the very next match
-  // is prefetchable, fetch that.
-  if (result.TopMatchIsStandaloneVerbatimMatch() && (result.size() > 1) &&
-      SearchProvider::ShouldPrefetch(result.match_at(1)))
-    return &result.match_at(1);
-
-  return NULL;
-}
-
-// Calls back to the OmniboxController when the requested image is downloaded.
-// This is a separate class instead of being implemented on OmniboxController
-// because BitmapFetcherService currently takes ownership of this object.
-// TODO(dschuyler): Make BitmapFetcherService use the more typical non-owning
-// ObserverList pattern and have OmniboxController implement the Observer call
-// directly.
-class AnswerImageObserver : public BitmapFetcherService::Observer {
- public:
-  explicit AnswerImageObserver(
-      const base::WeakPtr<OmniboxController>& controller)
-      : controller_(controller) {}
-
-  void OnImageChanged(BitmapFetcherService::RequestId request_id,
-                      const SkBitmap& image) override;
-
- private:
-  const base::WeakPtr<OmniboxController> controller_;
-  DISALLOW_COPY_AND_ASSIGN(AnswerImageObserver);
-};
-
-void AnswerImageObserver::OnImageChanged(
-    BitmapFetcherService::RequestId request_id,
-    const SkBitmap& image) {
-  DCHECK(!image.empty());
-  DCHECK(controller_);
-  controller_->SetAnswerBitmap(image);
-}
-
-}  // namespace
-
 OmniboxController::OmniboxController(OmniboxEditModel* omnibox_edit_model,
-                                     Profile* profile)
+                                     OmniboxClient* client)
     : omnibox_edit_model_(omnibox_edit_model),
-      profile_(profile),
+      client_(client),
       popup_(NULL),
       autocomplete_controller_(new AutocompleteController(
-          make_scoped_ptr(new ChromeAutocompleteProviderClient(profile)),
+          client_->CreateAutocompleteProviderClient(),
           this,
           AutocompleteClassifier::kDefaultOmniboxProviders)),
-      request_id_(BitmapFetcherService::REQUEST_ID_INVALID),
-      weak_ptr_factory_(this) {
-}
+      weak_ptr_factory_(this) {}
 
 OmniboxController::~OmniboxController() {
-  BitmapFetcherService* image_service =
-      BitmapFetcherServiceFactory::GetForBrowserContext(profile_);
-  if (image_service)
-    image_service->CancelRequest(request_id_);
 }
 
 void OmniboxController::StartAutocomplete(
@@ -132,8 +47,6 @@ void OmniboxController::OnResultChanged(bool default_match_changed) {
     const AutocompleteResult::const_iterator match(result().default_match());
     if (match != result().end()) {
       current_match_ = *match;
-      if (!prerender::IsOmniboxEnabled(profile_))
-        DoPreconnect(*match);
       omnibox_edit_model_->OnCurrentMatchChanged();
     } else {
       InvalidateCurrentMatch();
@@ -151,37 +64,11 @@ void OmniboxController::OnResultChanged(bool default_match_changed) {
     omnibox_edit_model_->AcceptTemporaryTextAsUserText();
   }
 
-  if (chrome::IsInstantExtendedAPIEnabled() &&
-     ((default_match_changed && result().default_match() != result().end()) ||
-      (chrome::ShouldAllowPrefetchNonDefaultMatch() && !result().empty()))) {
-    InstantSuggestion prefetch_suggestion;
-    const AutocompleteMatch* match_to_prefetch = GetMatchToPrefetch(result());
-    if (match_to_prefetch) {
-      prefetch_suggestion.text = match_to_prefetch->contents;
-      prefetch_suggestion.metadata =
-          SearchProvider::GetSuggestMetadata(*match_to_prefetch);
-    }
-    // Send the prefetch suggestion unconditionally to the InstantPage. If
-    // there is no suggestion to prefetch, we need to send a blank query to
-    // clear the prefetched results.
-    omnibox_edit_model_->SetSuggestionToPrefetch(prefetch_suggestion);
-  }
-
-  for (AutocompleteResult::const_iterator match(result().begin());
-       match != result().end(); ++match) {
-    if (match->answer) {
-      BitmapFetcherService* image_service =
-          BitmapFetcherServiceFactory::GetForBrowserContext(profile_);
-      if (image_service) {
-        image_service->CancelRequest(request_id_);
-        request_id_ = image_service->RequestImage(
-            match->answer->second_line().image_url(),
-            new AnswerImageObserver(weak_ptr_factory_.GetWeakPtr()));
-      }
-      // We only fetch one answer image.
-      break;
-    }
-  }
+  // Note: The client outlives |this|, so bind a weak pointer to the callback
+  // passed in to eliminate the potential for crashes on shutdown.
+  client_->OnResultChanged(result(), default_match_changed,
+                           base::Bind(&OmniboxController::SetAnswerBitmap,
+                                      weak_ptr_factory_.GetWeakPtr()));
 }
 
 void OmniboxController::InvalidateCurrentMatch() {
@@ -194,23 +81,6 @@ void OmniboxController::ClearPopupKeywordMode() const {
     popup_->SetSelectedLineState(OmniboxPopupModel::NORMAL);
 }
 
-void OmniboxController::DoPreconnect(const AutocompleteMatch& match) {
-  if (!match.destination_url.SchemeIs(extensions::kExtensionScheme)) {
-    // Warm up DNS Prefetch cache, or preconnect to a search service.
-    UMA_HISTOGRAM_ENUMERATION("Autocomplete.MatchType", match.type,
-                              AutocompleteMatchType::NUM_TYPES);
-    if (profile_->GetNetworkPredictor()) {
-      profile_->GetNetworkPredictor()->AnticipateOmniboxUrl(
-          match.destination_url,
-          predictors::AutocompleteActionPredictor::IsPreconnectable(match));
-    }
-    // We could prefetch the alternate nav URL, if any, but because there
-    // can be many of these as a user types an initial series of characters,
-    // the OS DNS cache could suffer eviction problems for minimal gain.
-  }
-}
-
 void OmniboxController::SetAnswerBitmap(const SkBitmap& bitmap) {
-  request_id_ = BitmapFetcherService::REQUEST_ID_INVALID;
   popup_->SetAnswerBitmap(bitmap);
 }
diff --git a/chrome/browser/ui/omnibox/omnibox_controller.h b/chrome/browser/ui/omnibox/omnibox_controller.h
index a48ca9d..e332b181 100644
--- a/chrome/browser/ui/omnibox/omnibox_controller.h
+++ b/chrome/browser/ui/omnibox/omnibox_controller.h
@@ -9,7 +9,6 @@
 #include "base/compiler_specific.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/strings/string16.h"
-#include "chrome/browser/bitmap_fetcher/bitmap_fetcher_service.h"
 #include "components/omnibox/browser/autocomplete_controller.h"
 #include "components/omnibox/browser/autocomplete_controller_delegate.h"
 #include "components/omnibox/browser/autocomplete_match.h"
@@ -18,9 +17,9 @@ class AUtocompleteInput;
 struct AutocompleteMatch;
 class AutocompleteResult;
 class InstantController;
+class OmniboxClient;
 class OmniboxEditModel;
 class OmniboxPopupModel;
-class Profile;
 
 namespace gfx {
 class Rect;
@@ -37,7 +36,7 @@ class Rect;
 class OmniboxController : public AutocompleteControllerDelegate {
  public:
   OmniboxController(OmniboxEditModel* omnibox_edit_model,
-                    Profile* profile);
+                    OmniboxClient* client);
   ~OmniboxController() override;
 
   // The |current_url| field of input is only set for mobile ports.
@@ -71,18 +70,15 @@ class OmniboxController : public AutocompleteControllerDelegate {
     return autocomplete_controller_->result();
   }
 
-  // TODO(beaudoin): Make private once OmniboxEditModel no longer refers to it.
-  void DoPreconnect(const AutocompleteMatch& match);
-
+ private:
   // Stores the bitmap in the OmniboxPopupModel.
   void SetAnswerBitmap(const SkBitmap& bitmap);
 
- private:
   // Weak, it owns us.
   // TODO(beaudoin): Consider defining a delegate to ease unit testing.
   OmniboxEditModel* omnibox_edit_model_;
 
-  Profile* profile_;
+  OmniboxClient* client_;
 
   OmniboxPopupModel* popup_;
 
@@ -94,8 +90,6 @@ class OmniboxController : public AutocompleteControllerDelegate {
   // some time to extract these fields and use a tighter structure here.
   AutocompleteMatch current_match_;
 
-  BitmapFetcherService::RequestId request_id_;
-
   base::WeakPtrFactory<OmniboxController> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(OmniboxController);
diff --git a/chrome/browser/ui/omnibox/omnibox_controller_unittest.cc b/chrome/browser/ui/omnibox/omnibox_controller_unittest.cc
index bcb746a..2638f9e 100644
--- a/chrome/browser/ui/omnibox/omnibox_controller_unittest.cc
+++ b/chrome/browser/ui/omnibox/omnibox_controller_unittest.cc
@@ -3,14 +3,67 @@
 // found in the LICENSE file.
 
 #include "base/prefs/pref_service.h"
+#include "chrome/browser/autocomplete/chrome_autocomplete_provider_client.h"
+#include "chrome/browser/ui/omnibox/omnibox_client.h"
 #include "chrome/browser/ui/omnibox/omnibox_controller.h"
+#include "chrome/test/base/chrome_render_view_host_test_harness.h"
 #include "chrome/test/base/testing_profile.h"
 #include "components/omnibox/browser/autocomplete_controller.h"
 #include "components/omnibox/browser/autocomplete_provider.h"
-#include "content/public/test/test_browser_thread_bundle.h"
+#include "components/sessions/session_id.h"
+#include "content/public/browser/web_contents.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-class OmniboxControllerTest : public testing::Test {
+namespace {
+class TestOmniboxClient : public OmniboxClient {
+ public:
+  TestOmniboxClient(content::WebContents* web_contents, Profile* profile)
+      : web_contents_(web_contents), profile_(profile) {}
+  ~TestOmniboxClient() override {}
+
+  // OmniboxClient:
+  scoped_ptr<AutocompleteProviderClient> CreateAutocompleteProviderClient()
+      override {
+    return make_scoped_ptr(new ChromeAutocompleteProviderClient(profile_));
+  }
+  bool CurrentPageExists() const override { return true; }
+  const GURL& GetURL() const override { return web_contents_->GetURL(); }
+  bool IsInstantNTP() const override { return false; }
+  bool IsSearchResultsPage() const override { return false; }
+  bool IsLoading() const override { return false; }
+  bool IsPasteAndGoEnabled() const override { return false; }
+  content::NavigationController& GetNavigationController() const override {
+    return web_contents_->GetController();
+  }
+  const SessionID& GetSessionID() const override { return session_id_; }
+  bool ProcessExtensionKeyword(TemplateURL* template_url,
+                               const AutocompleteMatch& match,
+                               WindowOpenDisposition disposition) override {
+    return false;
+  }
+  void OnInputStateChanged() override {}
+  void OnFocusChanged(OmniboxFocusState state,
+                      OmniboxFocusChangeReason reason) override {}
+  void OnResultChanged(const AutocompleteResult& result,
+                       bool default_match_changed,
+                       const base::Callback<void(const SkBitmap& bitmap)>&
+                           on_bitmap_fetched) override {}
+  void OnCurrentMatchChanged(const AutocompleteMatch& match) override {}
+  void OnURLOpenedFromOmnibox(OmniboxLog* log) override {}
+  void DoPrerender(const AutocompleteMatch& match) override {}
+  void DoPreconnect(const AutocompleteMatch& match) override {}
+
+ private:
+  content::WebContents* web_contents_;
+  Profile* profile_;
+  SessionID session_id_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestOmniboxClient);
+};
+
+}  // namespace
+
+class OmniboxControllerTest : public ChromeRenderViewHostTestHarness {
  protected:
   OmniboxControllerTest();
   ~OmniboxControllerTest() override;
@@ -18,14 +71,17 @@ class OmniboxControllerTest : public testing::Test {
   void CreateController();
   void AssertProviders(int expected_providers);
 
-  PrefService* GetPrefs() { return profile_.GetPrefs(); }
+  PrefService* GetPrefs() { return profile()->GetPrefs(); }
   const AutocompleteController::Providers& GetAutocompleteProviders() const {
     return omnibox_controller_->autocomplete_controller()->providers();
   }
 
  private:
-  content::TestBrowserThreadBundle thread_bundle_;
-  TestingProfile profile_;
+  // ChromeRenderViewTestHarness:
+  void SetUp() override;
+  void TearDown() override;
+
+  scoped_ptr<TestOmniboxClient> omnibox_client_;
   scoped_ptr<OmniboxController> omnibox_controller_;
 
   DISALLOW_COPY_AND_ASSIGN(OmniboxControllerTest);
@@ -38,7 +94,8 @@ OmniboxControllerTest::~OmniboxControllerTest() {
 }
 
 void OmniboxControllerTest::CreateController() {
-  omnibox_controller_.reset(new OmniboxController(NULL, &profile_));
+  DCHECK(omnibox_client_);
+  omnibox_controller_.reset(new OmniboxController(NULL, omnibox_client_.get()));
 }
 
 // Checks that the list of autocomplete providers used by the OmniboxController
@@ -60,6 +117,19 @@ void OmniboxControllerTest::AssertProviders(int expected_providers) {
   ASSERT_EQ(0, expected_providers);
 }
 
+void OmniboxControllerTest::SetUp() {
+  ChromeRenderViewHostTestHarness::SetUp();
+
+  omnibox_client_.reset(new TestOmniboxClient(web_contents(), profile()));
+}
+
+void OmniboxControllerTest::TearDown() {
+  omnibox_controller_.reset();
+  omnibox_client_.reset();
+
+  ChromeRenderViewHostTestHarness::TearDown();
+}
+
 TEST_F(OmniboxControllerTest, CheckDefaultAutocompleteProviders) {
   CreateController();
   // First collect the basic providers.
diff --git a/chrome/browser/ui/omnibox/omnibox_edit_model.cc b/chrome/browser/ui/omnibox/omnibox_edit_model.cc
index 29ec82a..89a6265 100644
--- a/chrome/browser/ui/omnibox/omnibox_edit_model.cc
+++ b/chrome/browser/ui/omnibox/omnibox_edit_model.cc
@@ -20,7 +20,6 @@
 #include "chrome/browser/autocomplete/chrome_autocomplete_scheme_classifier.h"
 #include "chrome/browser/bookmarks/bookmark_model_factory.h"
 #include "chrome/browser/bookmarks/bookmark_stats.h"
-#include "chrome/browser/extensions/api/omnibox/omnibox_api.h"
 #include "chrome/browser/net/predictor.h"
 #include "chrome/browser/predictors/autocomplete_action_predictor.h"
 #include "chrome/browser/predictors/autocomplete_action_predictor_factory.h"
@@ -57,7 +56,6 @@
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/user_metrics.h"
-#include "extensions/common/constants.h"
 #include "ui/gfx/image/image.h"
 #include "url/url_util.h"
 
@@ -195,9 +193,9 @@ OmniboxEditModel::OmniboxEditModel(OmniboxView* view,
                                    OmniboxEditController* controller,
                                    scoped_ptr<OmniboxClient> client,
                                    Profile* profile)
-    : view_(view),
+    : client_(client.Pass()),
+      view_(view),
       controller_(controller),
-      client_(client.Pass()),
       focus_state_(OMNIBOX_FOCUS_NONE),
       focus_source_(INVALID),
       user_input_in_progress_(false),
@@ -210,7 +208,7 @@ OmniboxEditModel::OmniboxEditModel(OmniboxView* view,
       profile_(profile),
       in_revert_(false),
       allow_exact_keyword_match_(false) {
-  omnibox_controller_.reset(new OmniboxController(this, profile));
+  omnibox_controller_.reset(new OmniboxController(this, client_.get()));
 }
 
 OmniboxEditModel::~OmniboxEditModel() {
@@ -399,7 +397,7 @@ void OmniboxEditModel::OnChanged() {
         client_->DoPrerender(current_match);
       break;
     case AutocompleteActionPredictor::ACTION_PRECONNECT:
-      omnibox_controller_->DoPreconnect(current_match);
+      client_->DoPreconnect(current_match);
       break;
     case AutocompleteActionPredictor::ACTION_NONE:
       break;
@@ -1348,6 +1346,8 @@ void OmniboxEditModel::OnCurrentMatchChanged() {
 
   const AutocompleteMatch& match = omnibox_controller_->current_match();
 
+  client_->OnCurrentMatchChanged(match);
+
   // We store |keyword| and |is_keyword_hint| in temporary variables since
   // OnPopupDataChanged use their previous state to detect changes.
   base::string16 keyword;
@@ -1364,11 +1364,6 @@ void OmniboxEditModel::OnCurrentMatchChanged() {
   OnPopupDataChanged(inline_autocompletion, NULL, keyword, is_keyword_hint);
 }
 
-void OmniboxEditModel::SetSuggestionToPrefetch(
-    const InstantSuggestion& suggestion) {
-  client_->SetSuggestionToPrefetch(suggestion);
-}
-
 // static
 const char OmniboxEditModel::kCutOrCopyAllTextHistogram[] =
     "Omnibox.CutOrCopyAllText";
diff --git a/chrome/browser/ui/omnibox/omnibox_edit_model.h b/chrome/browser/ui/omnibox/omnibox_edit_model.h
index a8cf82e..0c6906e 100644
--- a/chrome/browser/ui/omnibox/omnibox_edit_model.h
+++ b/chrome/browser/ui/omnibox/omnibox_edit_model.h
@@ -11,7 +11,6 @@
 #include "base/strings/string16.h"
 #include "base/time/time.h"
 #include "chrome/browser/ui/omnibox/omnibox_controller.h"
-#include "chrome/common/instant_types.h"
 #include "components/metrics/proto/omnibox_event.pb.h"
 #include "components/omnibox/browser/autocomplete_controller_delegate.h"
 #include "components/omnibox/browser/autocomplete_input.h"
@@ -341,9 +340,6 @@ class OmniboxEditModel {
   // Called when the current match has changed in the OmniboxController.
   void OnCurrentMatchChanged();
 
-  // Sends the current SearchProvider suggestion to the Instant page if any.
-  void SetSuggestionToPrefetch(const InstantSuggestion& suggestion);
-
   // Name of the histogram tracking cut or copy omnibox commands.
   static const char kCutOrCopyAllTextHistogram[];
 
@@ -440,14 +436,16 @@ class OmniboxEditModel {
   // the view.
   void SetFocusState(OmniboxFocusState state, OmniboxFocusChangeReason reason);
 
+  // NOTE: |client_| must outlive |omnibox_controller_|, as the latter has a
+  // reference to the former.
+  scoped_ptr<OmniboxClient> client_;
+
   scoped_ptr<OmniboxController> omnibox_controller_;
 
   OmniboxView* view_;
 
   OmniboxEditController* controller_;
 
-  scoped_ptr<OmniboxClient> client_;
-
   OmniboxFocusState focus_state_;
 
   // Used to keep track whether the input currently in progress originated by
author	blundell <blundell@chromium.org>	2015-07-29 11:16:20 -0700
committer	Commit bot <commit-bot@chromium.org>	2015-07-29 18:16:58 +0000
commit	3c100cee848bd29360dad2cba9a7013910dacafb (patch)
tree	ef1449284575d5fa1ca87eaa7c94c9126ec726a3
parent	587c07234226e832a986b7ed055d8c918415695e (diff)
download	chromium_src-3c100cee848bd29360dad2cba9a7013910dacafb.zip chromium_src-3c100cee848bd29360dad2cba9a7013910dacafb.tar.gz chromium_src-3c100cee848bd29360dad2cba9a7013910dacafb.tar.bz2