Views: Draw Textfield selected text in gray when top-level Widget loses focus.

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 4163c40a3742095f01fec3da7cf14d44db94aae4..9120de589761454903203b4fd8c7904b124826e5 100644
--- a/ui/views/controls/textfield/textfield.cc
+++ b/ui/views/controls/textfield/textfield.cc
@@ -251,6 +251,7 @@ Textfield::Textfield()
       background_color_(SK_ColorWHITE),
       selection_text_color_(SK_ColorWHITE),
       selection_background_color_(SK_ColorBLUE),
+      focus_manager_(nullptr),
       placeholder_text_color_(kDefaultPlaceholderTextColor),
       invalid_(false),
       text_input_type_(ui::TEXT_INPUT_TYPE_TEXT),
@@ -279,6 +280,13 @@ Textfield::Textfield()
 }
 
 Textfield::~Textfield() {
+  // If not currently attached to a Widget, |focus_manager_| would be null.
+  if (focus_manager_) {
+    // The Textfield may still be listening to focus changes if the Textfield is
+    // destroyed by its controller from OnKey events.
+    focus_manager_->RemoveFocusChangeListener(this);
+    focus_manager_ = nullptr;
+  }
   if (GetInputMethod()) {
     // The textfield should have been blurred before destroy.
     DCHECK(this != GetInputMethod()->GetTextInputClient());
@@ -429,18 +437,23 @@ SkColor Textfield::GetSelectionBackgroundColor() const {
 void Textfield::SetSelectionBackgroundColor(SkColor color) {
   selection_background_color_ = color;
   use_default_selection_background_color_ = false;
-  GetRenderText()->set_selection_background_focused_color(
+  GetRenderText()->set_selection_background_color(
       GetSelectionBackgroundColor());
   SchedulePaint();
 }
 
 void Textfield::UseDefaultSelectionBackgroundColor() {
   use_default_selection_background_color_ = true;
-  GetRenderText()->set_selection_background_focused_color(
+  GetRenderText()->set_selection_background_color(
       GetSelectionBackgroundColor());
   SchedulePaint();
 }
 
+SkColor Textfield::GetUnfocusedSelectionBackgroundColor() const {
+  return GetNativeTheme()->GetSystemColor(
+      ui::NativeTheme::kColorId_TextfieldSelectionBackgroundUnfocused);
+}
+
 bool Textfield::GetCursorEnabled() const {
   return GetRenderText()->cursor_enabled();
 }
@@ -995,6 +1008,29 @@ void Textfield::OnEnabledChanged() {
   SchedulePaint();
 }
 
+void Textfield::ViewHierarchyChanged(
+    const ViewHierarchyChangedDetails& details) {
+  // Textfields only care about focus changes if the entire Widget has lost
+  // focus, so don't bother listening if there is no Widget.
+  if (!GetWidget())
+    return;
+
+  if (details.parent->Contains(this) && details.move_view == nullptr) {
+    if (details.is_add) {
+      FocusManager* focus_manager = GetFocusManager();
+      // There should never be two different FocusManagers for the same Widget
+      // at the same time.
+      DCHECK(!focus_manager_ && focus_manager);
+      focus_manager_ = focus_manager;
+      focus_manager_->AddFocusChangeListener(this);
+    } else {
+      DCHECK(focus_manager_);
+      focus_manager_->RemoveFocusChangeListener(this);
+      focus_manager_ = nullptr;
+    }
+  }
+}
+
 void Textfield::OnPaint(gfx::Canvas* canvas) {
   OnPaintBackground(canvas);
   PaintTextAndCursor(canvas);
@@ -1003,6 +1039,8 @@ void Textfield::OnPaint(gfx::Canvas* canvas) {
 
 void Textfield::OnFocus() {
   GetRenderText()->set_focused(true);
+  GetRenderText()->set_selection_background_color(
+      GetSelectionBackgroundColor());

[tapted, 2016/10/24 06:04:16]

Can this move to OnWillChangeFocus? Just to keep the logic in one place..

[Patti Lor, 2016/10/27 00:17:27]

Done.

   if (ShouldShowCursor())
     GetRenderText()->set_cursor_visible(true);
   if (GetInputMethod())
@@ -1048,8 +1086,12 @@ void Textfield::OnNativeThemeChanged(const ui::NativeTheme* theme) {
   UpdateBackgroundColor();
   render_text->set_cursor_color(GetTextColor());
   render_text->set_selection_color(GetSelectionTextColor());
-  render_text->set_selection_background_focused_color(
-      GetSelectionBackgroundColor());
+  if (HasFocus()) {
+    render_text->set_selection_background_color(GetSelectionBackgroundColor());
+  } else {
+    render_text->set_selection_background_color(
+        GetUnfocusedSelectionBackgroundColor());
+  }
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -1126,6 +1168,40 @@ bool Textfield::CanStartDragForView(View* sender,
 }
 
 ////////////////////////////////////////////////////////////////////////////////
+// Textfield, FocusChangeListener overrides:
+
+void Textfield::OnWillChangeFocus(View* focus_before, View* focus_after) {
+  // If this was the last focused View before switching Widgets, draw the text
+  // selection grayed out.
+  if (focus_before == this && focus_after == nullptr) {
+    GetRenderText()->set_selection_background_color(
+        GetUnfocusedSelectionBackgroundColor());
+    GetRenderText()->set_draw_text_selection(true);
+  }  // If another View in the same Widget is focused, don't draw the selection.
+  else if (focus_before == this && focus_after != nullptr) {

[tapted, 2016/10/24 06:04:16]

nit: we usually have the `else` on the same line as the closing brace. Perhaps
we can break this up a bit with some early returns. E.g.

if (focus_after != this && focus_before != this)
  return;

SkColor selection_color = focus_after
    ? GetSelectionBackgroundColor()
    : GetUnfocusedSelectionBackgroundColor();

// Selection is drawn if |this| has focus, or the Widget loses activation, but
// not if another View in this Widget is gaining focus.
GetRenderText()->set_draw_text_selection(!focus_after || focus_after == this);
GetRenderText()->(selection_color);

[Patti Lor, 2016/10/27 00:17:27]

Done, thanks!

+    GetRenderText()->set_draw_text_selection(false);
+  }  // This is focused now, draw the text selection.
+  else if (focus_after == this) {
+    GetRenderText()->set_draw_text_selection(true);
+  }
+}
+
+void Textfield::OnDidChangeFocus(View* focused_before, View* focused_now) {}
+
+// On Windows and Linux, on Widget destruction, Textfields inside a TYPE_CONTROL
+// Widget will be destroyed after the FocusManager. Make sure to remove this

[tapted, 2016/10/24 03:11:19]

It might be a bug that Mac is different here, or we might just not test it,
having no need for TYPE_CONTROL yet. Can we remove the #ifdef?

[Patti Lor, 2016/10/27 00:17:27]

Yes - OnViewHierarchyChanged will get called before the Mac code can get around
to destroying the FocusManager, so removing the ifdef just means this will never
be called on Mac (at least in the case where TYPE_CONTROL Widgets are being
destroyed - there may be another case where this gets hit which I didn't think
of). Done.

+// Textfield as an observer before the FocusManager is destroyed.
+#if !defined(OS_MACOSX)
+void Textfield::OnFocusManagerDestroying() {
+  if (!focus_manager_)
+    return;
+
+  focus_manager_->RemoveFocusChangeListener(this);
+  focus_manager_ = nullptr;
+}
+#endif
+
+////////////////////////////////////////////////////////////////////////////////
 // Textfield, WordLookupClient overrides:
 
 bool Textfield::GetDecoratedWordAtPoint(const gfx::Point& point,