Prevent layout manager from being invalidated during layout.

Fixes potential hard crash when arbitrary View subclass calls
InvalidateLayout() during OnBoundsChanged(), etc.

(We saw this in at least one example in the wild.)

Change-Id: I639fcaf0198fa899989b36a6264d1caa74ef8404
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1900253
Commit-Queue: Caroline Rising <corising@chromium.org>
Reviewed-by: Caroline Rising <corising@chromium.org>
Auto-Submit: Dana Fried <dfried@chromium.org>
Cr-Commit-Position: refs/heads/master@{#713100}

ui/views/layout/animating_layout_manager.cc

@@ -334,29 +334,6 @@ int AnimatingLayoutManager::GetPreferredHeightForWidth(const View* host,
              : target_layout_manager()->GetPreferredHeightForWidth(host, width);
 }
 
-void AnimatingLayoutManager::Layout(View* host) {
-  DCHECK_EQ(host_view(), host);
-  // Changing the size of a view directly will lead to a layout call rather
-  // than an invalidation. This should reset the layout (but see the note in
-  // RecalculateTarget() below).
-  if (should_animate_bounds_) {
-    const int host_main = GetMainAxis(orientation(), host->size());
-    const int desired_main =
-        GetMainAxis(orientation(), current_layout_.host_size);
-    if (desired_main > host_main)
-      ResetLayoutToSize(host->size());
-  } else if (!cached_layout_size() || host->size() != *cached_layout_size()) {
-    ResetLayout();
-  }
-
-  ApplyLayout(current_layout_);
-
-  // Send animating stopped events on layout so the current layout during the
-  // event represents the final state instead of an intermediate state.
-  if (is_animating_ && current_offset_ == 1.0)
-    OnAnimationEnded();
-}
-
 std::vector<View*> AnimatingLayoutManager::GetChildViewsInPaintOrder(
     const View* host) const {
   DCHECK_EQ(host_view(), host);
@@ -426,6 +403,29 @@ void AnimatingLayoutManager::OnLayoutChanged() {
   RecalculateTarget();
 }
 
+void AnimatingLayoutManager::LayoutImpl() {
+  // Changing the size of a view directly will lead to a layout call rather
+  // than an invalidation. This should reset the layout (but see the note in
+  // RecalculateTarget() below).
+  const gfx::Size host_size = host_view()->size();
+  if (should_animate_bounds_) {
+    const int host_main = GetMainAxis(orientation(), host_size);
+    const int desired_main =
+        GetMainAxis(orientation(), current_layout_.host_size);
+    if (desired_main > host_main)
+      ResetLayoutToSize(host_size);
+  } else if (!cached_layout_size() || host_size != *cached_layout_size()) {
+    ResetLayout();
+  }
+
+  ApplyLayout(current_layout_);
+
+  // Send animating stopped events on layout so the current layout during the
+  // event represents the final state instead of an intermediate state.
+  if (is_animating_ && current_offset_ == 1.0)
+    OnAnimationEnded();
+}
+
 bool AnimatingLayoutManager::RecalculateTarget() {
   constexpr double kResetAnimationThreshold = 0.8;
 

ui/views/layout/animating_layout_manager.h

@@ -159,7 +159,6 @@ class VIEWS_EXPORT AnimatingLayoutManager : public LayoutManagerBase {
   gfx::Size GetPreferredSize(const View* host) const override;
   gfx::Size GetMinimumSize(const View* host) const override;
   int GetPreferredHeightForWidth(const View* host, int width) const override;
-  void Layout(View* host) override;
   std::vector<View*> GetChildViewsInPaintOrder(const View* host) const override;
 
   // Queues an action to take place after the current animation completes.
@@ -188,6 +187,7 @@ class VIEWS_EXPORT AnimatingLayoutManager : public LayoutManagerBase {
       const SizeBounds& size_bounds) const override;
   void OnInstalled(View* host) override;
   void OnLayoutChanged() override;
+  void LayoutImpl() override;
 
  private:
   struct LayoutFadeInfo;

ui/views/layout/layout_manager_base.cc

@@ -41,8 +41,12 @@ int LayoutManagerBase::GetPreferredHeightForWidth(const View* host,
 
 void LayoutManagerBase::Layout(View* host) {
   DCHECK_EQ(host_view_, host);
-  const gfx::Size size = host->size();
-  ApplyLayout(GetProposedLayout(size));
+  // A handful of views will cause invalidations while they are being
+  // positioned, which can result in loops or loss of layout data during layout
+  // application. Therefore we protect the layout manager from spurious
+  // invalidations during the layout process.
+  base::AutoReset<bool> setter(&suppress_invalidate_, true);
+  LayoutImpl();
 }
 
 std::vector<View*> LayoutManagerBase::GetChildViewsInPaintOrder(
@@ -94,6 +98,10 @@ bool LayoutManagerBase::IsChildIncludedInLayout(const View* child,
   return !it->second.ignored && (include_hidden || it->second.can_be_visible);
 }
 
+void LayoutManagerBase::LayoutImpl() {
+  ApplyLayout(GetProposedLayout(host_view_->size()));
+}
+
 void LayoutManagerBase::ApplyLayout(const ProposedLayout& layout) {
   for (auto& child_layout : layout.child_layouts) {
     DCHECK_EQ(host_view_, child_layout.child_view->parent());

ui/views/layout/layout_manager_base.h

@@ -49,7 +49,7 @@ class VIEWS_EXPORT LayoutManagerBase : public LayoutManager {
   gfx::Size GetPreferredSize(const View* host) const override;
   gfx::Size GetMinimumSize(const View* host) const override;
   int GetPreferredHeightForWidth(const View* host, int width) const override;
-  void Layout(View* host) override;
+  void Layout(View* host) final;
 
  protected:
   LayoutManagerBase();
@@ -103,6 +103,11 @@ class VIEWS_EXPORT LayoutManagerBase : public LayoutManager {
   virtual ProposedLayout CalculateProposedLayout(
       const SizeBounds& size_bounds) const = 0;
 
+  // Does the actual work of laying out the host view and its children.
+  // Default implementation is just getting the proposed layout for the host
+  // size and then applying it.
+  virtual void LayoutImpl();
+
   // Applies |layout| to the children of the host view.
   void ApplyLayout(const ProposedLayout& layout);