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

components/omnibox/browser/omnibox_edit_model.h

Index: components/omnibox/browser/omnibox_edit_model.h
diff --git a/components/omnibox/browser/omnibox_edit_model.h b/components/omnibox/browser/omnibox_edit_model.h
index de6ff02da4bedd70185ef080522c523a1a475c2d..8256f04e033eb83c6d43a170e98d361a16f8277e 100644
--- a/components/omnibox/browser/omnibox_edit_model.h
+++ b/components/omnibox/browser/omnibox_edit_model.h
@@ -37,11 +37,13 @@ class Rect;
 
 // Reasons why the Omnibox could change into keyword mode.
 // These numeric values are used in UMA logs; do not change them.
-enum EnteredKeywordModeMethod {
-  ENTERED_KEYWORD_MODE_VIA_TAB = 0,
-  ENTERED_KEYWORD_MODE_VIA_SPACE_AT_END = 1,
-  ENTERED_KEYWORD_MODE_VIA_SPACE_IN_MIDDLE = 2,
-  ENTERED_KEYWORD_MODE_NUM_ITEMS
+enum class KeywordModeEntryMethod {
+  TAB = 0,
+  SPACE_AT_END = 1,
+  SPACE_IN_MIDDLE = 2,
+  KEYBOARD_SHORTCUT = 3,
+  QUESTION_MARK = 4,
+  NUM_ITEMS,
 };
 
 class OmniboxEditModel {
@@ -60,6 +62,7 @@ class OmniboxEditModel {
           const base::string16& gray_text,
           const base::string16& keyword,
           bool is_keyword_hint,
+          KeywordModeEntryMethod keyword_mode_entry_method,
           bool url_replacement_enabled,
           OmniboxFocusState focus_state,
           FocusSource focus_source,
@@ -72,12 +75,26 @@ class OmniboxEditModel {
     const base::string16 gray_text;
     const base::string16 keyword;
     const bool is_keyword_hint;
+    KeywordModeEntryMethod keyword_mode_entry_method;
     bool url_replacement_enabled;
     OmniboxFocusState focus_state;
     FocusSource focus_source;
     const AutocompleteInput autocomplete_input;
   };
 
+  // Used by |OnAfterPossibleChange|.
+  struct TextStateChange {

[Peter Kasting, 2016/06/04 02:17:18]

Nit: I think this should be defined in OmniboxView instead of here.  It seems
like everything else related to this, including the method that actually
computes it, is defined in that class already, and while its consumer is in this
class, all its concepts relate to things in the view.

Assuming you follow my name suggestion there, I would then name this
StateChanges and document like "Represents the changes between two State
objects.  This is used by the model to determine how its internal state should
be updated after the view state changes.  See
OmniboxEditModel::OnAfterPossibleChange()."

[Tom (Use chromium acct), 2016/06/04 20:39:09]

Done, but unfortunately you cannot forward-declare nested classes, so I instead
moved this to OmniboxViewStateChanges

+    // |old_text| and |new_text| are not owned.
+    const base::string16* old_text;
+    const base::string16* new_text;
+    size_t new_sel_start;
+    size_t new_sel_end;
+    bool selection_differs;
+    bool text_differs;
+    bool keyword_differs;
+    bool just_deleted_text;
+  };
+
   OmniboxEditModel(OmniboxView* view,
                    OmniboxEditController* controller,
                    std::unique_ptr<OmniboxClient> client);
@@ -237,10 +254,14 @@ class OmniboxEditModel {
   }
 
   // Accepts the current keyword hint as a keyword. It always returns true for
-  // caller convenience. |entered_method| indicates how the use entered
-  // keyword mode. This parameter is only used for metrics/logging; it's not
-  // used to change user-visible behavior.
-  bool AcceptKeyword(EnteredKeywordModeMethod entered_method);
+  // caller convenience. |entered_method| indicates how the user entered
+  // keyword mode.
+  bool AcceptKeyword(KeywordModeEntryMethod entry_method);
+
+  // Sets the current keyword to that of the user's default search provider and
+  // updates the view so the user sees the keyword chip in the omnibox.
+  void EnterKeywordModeForDefaultSearchProvider(
+      KeywordModeEntryMethod entry_method);
 
   // Accepts the current temporary text as the user text.
   void AcceptTemporaryTextAsUserText();
@@ -324,19 +345,13 @@ class OmniboxEditModel {
   // Called by the OmniboxView after something changes, with details about what
   // state changes occured.  Updates internal state, updates the popup if
   // necessary, and returns true if any significant changes occurred.  Note that
-  // |text_differs| may be set even if |old_text| == |new_text|, e.g. if we've
-  // just committed an IME composition.
+  // |text_change.text_differs| may be set even if |text_change.old_text| ==
+  // |text_change.new_text|, e.g. if we've just committed an IME composition.
   //
   // If |allow_keyword_ui_change| is false then the change should not affect
   // keyword ui state, even if the text matches a keyword exactly. This value
   // may be false when the user is composing a text with an IME.
-  bool OnAfterPossibleChange(const base::string16& old_text,
-                             const base::string16& new_text,
-                             size_t selection_start,
-                             size_t selection_end,
-                             bool selection_differs,
-                             bool text_differs,
-                             bool just_deleted_text,
+  bool OnAfterPossibleChange(const TextStateChange& text_change,
                              bool allow_keyword_ui_change);
 
   // Called when the current match has changed in the OmniboxController.
@@ -547,6 +562,11 @@ class OmniboxEditModel {
   // keyword_ to show a "Press <tab> to search" sort of hint.
   bool is_keyword_hint_;
 
+  // Indicates how the user entered keyword mode if the user is actually in
+  // keyword mode.  Otherwise, the value of this variable is undefined.  This
+  // is used to restore the user's search terms upon a call to ClearKeyword().
+  KeywordModeEntryMethod keyword_mode_entry_method_;
+
   // This is needed to properly update the SearchModel state when the user
   // presses escape.
   bool in_revert_;