Dynamically create/destroy FocusRing layers

This roughly halves(!) the number of active layers (per
Compositing.Browser.NumActiveLayers) on platforms where FocusRings are
enabled (Mac). This is due to FocusRing previously always painting on
layers regardless of there being anything to draw or not (e.g. it was
always an early return in ::OnPaint rather than preventing the layer
paint).

Bug: chromium:924232
Change-Id: If7b9fcfa2f24b42f2c9b2e86b1b2f1543908b7a9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1902066
Auto-Submit: Peter Boström <pbos@chromium.org>
Commit-Queue: Collin Baker <collinbaker@chromium.org>
Reviewed-by: Collin Baker <collinbaker@chromium.org>
Cr-Commit-Position: refs/heads/master@{#713254}

chrome/browser/ui/ash/accessibility/ax_tree_source_aura_unittest.cc

@@ -189,7 +189,7 @@ TEST_F(AXTreeSourceAuraTest, Serialize) {
   size_t node_count = out_update2.nodes.size();
 
   // We should have far more updates this time around.
-  ASSERT_GE(node_count, 8U);
+  EXPECT_GE(node_count, 7U);
 
   int text_field_update_index = -1;
   for (size_t i = 0; i < node_count; ++i) {

chrome/browser/ui/views/location_bar/location_bar_view.cc

@@ -401,7 +401,7 @@ OmniboxView* LocationBarView::GetOmniboxView() {
 // LocationBarView, public views::View implementation:
 
 bool LocationBarView::HasFocus() const {
-  return omnibox_view_->model()->has_focus();
+  return omnibox_view_ && omnibox_view_->model()->has_focus();
 }
 
 void LocationBarView::GetAccessibleNodeData(ui::AXNodeData* node_data) {

ui/views/controls/focus_ring.cc

@@ -71,7 +71,7 @@ void FocusRing::SetInvalid(bool invalid) {
 
 void FocusRing::SetHasFocusPredicate(const ViewPredicate& predicate) {
   has_focus_predicate_ = predicate;
-  SchedulePaint();
+  RefreshLayer();
 }
 
 void FocusRing::SetColor(base::Optional<SkColor> color) {
@@ -105,11 +105,18 @@ void FocusRing::ViewHierarchyChanged(
     // become a nullptr, it won't be able to do so in its destructor.
     details.parent->RemoveObserver(this);
   }
+  RefreshLayer();
 }
 
 void FocusRing::OnPaint(gfx::Canvas* canvas) {
-  if (!has_focus_predicate_(parent()))
+  // TODO(pbos): Reevaluate if this can turn into a DCHECK, e.g. we should
+  // never paint if there's no parent focus.
+  if (has_focus_predicate_) {
+    if (!(*has_focus_predicate_)(parent()))
+      return;
+  } else if (!parent()->HasFocus()) {
     return;
+  }
 
   cc::PaintFlags paint;
   paint.setAntiAlias(true);
@@ -145,21 +152,16 @@ void FocusRing::OnPaint(gfx::Canvas* canvas) {
 }
 
 void FocusRing::OnViewFocused(View* view) {
-  SchedulePaint();
+  RefreshLayer();
 }
 
 void FocusRing::OnViewBlurred(View* view) {
-  SchedulePaint();
+  RefreshLayer();
 }
 
 FocusRing::FocusRing() {
-  // A layer is necessary to paint beyond the parent's bounds.
-  SetPaintToLayer();
-  layer()->SetFillsBoundsOpaquely(false);
   // Don't allow the view to process events.
   set_can_process_events_within_subtree(false);
-
-  has_focus_predicate_ = [](View* p) -> bool { return p->HasFocus(); };
 }
 
 FocusRing::~FocusRing() {
@@ -167,6 +169,26 @@ FocusRing::~FocusRing() {
     parent()->RemoveObserver(this);
 }
 
+void FocusRing::RefreshLayer() {
+  // TODO(pbos): This always keeps the layer alive if |has_focus_predicate_| is
+  // set. This is done because we're not notified when the predicate might
+  // return a different result and there are call sites that call SchedulePaint
+  // on FocusRings and expect that to be sufficient.
+  // The cleanup would be to always call has_focus_predicate_ here and make sure
+  // that RefreshLayer gets called somehow whenever |has_focused_predicate_|
+  // returns a new value.
+  const bool should_paint =
+      has_focus_predicate_.has_value() || (parent() && parent()->HasFocus());
+  SetVisible(should_paint);
+  if (should_paint) {
+    // A layer is necessary to paint beyond the parent's bounds.
+    SetPaintToLayer();
+    layer()->SetFillsBoundsOpaquely(false);
+  } else {
+    DestroyLayer();
+  }
+}
+
 SkRRect FocusRing::RingRectFromPathRect(const SkRect& rect) const {
   const double corner_radius = GetCornerRadius();
   return RingRectFromPathRect(

ui/views/controls/focus_ring.h

@@ -83,6 +83,8 @@ class VIEWS_EXPORT FocusRing : public View, public ViewObserver {
  private:
   FocusRing();
 
+  void RefreshLayer();
+
   // Translates the provided SkRect or SkRRect, which is in the parent's
   // coordinate system, into this view's coordinate system, then insets it
   // appropriately to produce the focus ring "halo" effect. If the supplied rect
@@ -102,7 +104,7 @@ class VIEWS_EXPORT FocusRing : public View, public ViewObserver {
   base::Optional<SkColor> color_;
 
   // The predicate used to determine whether the parent has focus.
-  ViewPredicate has_focus_predicate_;
+  base::Optional<ViewPredicate> has_focus_predicate_;
 
   DISALLOW_COPY_AND_ASSIGN(FocusRing);
 };