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 27c0e6a573df57bcdef0a3b0cde35d815b49c623..4689ad87a6810ba8ee41530cfc9ad27a8ef85513 100644
--- a/ui/views/controls/textfield/textfield.cc
+++ b/ui/views/controls/textfield/textfield.cc
@@ -240,6 +240,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),
@@ -277,6 +278,7 @@ Textfield::~Textfield() {
     // The textfield should have been blurred before destroy.
     DCHECK(this != GetInputMethod()->GetTextInputClient());
   }
+  DCHECK(!focus_manager_);
 }
 
 void Textfield::SetReadOnly(bool read_only) {
@@ -424,18 +426,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 {

[msw, 2016/11/09 02:08:53]

Inline this or make a file-local function, it's not needed on Textfield.

+  return GetNativeTheme()->GetSystemColor(
+      ui::NativeTheme::kColorId_TextfieldSelectionBackgroundUnfocused);
+}
+
 bool Textfield::GetCursorEnabled() const {
   return GetRenderText()->cursor_enabled();
 }
@@ -941,6 +948,16 @@ void Textfield::OnEnabledChanged() {
   SchedulePaint();
 }
 
+void Textfield::ViewHierarchyChanged(
+    const ViewHierarchyChangedDetails& details) {
+  if (details.parent->Contains(this) && details.move_view == nullptr)

[msw, 2016/11/09 02:08:53]

Can you explain what's happening here? Shouldn't this be something like:
if (details.child == this)
  ObserveWidgetFocusChanges(details.is_add);

+    ObserveWidgetFocusChanges(details.is_add);
+}
+
+void Textfield::NativeViewHierarchyChanged() {
+  ObserveWidgetFocusChanges(true);

[msw, 2016/11/09 02:08:53]

Add a comment explaining why this is necessary; The function comment says it's
useful for tracking the view's FocusManager, but it's not obvious...

+}
+
 void Textfield::OnPaint(gfx::Canvas* canvas) {
   OnPaintBackground(canvas);
   PaintTextAndCursor(canvas);
@@ -994,8 +1011,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()) {

[msw, 2016/11/09 02:08:53]

optional nit: use an inlined ternary or something like line 1102:
SkColor selection_bg_color = HasFocus()
                                 ? GetSelectionBackgroundColor()
                                 : GetUnfocusedSelectionBackgroundColor();

+    render_text->set_selection_background_color(GetSelectionBackgroundColor());
+  } else {
+    render_text->set_selection_background_color(
+        GetUnfocusedSelectionBackgroundColor());
+  }
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -1072,6 +1093,25 @@ bool Textfield::CanStartDragForView(View* sender,
 }
 
 ////////////////////////////////////////////////////////////////////////////////
+// Textfield, FocusChangeListener overrides:
+
+void Textfield::OnWillChangeFocus(View* focus_before, View* focus_after) {

[msw, 2016/11/09 02:08:53]

It's really unfortunate that we can't do this via OnFocus/OnBlur... Why was it
determined that we shouldn't paint unfocused selections when the focus is
transferred to another view in the widget?

[tapted, 2016/11/11 00:10:39]

I think this would be inconsistent with the WebContents. Also the "Graphite"
option is a popular theme on Mac, which gives an (almost?) identical gray color
to unfocused selections (i.e. instead of Aqua/blue). This would make it almost
impossible to tell what textfield has focus based on the selection highlight.

+  if (focus_before != this && focus_after != this)
+    return;
+
+  SkColor selection_bg_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 == this || !focus_after);
+  GetRenderText()->set_selection_background_color(selection_bg_color);

[msw, 2016/11/09 02:08:53]

Should we SchedulePaint after this, just in case this happens after
Textfield::OnFocus or Textfield::OnBlur (in which case this might change the
appearance without scheduling a paint)

+}
+
+void Textfield::OnDidChangeFocus(View* focused_before, View* focused_now) {}
+
+////////////////////////////////////////////////////////////////////////////////
 // Textfield, WordLookupClient overrides:
 
 bool Textfield::GetDecoratedWordAtPoint(const gfx::Point& point,
@@ -1838,6 +1878,25 @@ void Textfield::UpdateSelectionClipboard() {
 #endif
 }
 
+void Textfield::ObserveWidgetFocusChanges(bool add_as_listener) {
+  if (focus_manager_) {
+    focus_manager_->RemoveFocusChangeListener(this);
+    focus_manager_ = nullptr;
+  }
+
+  if (add_as_listener) {
+    focus_manager_ = GetFocusManager();

[msw, 2016/11/09 02:08:53]

Should this early-return up top if |focus_manager_ == GetFocusManager() &&
add_as_listener|?

+    if (!focus_manager_) {
+      // |focus_manager_| will be null if GetWidget() is null (not currently in
+      // a Widget), or if the Widget's FocusManager is null and there is no
+      // parent Widget yet (e.g. TYPE_CONTROL Widgets). In the latter case, a
+      // call to NativeViewHierarchyChanged() should follow.
+      return;
+    }
+    focus_manager_->AddFocusChangeListener(this);
+  }
+}
+
 void Textfield::UpdateBackgroundColor() {
   const SkColor color = GetBackgroundColor();
   if (ui::MaterialDesignController::IsSecondaryUiMaterial()) {