Unnecessarily cursor appears in omnibox after clicking inside the 'Search box'

ui/views/controls/textfield/textfield.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ui/views/controls/textfield/textfield.h"
 
 #include <string>
 #include <utility>
 
 #include "base/memory/ptr_util.h"
@@ skipping 961 matching lines @@
   SchedulePaint();
 }
 
 void Textfield::OnPaint(gfx::Canvas* canvas) {
   OnPaintBackground(canvas);
   PaintTextAndCursor(canvas);
   OnPaintBorder(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);
   OnCaretBoundsChanged();
   if (ShouldBlinkCursor())
     StartBlinkingCursor();
   if (use_focus_ring_) {
     FocusRing::Install(this, invalid_
                                  ? ui::NativeTheme::kColorId_AlertSeverityHigh
                                  : ui::NativeTheme::kColorId_NumColors);
@@ skipping 906 matching lines @@
   View::SetBorder(std::move(border));
 }
 
 void Textfield::UpdateAfterChange(bool text_changed, bool cursor_changed) {
   if (text_changed) {
     if (controller_)
       controller_->ContentsChanged(this, text());
     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
       StopBlinkingCursor();
     NotifyAccessibilityEvent(ui::AX_EVENT_TEXT_SELECTION_CHANGED, true);
   }
   if (text_changed || cursor_changed) {
     OnCaretBoundsChanged();
     SchedulePaint();
   }
@@ skipping 156 matching lines @@
                                            Textfield::GetCaretBlinkMs()),
                             this, &Textfield::OnCursorBlinkTimerFired);
 }
 
 void Textfield::StopBlinkingCursor() {
   cursor_blink_timer_.Stop();
 }
 
 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();
 }
 
 }  // namespace views