Omnibox refactor, moved OnResultChanged to OmniboxController

chrome/browser/ui/omnibox/omnibox_controller.cc

Index: chrome/browser/ui/omnibox/omnibox_controller.cc
diff --git a/chrome/browser/ui/omnibox/omnibox_controller.cc b/chrome/browser/ui/omnibox/omnibox_controller.cc
index d99214ce4e9fac77fd31b52233060409a05564be..0adbe66e520552aea1ab1eca8065ddbea40d1edf 100644
--- a/chrome/browser/ui/omnibox/omnibox_controller.cc
+++ b/chrome/browser/ui/omnibox/omnibox_controller.cc
@@ -4,15 +4,31 @@
 
 #include "chrome/browser/ui/omnibox/omnibox_controller.h"
 
+#include "base/metrics/histogram.h"
 #include "chrome/browser/autocomplete/autocomplete_classifier.h"
 #include "chrome/browser/autocomplete/autocomplete_controller.h"
+#include "chrome/browser/autocomplete/autocomplete_match.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_edit_controller.h"
 #include "chrome/browser/ui/omnibox/omnibox_edit_model.h"
+#include "chrome/browser/ui/omnibox/omnibox_popup_model.h"
+#include "chrome/browser/ui/omnibox/omnibox_popup_view.h"
+#include "chrome/browser/ui/search/instant_controller.h"
+#include "extensions/common/constants.h"
+
+using predictors::AutocompleteActionPredictor;
 
 
 OmniboxController::OmniboxController(OmniboxEditModel* omnibox_edit_model,
                                      Profile* profile)
-    : omnibox_edit_model_(omnibox_edit_model) {
+    : omnibox_edit_model_(omnibox_edit_model),
+      profile_(profile) {
   autocomplete_controller_.reset(new AutocompleteController(profile, this,
       chrome::IsInstantExtendedAPIEnabled() ?
           AutocompleteClassifier::kInstantExtendedOmniboxProviders :
@@ -23,5 +39,105 @@ OmniboxController::~OmniboxController() {
 }
 
 void OmniboxController::OnResultChanged(bool default_match_changed) {
-  omnibox_edit_model_->OnResultChanged(default_match_changed);
+  // TODO(beaudoin): There should be no need to access the popup when using
+  //     instant extended, remove this reference.

[Mathieu, 2013/05/03 19:09:02]

nit: necessary indent?

[beaudoin, 2013/05/04 03:06:51]

Done.

+  const bool was_open = popup_->IsOpen();
+  if (default_match_changed) {
+    string16 inline_autocomplete_text;
+    string16 keyword;
+    bool is_keyword_hint = false;
+    const AutocompleteResult& result = this->result();
+    const AutocompleteResult::const_iterator match(result.default_match());
+    if (match != result.end()) {

[Mathieu, 2013/05/03 19:09:02]

Generally, don't hesitate to add comments if you feel you understand all this
code better now. I've always found these methods opaque and I would love more
comments such as "At this point we are in state X and this will do Y".

[beaudoin, 2013/05/04 03:06:51]

Did some digging and added some comment. Also refactored some stuff as I was
doing this. Let me know if you think other parts should be clarified.

Done.

+      if ((match->inline_autocomplete_offset != string16::npos) &&
+          (match->inline_autocomplete_offset <
+           match->fill_into_edit.length())) {
+        inline_autocomplete_text =
+            match->fill_into_edit.substr(match->inline_autocomplete_offset);
+      }
+
+      if (!prerender::IsOmniboxEnabled(profile_))
+        DoPreconnect(*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.
+
+      match->GetKeywordUIState(profile_, &keyword, &is_keyword_hint);
+    }
+
+    popup_->OnResultChanged();
+    omnibox_edit_model_->OnPopupDataChanged(inline_autocomplete_text, NULL,
+                                            keyword, is_keyword_hint);
+  } else {
+    popup_->OnResultChanged();
+  }
+
+  if (popup_->IsOpen()) {
+    omnibox_edit_model_->OnPopupBoundsChanged(
+        popup_->view()->GetTargetBounds());
+
+    InstantController* instant =
+        omnibox_edit_model_->controller()->GetInstant();
+    if (instant && !omnibox_edit_model_->in_revert()) {
+      instant->HandleAutocompleteResults(
+          *autocomplete_controller()->providers());
+    }
+  } else if (was_open) {
+    // Accepts the temporary text as the user text, because it makes little
+    // sense to have temporary text when the popup is closed.
+    omnibox_edit_model_->AcceptTemporaryTextAsUserText();
+  }
+}
+
+void OmniboxController::ClearPopupKeywordMode() const {
+  if (popup_->IsOpen() &&
+      popup_->selected_line_state() == OmniboxPopupModel::KEYWORD)
+    popup_->SetSelectedLineState(OmniboxPopupModel::NORMAL);
+}
+
+void OmniboxController::InfoForCurrentSelection(AutocompleteMatch* match,
+                                                GURL* alternate_nav_url) const {
+  DCHECK(match != NULL);
+  const AutocompleteResult& result = autocomplete_controller_->result();
+  if (!autocomplete_controller_->done()) {
+    // It's technically possible for |result| to be empty if no provider returns
+    // a synchronous result but the query has not completed synchronously;
+    // pratically, however, that should never actually happen.
+    if (result.empty())
+      return;
+    // The user cannot have manually selected a match, or the query would have
+    // stopped.  So the default match must be the desired selection.
+    *match = *result.default_match();
+  } else {
+    CHECK(popup_->IsOpen());
+    // If there are no results, the popup should be closed (so we should have
+    // failed the CHECK above), and URLsForDefaultMatch() should have been
+    // called instead.
+    CHECK(!result.empty());
+    CHECK(popup_->selected_line() < result.size());
+    *match = result.match_at(popup_->selected_line());
+  }
+  if (alternate_nav_url && popup_->manually_selected_match().empty())
+    *alternate_nav_url = result.alternate_nav_url();
+}
+
+const AutocompleteResult& OmniboxController::result() const {
+  return autocomplete_controller_->result();
+}
+
+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,
+                              AutocompleteMatch::NUM_TYPES);
+    if (profile_->GetNetworkPredictor()) {
+      profile_->GetNetworkPredictor()->AnticipateOmniboxUrl(
+          match.destination_url,
+          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.
+  }
 }