Use class properties for Ignored and can-be-visible state in a layout for a given view.

This will remove the need for the child_infos_ field in LayoutManagerBase.

Somewhat related to the work of integrating fill layout behavior into views::View.

Bug: 1159116
Change-Id: I968aa58fb3b29f78d96cd520e61ccb731c2e9ac9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2605567
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#839620}

ui/views/layout/layout_manager.h

@@ -91,7 +91,7 @@ class VIEWS_EXPORT LayoutManager {
 
  protected:
   // Sets the visibility of a view without triggering ViewVisibilitySet().
-  // During Layout(), use this method instead of View::SetVisibility().
+  // During Layout(), use this method instead of View::SetVisible().
   void SetViewVisibility(View* view, bool visible);
 
   // Gets the child views of the specified view in paint order (reverse

ui/views/layout/layout_manager_base.cc

@@ -9,6 +9,7 @@
 #include "base/auto_reset.h"
 #include "base/check_op.h"
 #include "ui/views/view.h"
+#include "ui/views/view_class_properties.h"
 
 namespace views {
 
@@ -100,9 +101,7 @@ ProposedLayout LayoutManagerBase::GetProposedLayout(
 
 void LayoutManagerBase::SetChildViewIgnoredByLayout(View* child_view,
                                                     bool ignored) {
-  auto it = child_infos_.find(child_view);
-  DCHECK(it != child_infos_.end());
-  if (it->second.ignored == ignored)
+  if (child_view->GetProperty(kViewIgnoredByLayoutKey) == ignored)
     return;
 
   base::AutoReset<bool> setter(&suppress_invalidate_, true);
@@ -112,9 +111,7 @@ void LayoutManagerBase::SetChildViewIgnoredByLayout(View* child_view,
 
 bool LayoutManagerBase::IsChildViewIgnoredByLayout(
     const View* child_view) const {
-  auto it = child_infos_.find(child_view);
-  DCHECK(it != child_infos_.end());
-  return it->second.ignored;
+  return child_view->GetProperty(kViewIgnoredByLayoutKey);
 }
 
 LayoutManagerBase::LayoutManagerBase() = default;
@@ -127,27 +124,12 @@ SizeBounds LayoutManagerBase::GetAvailableHostSize() const {
 
 bool LayoutManagerBase::IsChildIncludedInLayout(const View* child,
                                                 bool include_hidden) const {
-  const auto it = child_infos_.find(child);
-
-  // During callbacks when a child is removed we can get in a state where a view
-  // in the child list of the host view is not in |child_infos_|. In that case,
-  // the view is being removed and is not part of the layout.
-  if (it == child_infos_.end())
-    return false;
-
-  return !it->second.ignored && (include_hidden || it->second.can_be_visible);
+  return !child->GetProperty(kViewIgnoredByLayoutKey) &&
+         (include_hidden || child->GetProperty(kViewCanBeVisibleKey));
 }
 
 bool LayoutManagerBase::CanBeVisible(const View* child) const {
-  const auto it = child_infos_.find(child);
-
-  // During callbacks when a child is removed we can get in a state where a view
-  // in the child list of the host view is not in |child_infos_|. In that case,
-  // the view is being removed and is not part of the layout.
-  if (it == child_infos_.end())
-    return false;
-
-  return it->second.can_be_visible;
+  return child->GetProperty(kViewCanBeVisibleKey);
 }
 
 void LayoutManagerBase::LayoutImpl() {
@@ -253,7 +235,6 @@ void LayoutManagerBase::InvalidateLayout() {
 void LayoutManagerBase::Installed(View* host_view) {
   DCHECK(host_view);
   DCHECK(!host_view_);
-  DCHECK(child_infos_.empty());
 
   base::AutoReset<bool> setter(&suppress_invalidate_, true);
   PropagateInstalled(host_view);
@@ -261,7 +242,6 @@ void LayoutManagerBase::Installed(View* host_view) {
 
 void LayoutManagerBase::ViewAdded(View* host, View* view) {
   DCHECK_EQ(host_view_, host);
-  DCHECK(!base::Contains(child_infos_, view));
 
   base::AutoReset<bool> setter(&suppress_invalidate_, true);
   const bool invalidate = PropagateViewAdded(host, view);
@@ -271,11 +251,8 @@ void LayoutManagerBase::ViewAdded(View* host, View* view) {
 
 void LayoutManagerBase::ViewRemoved(View* host, View* view) {
   DCHECK_EQ(host_view_, host);
-  DCHECK(base::Contains(child_infos_, view));
-
-  auto it = child_infos_.find(view);
-  DCHECK(it != child_infos_.end());
-  const bool removed_visible = it->second.can_be_visible && !it->second.ignored;
+  const bool removed_visible = view->GetProperty(kViewCanBeVisibleKey) &&
+                               !view->GetProperty(kViewIgnoredByLayoutKey);
 
   base::AutoReset<bool> setter(&suppress_invalidate_, true);
   const bool invalidate = PropagateViewRemoved(host, view);
@@ -288,10 +265,8 @@ void LayoutManagerBase::ViewVisibilitySet(View* host,
                                           bool old_visibility,
                                           bool new_visibility) {
   DCHECK_EQ(host_view_, host);
-  auto it = child_infos_.find(view);
-  DCHECK(it != child_infos_.end());
-  const bool was_ignored = it->second.ignored;
-  if (it->second.can_be_visible == new_visibility)
+  const bool was_ignored = view->GetProperty(kViewIgnoredByLayoutKey);
+  if (view->GetProperty(kViewCanBeVisibleKey) == new_visibility)
     return;
 
   base::AutoReset<bool> setter(&suppress_invalidate_, true);
@@ -308,11 +283,11 @@ void LayoutManagerBase::AddOwnedLayoutInternal(
   if (host_view_) {
     owned_layout->Installed(host_view_);
     for (View* child_view : host_view_->children()) {
-      const ChildInfo& child_info = child_infos_.find(child_view)->second;
-      owned_layout->PropagateChildViewIgnoredByLayout(child_view,
-                                                      child_info.ignored);
-      owned_layout->PropagateViewVisibilitySet(host_view_, child_view,
-                                               child_info.can_be_visible);
+      owned_layout->PropagateChildViewIgnoredByLayout(
+          child_view, child_view->GetProperty(kViewIgnoredByLayoutKey));
+      owned_layout->PropagateViewVisibilitySet(
+          host_view_, child_view,
+          child_view->GetProperty(kViewCanBeVisibleKey));
     }
   }
   owned_layout->parent_layout_ = this;
@@ -328,7 +303,7 @@ LayoutManagerBase* LayoutManagerBase::GetRootLayoutManager() {
 
 bool LayoutManagerBase::PropagateChildViewIgnoredByLayout(View* child_view,
                                                           bool ignored) {
-  child_infos_[child_view].ignored = ignored;
+  child_view->SetProperty(kViewIgnoredByLayoutKey, ignored);
 
   bool result = false;
   for (auto& owned_layout : owned_layouts_) {
@@ -340,7 +315,7 @@ bool LayoutManagerBase::PropagateChildViewIgnoredByLayout(View* child_view,
 }
 
 bool LayoutManagerBase::PropagateViewAdded(View* host, View* view) {
-  child_infos_.emplace(view, ChildInfo{view->GetVisible(), false});
+  view->SetProperty(kViewCanBeVisibleKey, view->GetVisible());
 
   bool result = false;
 
@@ -352,7 +327,8 @@ bool LayoutManagerBase::PropagateViewAdded(View* host, View* view) {
 }
 
 bool LayoutManagerBase::PropagateViewRemoved(View* host, View* view) {
-  child_infos_.erase(view);
+  view->ClearProperty(kViewCanBeVisibleKey);
+  view->ClearProperty(kViewIgnoredByLayoutKey);
 
   bool result = false;
 
@@ -366,7 +342,7 @@ bool LayoutManagerBase::PropagateViewRemoved(View* host, View* view) {
 bool LayoutManagerBase::PropagateViewVisibilitySet(View* host,
                                                    View* view,
                                                    bool visible) {
-  child_infos_[view].can_be_visible = visible;
+  view->SetProperty(kViewCanBeVisibleKey, visible);
 
   bool result = false;
 
@@ -379,9 +355,8 @@ bool LayoutManagerBase::PropagateViewVisibilitySet(View* host,
 
 void LayoutManagerBase::PropagateInstalled(View* host) {
   host_view_ = host;
-  for (auto* it : host->children()) {
-    child_infos_.emplace(it, ChildInfo{it->GetVisible(), false});
-  }
+  for (auto* child : host->children())
+    child->SetProperty(kViewCanBeVisibleKey, child->GetVisible());
 
   for (auto& owned_layout : owned_layouts_)
     owned_layout->PropagateInstalled(host);

ui/views/layout/layout_manager_base.h

@@ -106,7 +106,7 @@ class VIEWS_EXPORT LayoutManagerBase : public LayoutManager {
 
   // Returns whether the specified child view can be visible. To be able to be
   // visible, |child| must be a child of the host view, and must have been
-  // visible when it was added or most recently had GetVisible(true) called on
+  // visible when it was added or most recently had SetVisible(true) called on
   // it by non-layout code.
   bool CanBeVisible(const View* child) const;
 
@@ -171,13 +171,6 @@ class VIEWS_EXPORT LayoutManagerBase : public LayoutManager {
  private:
   friend class LayoutManagerBaseAvailableSizeTest;
 
-  // Holds bookkeeping data used to determine inclusion of children in the
-  // layout.
-  struct ChildInfo {
-    bool can_be_visible = true;
-    bool ignored = false;
-  };
-
   // LayoutManager:
   void InvalidateLayout() final;
   void Installed(View* host) final;
@@ -203,7 +196,6 @@ class VIEWS_EXPORT LayoutManagerBase : public LayoutManager {
   void PropagateInvalidateLayout();
 
   View* host_view_ = nullptr;
-  std::map<const View*, ChildInfo> child_infos_;
   std::vector<std::unique_ptr<LayoutManagerBase>> owned_layouts_;
   LayoutManagerBase* parent_layout_ = nullptr;
 

ui/views/view.cc

@@ -56,6 +56,7 @@
 #include "ui/views/controls/scroll_view.h"
 #include "ui/views/drag_controller.h"
 #include "ui/views/metadata/metadata_impl_macros.h"
+#include "ui/views/view_class_properties.h"
 #include "ui/views/view_observer.h"
 #include "ui/views/view_tracker.h"
 #include "ui/views/views_features.h"
@@ -3125,14 +3126,18 @@ View::DefaultFillLayout::~DefaultFillLayout() = default;
 
 void View::DefaultFillLayout::Layout(View* host) {
   const gfx::Rect contents_bounds = host->GetContentsBounds();
-  for (auto* child : host->children())
-    child->SetBoundsRect(contents_bounds);
+  for (auto* child : host->children()) {
+    if (!child->GetProperty(kViewIgnoredByLayoutKey))
+      child->SetBoundsRect(contents_bounds);
+  }
 }
 
 gfx::Size View::DefaultFillLayout::GetPreferredSize(const View* host) const {
   gfx::Size preferred_size;
-  for (auto* child : host->children())
-    preferred_size.SetToMax(child->GetPreferredSize());
+  for (auto* child : host->children()) {
+    if (!child->GetProperty(kViewIgnoredByLayoutKey))
+      preferred_size.SetToMax(child->GetPreferredSize());
+  }
   return preferred_size;
 }
 
@@ -3140,10 +3145,13 @@ int View::DefaultFillLayout::GetPreferredHeightForWidth(const View* host,
                                                         int width) const {
   const gfx::Insets insets = host->GetInsets();
   int preferred_height = 0;
-  for (auto* child : host->children())
-    preferred_height = std::max(
-        preferred_height,
-        child->GetHeightForWidth(width - insets.width()) + insets.height());
+  for (auto* child : host->children()) {
+    if (!child->GetProperty(kViewIgnoredByLayoutKey)) {
+      preferred_height = std::max(
+          preferred_height,
+          child->GetHeightForWidth(width - insets.width()) + insets.height());
+    }
+  }
   return preferred_height;
 }
 

ui/views/view_class_properties.cc

@@ -41,5 +41,7 @@ DEFINE_OWNED_UI_CLASS_PROPERTY_KEY(FlexSpecification, kFlexBehaviorKey, nullptr)
 DEFINE_OWNED_UI_CLASS_PROPERTY_KEY(LayoutAlignment,
                                    kCrossAxisAlignmentKey,
                                    nullptr)
+DEFINE_UI_CLASS_PROPERTY_KEY(bool, kViewIgnoredByLayoutKey, false)
+DEFINE_UI_CLASS_PROPERTY_KEY(bool, kViewCanBeVisibleKey, false)
 
 }  // namespace views

ui/views/view_class_properties.h

@@ -58,6 +58,15 @@ VIEWS_EXPORT extern const ui::ClassProperty<FlexSpecification*>* const
 VIEWS_EXPORT extern const ui::ClassProperty<LayoutAlignment*>* const
     kCrossAxisAlignmentKey;
 
+// Property indicating whether a view should be ignored by a layout. Supported
+// by LayoutManagerBase and View::DefaultFillLayout
+VIEWS_EXPORT extern const ui::ClassProperty<bool>* const
+    kViewIgnoredByLayoutKey;
+
+// Property indicating whether a view would can be visible based on the criteria
+// outlined in LayoutManagerBase::CanBeVisible().
+VIEWS_EXPORT extern const ui::ClassProperty<bool>* const kViewCanBeVisibleKey;
+
 }  // namespace views
 
 // Declaring the template specialization here to make sure that the

ui/views/widget/root_view.cc

@@ -22,8 +22,8 @@
 #include "ui/events/keycodes/keyboard_codes.h"
 #include "ui/gfx/canvas.h"
 #include "ui/views/drag_controller.h"
-#include "ui/views/layout/fill_layout.h"
 #include "ui/views/metadata/metadata_impl_macros.h"
+#include "ui/views/view_class_properties.h"
 #include "ui/views/view_targeter.h"
 #include "ui/views/widget/root_view_targeter.h"
 #include "ui/views/widget/widget.h"
@@ -210,7 +210,7 @@ void RootView::SetContentsView(View* contents_view) {
       << "Can't be called until after the native widget is created!";
   // The ContentsView must be set up _after_ the window is created so that its
   // Widget pointer is valid.
-  SetLayoutManager(std::make_unique<FillLayout>());
+  SetUseDefaultFillLayout(true);
   if (!children().empty())
     RemoveAllChildViews(true);
   AddChildView(contents_view);
@@ -268,8 +268,7 @@ void RootView::AnnounceText(const base::string16& text) {
   DCHECK(GetContentsView());
   if (!announce_view_) {
     announce_view_ = AddChildView(std::make_unique<AnnounceTextView>());
-    static_cast<FillLayout*>(GetLayoutManager())
-        ->SetChildViewIgnoredByLayout(announce_view_, true);
+    announce_view_->SetProperty(kViewIgnoredByLayoutKey, true);
   }
   announce_view_->Announce(text);
 #endif