Convert visible() to a property.

Updated usages to a visible changed callback.

Bug: 938501
Change-Id: I01fe4f9f21f3f5b40fedb05edde0331afce97604
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1505848
Commit-Queue: Allen Bauer <kylixrd@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#653559}

chrome/browser/ui/views/media_router/presentation_receiver_window_view_browsertest.cc

@@ -119,7 +119,7 @@ IN_PROC_BROWSER_TEST_F(PresentationReceiverWindowViewBrowserTest,
   // It waits for the location bar visibility to change rather than simply using
   // RunLoop::RunUntilIdle because in Mash, the fullscreen change takes place in
   // another process.
-  class FullscreenWaiter final : public views::ViewObserver {
+  class FullscreenWaiter final {
    public:
     enum class AwaitType {
       kOutOfFullscreen,
@@ -132,15 +132,16 @@ IN_PROC_BROWSER_TEST_F(PresentationReceiverWindowViewBrowserTest,
         : receiver_view_(receiver_view),
           await_type_(await_type),
           fullscreen_callback_(std::move(fullscreen_callback)) {
-      receiver_view_->location_bar_view()->AddObserver(this);
-    }
-    ~FullscreenWaiter() final {
-      receiver_view_->location_bar_view()->RemoveObserver(this);
+      auto* location_bar_view = receiver_view_->location_bar_view();
+      subscription_ =
+          location_bar_view->AddVisibleChangedCallback(base::BindRepeating(
+              &FullscreenWaiter::OnViewVisibilityChanged,
+              base::Unretained(this), base::Unretained(location_bar_view)));
     }
+    ~FullscreenWaiter() = default;
 
    private:
-    void OnViewVisibilityChanged(views::View* observed_view,
-                                 views::View* starting_view) override {
+    void OnViewVisibilityChanged(views::View* observed_view) {
       bool fullscreen = !observed_view->visible();
       EXPECT_EQ(fullscreen, receiver_view_->IsFullscreen());
       if (fullscreen == (await_type_ == AwaitType::kIntoFullscreen))
@@ -148,6 +149,7 @@ IN_PROC_BROWSER_TEST_F(PresentationReceiverWindowViewBrowserTest,
     }
 
     PresentationReceiverWindowView* const receiver_view_;
+    views::PropertyChangedSubscription subscription_;
     const AwaitType await_type_;
     base::OnceClosure fullscreen_callback_;
 

ui/views/view.cc

@@ -424,12 +424,7 @@ void View::SetVisible(bool visible) {
       layout_manager->ViewVisibilitySet(parent_, this, visible);
   }
 
-  if (visible != visible_) {
-    // If the View is currently visible, schedule paint to refresh parent.
-    // TODO(beng): not sure we should be doing this if we have a layer.
-    if (visible_)
-      SchedulePaint();
-
+  if (visible_ != visible) {
     visible_ = visible;
     AdvanceFocusIfNecessary();
 
@@ -444,21 +439,25 @@ void View::SetVisible(bool visible) {
     PropagateVisibilityNotifications(this, visible_);
     UpdateLayerVisibility();
 
-    // If we are newly visible, schedule paint.
-    if (visible_)
-      SchedulePaint();
+    // Notify all other subscriptions of the change.
+    OnPropertyChanged(&visible_, kPropertyEffectsLayout);
   }
 }
 
+PropertyChangedSubscription View::AddVisibleChangedCallback(
+    PropertyChangedCallback callback) {
+  return AddPropertyChangedCallback(&visible_, std::move(callback));
+}
+
 bool View::IsDrawn() const {
   return visible_ && parent_ ? parent_->IsDrawn() : false;
 }
 
-void View::SetEnabled(bool is_enabled) {
-  if (enabled_ == is_enabled)
+void View::SetEnabled(bool enabled) {
+  if (enabled_ == enabled)
     return;
 
-  enabled_ = is_enabled;
+  enabled_ = enabled;
   AdvanceFocusIfNecessary();
   OnPropertyChanged(&enabled_, kPropertyEffectsPaint);
 }
@@ -968,7 +967,7 @@ View* View::GetTooltipHandlerForPoint(const gfx::Point& point) {
   View::Views children = GetChildrenInZOrder();
   DCHECK_EQ(children_.size(), children.size());
   for (auto* child : base::Reversed(children)) {
-    if (!child->visible())
+    if (!child->GetVisible())
       continue;
 
     gfx::Point point_in_child_coords(point);
@@ -1610,7 +1609,7 @@ void View::MoveLayerToParent(ui::Layer* parent_layer,
 void View::UpdateLayerVisibility() {
   bool visible = visible_;
   for (const View* v = parent_; visible && v && !v->layer(); v = v->parent_)
-    visible = v->visible();
+    visible = v->GetVisible();
 
   UpdateChildLayerVisibility(visible);
 }
@@ -2066,7 +2065,7 @@ void View::AddChildViewAtImpl(View* view, int index) {
   if (widget) {
     RegisterChildrenForVisibleBoundsNotification(view);
 
-    if (view->visible())
+    if (view->GetVisible())
       view->SchedulePaint();
   }
 
@@ -2102,7 +2101,7 @@ void View::DoRemoveChildView(View* view,
   bool is_removed_from_widget = false;
   if (widget) {
     UnregisterChildrenForVisibleBoundsNotification(view);
-    if (view->visible())
+    if (view->GetVisible())
       view->SchedulePaint();
 
     is_removed_from_widget = !new_parent || new_parent->GetWidget() != widget;
@@ -2707,6 +2706,7 @@ bool View::DoDrag(const ui::LocatedEvent& event,
 // declaration for View.
 BEGIN_METADATA(View)
 ADD_PROPERTY_METADATA(View, bool, Enabled)
+ADD_PROPERTY_METADATA(View, bool, Visible)
 END_METADATA()
 
 }  // namespace views

ui/views/view.h

@@ -550,12 +550,23 @@ class VIEWS_EXPORT View : public ui::LayerDelegate,
   // GetPreferredSize().height().
   virtual int GetHeightForWidth(int w) const;
 
+  // The |Visible| property. See comment above for instructions on declaring and
+  // implementing a property.
+  //
   // Sets whether this view is visible. Painting is scheduled as needed. Also,
   // clears focus if the focused view or one of its ancestors is set to be
   // hidden.
   virtual void SetVisible(bool visible);
+  // Return whether a view is visible.
+  bool GetVisible() const { return visible_; }
+
+  // Adds a callback subscription associated with the above Visible property.
+  // The callback will be invoked whenever the Visible property changes.
+  PropertyChangedSubscription AddVisibleChangedCallback(
+      PropertyChangedCallback callback) WARN_UNUSED_RESULT;
 
-  // Return whether a view is visible
+  // NOTE: Deprecated. Please use GetVisible() which is the getter for the
+  // |Visible| property.
   bool visible() const { return visible_; }
 
   // Returns true if this view is drawn on screen.
@@ -567,7 +578,7 @@ class VIEWS_EXPORT View : public ui::LayerDelegate,
   // Set whether this view is enabled. A disabled view does not receive keyboard
   // or mouse inputs. If |enabled| differs from the current value, SchedulePaint
   // is invoked. Also, clears focus if the focused view is disabled.
-  void SetEnabled(bool is_enabled);
+  void SetEnabled(bool enabled);
   // Returns whether the view is enabled.
   bool GetEnabled() const { return enabled_; }
 

ui/views/view_unittest.cc

@@ -5010,6 +5010,17 @@ TEST_F(ViewTest, TestEnabledChangedCallback) {
   EXPECT_FALSE(test_view.GetEnabled());
 }
 
+TEST_F(ViewTest, TestVisibleChangedCallback) {
+  View test_view;
+  bool visibility_changed = false;
+  auto subscription = test_view.AddVisibleChangedCallback(base::BindRepeating(
+      [](bool* visibility_changed) { *visibility_changed = true; },
+      &visibility_changed));
+  test_view.SetVisible(false);
+  EXPECT_TRUE(visibility_changed);
+  EXPECT_FALSE(test_view.GetVisible());
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // Observer tests.
 ////////////////////////////////////////////////////////////////////////////////