Interpret '?' and Ctrl-K or Ctrl-E as putting omnibox in keyword search mode for Default Search Pro…

components/omnibox/browser/omnibox_edit_model.cc

Index: components/omnibox/browser/omnibox_edit_model.cc
diff --git a/components/omnibox/browser/omnibox_edit_model.cc b/components/omnibox/browser/omnibox_edit_model.cc
index 87302727e29dab8cfd900d491954909f218c6b3c..d9a9a83fe090238667cf963373305dbc4c450612 100644
--- a/components/omnibox/browser/omnibox_edit_model.cc
+++ b/components/omnibox/browser/omnibox_edit_model.cc
@@ -45,6 +45,7 @@ using bookmarks::BookmarkModel;
 using metrics::OmniboxEventProto;
 
 
+

[Peter Kasting, 2016/04/08 00:39:56]

Nit: Don't add this

[Tom (Use chromium acct), 2016/04/12 20:03:05]

Done.

 // Helpers --------------------------------------------------------------------
 
 namespace {
@@ -149,6 +150,7 @@ OmniboxEditModel::State::State(bool user_input_in_progress,
                                const base::string16& gray_text,
                                const base::string16& keyword,
                                bool is_keyword_hint,
+                               EnteredKeywordModeMethod keyword_entered_method,
                                bool url_replacement_enabled,
                                OmniboxFocusState focus_state,
                                FocusSource focus_source,
@@ -158,6 +160,7 @@ OmniboxEditModel::State::State(bool user_input_in_progress,
       gray_text(gray_text),
       keyword(keyword),
       is_keyword_hint(is_keyword_hint),
+      keyword_entered_method(keyword_entered_method),
       url_replacement_enabled(url_replacement_enabled),
       focus_state(focus_state),
       focus_source(focus_source),
@@ -183,6 +186,7 @@ OmniboxEditModel::OmniboxEditModel(OmniboxView* view,
       user_input_in_progress_(false),
       user_input_since_focus_(true),
       just_deleted_text_(false),
+      just_cleared_keyword_(false),
       has_temporary_text_(false),
       paste_state_(NONE),
       control_key_state_(UP),
@@ -217,7 +221,7 @@ const OmniboxEditModel::State OmniboxEditModel::GetStateForTabSwitch() {
                         user_input_in_progress_);
   return State(
       user_input_in_progress_, user_text_, view_->GetGrayTextAutocompletion(),
-      keyword_, is_keyword_hint_,
+      keyword_, is_keyword_hint_, keyword_entered_method_,
       controller_->GetToolbarModel()->url_replacement_enabled(),
       focus_state_, focus_source_, input_);
 }
@@ -246,6 +250,7 @@ void OmniboxEditModel::RestoreState(const State* state) {
     // DisplayTextFromUserText(), as its result depends upon this state.
     keyword_ = state->keyword;
     is_keyword_hint_ = state->is_keyword_hint;
+    keyword_entered_method_ = state->keyword_entered_method;
     view_->SetUserText(state->user_text,
         DisplayTextFromUserText(state->user_text), false);
     view_->SetGrayTextAutocompletion(state->gray_text);
@@ -328,6 +333,15 @@ bool OmniboxEditModel::CommitSuggestedText() {
 }
 
 void OmniboxEditModel::OnChanged() {
+  // If the user input a "?", put them into keyword mode.
+  if (!just_cleared_keyword_ && !just_deleted_text_) {
+    if (user_text_ == base::ASCIIToUTF16("?")) {

[Peter Kasting, 2016/04/08 00:39:56]

Nit: Combine these ifs

[Tom (Use chromium acct), 2016/04/12 20:03:05]

Done.

+      user_text_ = base::string16();
+      SetKeywordWithDefaultSearchProvider(
+          ENTERED_KEYWORD_MODE_VIA_QUESTION_MARK);
+    }
+  }
+
   // Hide any suggestions we might be showing.
   view_->SetGrayTextAutocompletion(base::string16());
 
@@ -625,6 +639,23 @@ void OmniboxEditModel::AcceptInput(WindowOpenDisposition disposition,
                    popup_model()->selected_line());
 }
 
+void OmniboxEditModel::SetKeywordWithDefaultSearchProvider(
+    EnteredKeywordModeMethod keyword_entered_method) {
+  autocomplete_controller()->Stop(false);
+
+  keyword_ = client_->GetTemplateURLService()->
+      GetDefaultSearchProvider()->keyword();
+

[Peter Kasting, 2016/04/08 00:39:56]

Nit: I'd remove this blank line

[Tom (Use chromium acct), 2016/04/12 20:03:05]

Done.

+  is_keyword_hint_ = false;
+  keyword_entered_method_ = keyword_entered_method;
+
+  view_->OnTemporaryTextMaybeChanged(user_text_, false, true);
+
+  UMA_HISTOGRAM_ENUMERATION(kEnteredKeywordModeHistogram,
+                            keyword_entered_method,
+                            ENTERED_KEYWORD_MODE_NUM_ITEMS);
+}
+
 void OmniboxEditModel::OpenMatch(AutocompleteMatch match,
                                  WindowOpenDisposition disposition,
                                  const GURL& alternate_nav_url,
@@ -788,6 +819,7 @@ bool OmniboxEditModel::AcceptKeyword(EnteredKeywordModeMethod entered_method) {
 
   autocomplete_controller()->Stop(false);
   is_keyword_hint_ = false;
+  keyword_entered_method_ = entered_method;
 
   if (popup_model() && popup_model()->IsOpen())
     popup_model()->SetSelectedLineState(OmniboxPopupModel::KEYWORD);
@@ -844,6 +876,8 @@ void OmniboxEditModel::AcceptTemporaryTextAsUserText() {
 
 void OmniboxEditModel::ClearKeyword() {
   autocomplete_controller()->Stop(false);
+  view_->OnBeforePossibleChange();

[Peter Kasting, 2016/04/08 00:39:56]

This will result in calling this in the "state 4" case below, which is bad.

[Tom (Use chromium acct), 2016/04/12 20:03:05]

Done.
I also moved view_->OnAfterPossibleChange(false) so it isn't called during state
4, can you confirm this is correct?

+  just_cleared_keyword_ = true;
 
   // While we're always in keyword mode upon reaching here, sometimes we've just
   // toggled in via space or tab, and sometimes we're on a non-toggled line
@@ -854,6 +888,30 @@ void OmniboxEditModel::ClearKeyword() {
 
   omnibox_controller_->ClearPopupKeywordMode();
 
+  // If we entered keyword mode in a special way like using a keyboard shortcut
+  // or typing a question mark in a blank omnibox, don't restore the keyword.
+  const char* clear_text = nullptr;

[Peter Kasting, 2016/04/08 00:39:56]

Use a std::string in preference to char*.

Or better yet, avoid the switch and the temp by restructuring like this:

  if (keyword_mode_entry_method == KeywordModeEntryMethod::KEYBOARD_SHORTCUT ||
      keyword_mode_entry_method == KeywordModeEntryMethod::QUESTION_MARK) {
    if (keyword_mode_entry_method == KeywordModeEntryMethod::QUESTION_MARK)
      view_->SetWindowTextAndCaretPos('?' + view_->GetText(), 1, false, false);
    else
      view_->SetWindowTextAndCaretPos(view_->GetText(), 0, false, false);
    ...
  }

[Tom (Use chromium acct), 2016/04/12 20:03:05]

Done.

+  switch (keyword_entered_method_) {
+    case ENTERED_KEYWORD_MODE_VIA_KEYBOARD_SHORTCUT:
+      clear_text = "";
+      break;
+    case ENTERED_KEYWORD_MODE_VIA_QUESTION_MARK:
+      clear_text = "?";
+      break;
+    default:
+      break;
+  }
+  if (clear_text) {
+    view_->SetWindowTextAndCaretPos(
+        base::ASCIIToUTF16(clear_text) + view_->GetText(), strlen(clear_text),
+        false, false);
+    keyword_.clear();
+    is_keyword_hint_ = false;
+    view_->OnAfterPossibleChange(false);
+    just_cleared_keyword_ = false;
+    return;
+  }
+
   // There are several possible states we could have been in before the user hit
   // backspace or shift-tab to enter this function:
   // (1) was_toggled_into_keyword_mode == false, has_temporary_text_ == false
@@ -919,8 +977,9 @@ void OmniboxEditModel::ClearKeyword() {
         false, false);
     keyword_.clear();
     is_keyword_hint_ = false;
-    view_->OnAfterPossibleChange(false);
   }
+  view_->OnAfterPossibleChange(false);
+  just_cleared_keyword_ = false;
 }
 
 void OmniboxEditModel::OnSetFocus(bool control_down) {
@@ -1155,6 +1214,7 @@ bool OmniboxEditModel::OnAfterPossibleChange(const base::string16& old_text,
                                              size_t selection_end,
                                              bool selection_differs,
                                              bool text_differs,
+                                             bool keyword_differs,
                                              bool just_deleted_text,
                                              bool allow_keyword_ui_change) {
   // Update the paste state as appropriate: if we're just finishing a paste
@@ -1194,7 +1254,7 @@ bool OmniboxEditModel::OnAfterPossibleChange(const base::string16& old_text,
     control_key_state_ = DOWN_WITH_CHANGE;
 
   if (!user_text_changed)
-    return false;
+    return keyword_differs;
 
   // If the user text has not changed, we do not want to change the model's
   // state associated with the text.  Otherwise, we can get surprising behavior