Fix: Cursor missing in omnibox after entering a alphabet in NTP 'Search box'

ui/views/controls/textfield/textfield.cc

Index: ui/views/controls/textfield/textfield.cc
diff --git a/ui/views/controls/textfield/textfield.cc b/ui/views/controls/textfield/textfield.cc
index fdfe19b095fc0cdb63d857ba800f0d88b509abf0..751e8fa3c87866e1e749f0a04679db40f5a7e94d 100644
--- a/ui/views/controls/textfield/textfield.cc
+++ b/ui/views/controls/textfield/textfield.cc
@@ -769,6 +769,7 @@ bool Textfield::AcceleratorPressed(const ui::Accelerator& accelerator) {
   ui::KeyEvent event(accelerator.type(), accelerator.key_code(),
                      accelerator.modifiers());
   ExecuteTextEditCommand(GetCommandForKeyEvent(event));
+  UpdateAfterChange(false, true);

[sadrul, 2017/03/08 02:02:52]

We shouldn't need this here? ExecuteTextEditCommand() should (and looks like
does) make the necessary call to UpdateAfterChange() when appropriate. Are there
cases when that does not happen?

[yiyix, 2017/03/08 03:32:41]

ExecuteTextEditCommand first calls OmniboxViewViews::ExecuteTextEditCommand,
command "move up", "move down" and "paste" is implemented in the
OmniboxViewViews.  OnAfterPossibleChange is called after paste. So the cursor's
visibility may change after calling paste and it is not updated in
UpdateAfterChange. To catch this particular path, i added this line.

[sadrul, 2017/03/08 13:45:36]

OK. So it looks like there *are* some cases where UpdateAfterChange() does not
get called. That's a bit unfortunate. For the call to UpdateAfterChange() here,
why do we use false for text-change and true for cursor-change?

I think we can make this code a little cleaner. We can probably split
ExecuteTextEditCommand() into two parts: non-virtual ExecuteTextEditCommand(),
which calls a virtual ExecuteTextEditCommandImpl() (or a better name).
OmniboxViewViews overrides the latter. For Textfield,
ExecuteTextEditCommandImpl() includes only the big switch-case block, and the
rest remains in ExecuteTextEditCommand(). That way, ExecuteTextEditCommand()
always calls UpdateAfterChange() when necessary, with the right parameters.
OmniboxViewViews::ExecuteTextEditCommand() can also be cleaned a little (e.g. we
could remove 829:836 in that file.)

   return true;
 }
 
@@ -883,8 +884,8 @@ int Textfield::OnPerformDrop(const ui::DropTargetEvent& event) {
     model_->InsertText(new_text);
   }
   skip_input_method_cancel_composition_ = false;
-  UpdateAfterChange(true, true);
   OnAfterUserAction();
+  UpdateAfterChange(true, true);
   return move ? ui::DragDropTypes::DRAG_MOVE : ui::DragDropTypes::DRAG_COPY;
 }
 
@@ -1263,8 +1264,8 @@ void Textfield::SetCompositionText(const ui::CompositionText& composition) {
   skip_input_method_cancel_composition_ = true;
   model_->SetCompositionText(composition);
   skip_input_method_cancel_composition_ = false;
-  UpdateAfterChange(true, true);
   OnAfterUserAction();
+  UpdateAfterChange(true, true);
 }
 
 void Textfield::ConfirmCompositionText() {
@@ -1275,8 +1276,8 @@ void Textfield::ConfirmCompositionText() {
   skip_input_method_cancel_composition_ = true;
   model_->ConfirmCompositionText();
   skip_input_method_cancel_composition_ = false;
-  UpdateAfterChange(true, true);
   OnAfterUserAction();
+  UpdateAfterChange(true, true);
 }
 
 void Textfield::ClearCompositionText() {
@@ -1287,8 +1288,8 @@ void Textfield::ClearCompositionText() {
   skip_input_method_cancel_composition_ = true;
   model_->CancelCompositionText();
   skip_input_method_cancel_composition_ = false;
-  UpdateAfterChange(true, true);
   OnAfterUserAction();
+  UpdateAfterChange(true, true);
 }
 
 void Textfield::InsertText(const base::string16& new_text) {
@@ -1300,8 +1301,8 @@ void Textfield::InsertText(const base::string16& new_text) {
   skip_input_method_cancel_composition_ = true;
   model_->InsertText(new_text);
   skip_input_method_cancel_composition_ = false;
-  UpdateAfterChange(true, true);
   OnAfterUserAction();
+  UpdateAfterChange(true, true);
 }
 
 void Textfield::InsertChar(const ui::KeyEvent& event) {
@@ -1583,8 +1584,8 @@ void Textfield::DoInsertChar(base::char16 ch) {
   model_->InsertChar(ch);
   skip_input_method_cancel_composition_ = false;
 
-  UpdateAfterChange(true, true);
   OnAfterUserAction();
+  UpdateAfterChange(true, true);
 }
 
 gfx::RenderText* Textfield::GetRenderText() const {
@@ -1795,8 +1796,8 @@ void Textfield::ExecuteTextEditCommand(ui::TextEditCommand command) {
   cursor_changed |= GetSelectionModel() != selection_model;
   if (cursor_changed && HasSelection())
     UpdateSelectionClipboard();
-  UpdateAfterChange(text_changed, cursor_changed);
   OnAfterUserAction();
+  UpdateAfterChange(text_changed, cursor_changed);
 }
 
 ////////////////////////////////////////////////////////////////////////////////