Unnecessarily cursor appears in omnibox after clicking inside the '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 01a666be628b77420bc5d76c902de3877e681d9f..01ca7439765f3ff74908349e1dcbb233ca56f7a6 100644
--- a/ui/views/controls/textfield/textfield.cc
+++ b/ui/views/controls/textfield/textfield.cc
@@ -979,10 +979,11 @@ void Textfield::OnPaint(gfx::Canvas* canvas) {
 }
 
 void Textfield::OnFocus() {
-  GetRenderText()->set_focused(true);
+  gfx::RenderText* render_text = GetRenderText();
+  render_text->set_focused(true);
   if (ShouldShowCursor()) {

[sadrul, 2017/03/01 20:51:24]

Can you look at whether ShouldShowCursor() should check
render_text->cursor_enabled() instead?

[yiyix, 2017/03/01 22:18:58]

ShouldShowCursor() does not check render_text->cursor_enabled(). Yes, I agree
that this check should be done in ShouldShowCursor(). Thank you for catching
this. 

     UpdateCursorView();
-    cursor_view_.SetVisible(true);
+    cursor_view_.SetVisible(render_text->cursor_enabled());
   }
   if (GetInputMethod())
     GetInputMethod()->SetFocusedTextInputClient(this);
@@ -1909,8 +1910,9 @@ void Textfield::UpdateAfterChange(bool text_changed, bool cursor_changed) {
     NotifyAccessibilityEvent(ui::AX_EVENT_TEXT_CHANGED, true);
   }
   if (cursor_changed) {
-    cursor_view_.SetVisible(ShouldShowCursor());
     UpdateCursorView();
+    cursor_view_.SetVisible(GetRenderText()->cursor_enabled() &&
+                            ShouldShowCursor());
     if (ShouldBlinkCursor())
       StartBlinkingCursor();
     else
@@ -2087,7 +2089,8 @@ void Textfield::StopBlinkingCursor() {
 
 void Textfield::OnCursorBlinkTimerFired() {
   DCHECK(ShouldBlinkCursor());
-  cursor_view_.SetVisible(!cursor_view_.visible());
+  cursor_view_.SetVisible(GetRenderText()->cursor_enabled() &&
+                          !cursor_view_.visible());

[sadrul, 2017/03/01 20:51:24]

If cursor is disabled in RenderText, is there a reason to run the blink-timer at
all?

[yiyix, 2017/03/01 22:18:58]

After the modification, this function is only called if ShouldBlinkCursor()
returns true. No need to check GetRenderText()->cursor_enabled() in this
function. 

   UpdateCursorView();
 }