Instant API: tell page whether the browser is capturing key strokes.

chrome/browser/ui/omnibox/omnibox_edit_model.cc

Index: chrome/browser/ui/omnibox/omnibox_edit_model.cc
diff --git a/chrome/browser/ui/omnibox/omnibox_edit_model.cc b/chrome/browser/ui/omnibox/omnibox_edit_model.cc
index 668d9a820ea8ff5b715b1dba83d0cf0d3f6ab002..862d74d27802d56cf8dd64e69eaa58463002ba8a 100644
--- a/chrome/browser/ui/omnibox/omnibox_edit_model.cc
+++ b/chrome/browser/ui/omnibox/omnibox_edit_model.cc
@@ -136,7 +136,7 @@ const OmniboxEditModel::State OmniboxEditModel::GetStateForTabSwitch() {
 }
 
 void OmniboxEditModel::RestoreState(const State& state) {
-  SetCaretVisibility(state.is_caret_visible);
+  SetCaretVisibilityInternal(state.is_caret_visible, FOCUS_CHANGE_TAB_SWITCH);
   // Restore any user editing.
   if (state.user_input_in_progress) {
     // NOTE: Be sure and set keyword-related state BEFORE invoking
@@ -715,11 +715,19 @@ const AutocompleteResult& OmniboxEditModel::result() const {
 void OmniboxEditModel::OnSetFocus(bool control_down) {
   has_focus_ = true;
   control_key_state_ = control_down ? DOWN_WITHOUT_CHANGE : UP;
-  // Restore caret visibility whenever the user focuses back into the omnibox.
+  // If the omnibox lost focus while the caret was hidden and then regained
+  // focus, OnSetFocus() is called and should restore visibility. Note that
+  // focus can be regained without an accompanying call to
+  // OmniboxView::SetFocus(), e.g. by tabbing in.
   SetCaretVisibility(true);
 
-  if (InstantController* instant = controller_->GetInstant())
-    instant->OmniboxGotFocus();
+  // The SetCaretVisibility call above will already send an update to the
+  // InstantController in some cases but we still need this here since it won't
+  // send an update if the caret was already visible.

[Peter Kasting, 2012/12/08 01:12:21]

But in that case why do we need to send an update?  The InstantController
shouldn't care, unless it doesn't realize the caret is already visible, in which
case you have a bug somewhere else.  No?

[samarth, 2012/12/08 01:51:34]

It's because InstantController also needs to know whether the omnibox has focus
or not (for other reasons), as in it cares it cares when the state changes from
FOCUS_NONE to FOCUS_VISIBLE.

[Peter Kasting, 2012/12/08 02:07:52]

Perhaps we should move this to the top of the function and call it only if
!has_focus_.

It would also be good to change the comment to something like:

"SetCaretVisibility() will notify the InstantController if caret visibility
changes while we have focus, but it also needs to know when we actually gain
focus to begin with, so we have to manually notify here."

This also makes me wonder about a larger change: combine the caret visibility
and focus tracking bools in the edit to a single caret visibility member that
uses the FOCUS_VISIBLE, FOCUS_INVISIBLE, FOCUS_NONE values.  Do this right and I
suspect we eliminate or at least clarify a lot of separate calls to the instant
controller.

[samarth, 2012/12/10 23:45:00]

Done.

+  if (InstantController* instant = controller_->GetInstant()) {
+    instant->OmniboxFocusChanged(FOCUS_VISIBLE,
+                                 FOCUS_CHANGE_EXPLICIT, NULL);
+  }
 
   content::WebContents* web_contents = controller_->GetWebContents();
   if (web_contents) {
@@ -736,17 +744,16 @@ void OmniboxEditModel::OnSetFocus(bool control_down) {
 }
 
 void OmniboxEditModel::SetCaretVisibility(bool visible) {
-  if (has_focus_ && visible != is_caret_visible_) {
-    is_caret_visible_ = visible;
-    view_->ApplyCaretVisibility();
-  }
+  SetCaretVisibilityInternal(visible, FOCUS_CHANGE_EXPLICIT);
 }
 
 void OmniboxEditModel::OnWillKillFocus(gfx::NativeView view_gaining_focus) {
   SetInstantSuggestion(InstantSuggestion());
 
-  if (InstantController* instant = controller_->GetInstant())
-    instant->OmniboxLostFocus(view_gaining_focus);
+  if (InstantController* instant = controller_->GetInstant()) {
+    instant->OmniboxFocusChanged(FOCUS_NONE, FOCUS_CHANGE_EXPLICIT,
+                                 view_gaining_focus);
+  }
 
   // TODO(jered): Rip this out along with StartZeroSuggest.
   autocomplete_controller_->StopZeroSuggest();
@@ -952,7 +959,7 @@ bool OmniboxEditModel::OnAfterPossibleChange(const string16& old_text,
   // Restore caret visibility whenever the user changes text or selection in the
   // omnibox.
   if (text_differs || selection_differs)
-    SetCaretVisibility(true);
+    SetCaretVisibilityInternal(true, FOCUS_CHANGE_TYPING);
 
   // Modifying the selection counts as accepting the autocompleted text.
   const bool user_text_changed =
@@ -1288,3 +1295,16 @@ void OmniboxEditModel::ClassifyStringForPasteAndGo(
   AutocompleteClassifierFactory::GetForProfile(profile_)->Classify(text,
       string16(), false, false, match, alternate_nav_url);
 }
+
+void OmniboxEditModel::SetCaretVisibilityInternal(
+    bool visible,
+    OmniboxFocusChangeReason reason) {
+  if (has_focus_ && visible != is_caret_visible_) {
+    if (InstantController* instant = controller_->GetInstant()) {
+      instant->OmniboxFocusChanged(visible ? FOCUS_VISIBLE : FOCUS_INVISIBLE,
+                                   reason, NULL);
+    }
+    is_caret_visible_ = visible;
+    view_->ApplyCaretVisibility();
+  }
+}