GridLayout: Guard against layout queries while adding a view.

ui/views/layout/grid_layout_unittest.cc

Index: ui/views/layout/grid_layout_unittest.cc
diff --git a/ui/views/layout/grid_layout_unittest.cc b/ui/views/layout/grid_layout_unittest.cc
index 0d9dc06ec06f9d256e0d9c16939ed9962863955f..35cc80b3a6e68ecfb1e4a1c6baa65c5657551db7 100644
--- a/ui/views/layout/grid_layout_unittest.cc
+++ b/ui/views/layout/grid_layout_unittest.cc
@@ -5,6 +5,7 @@
 #include "ui/views/layout/grid_layout.h"
 
 #include "base/compiler_specific.h"
+#include "base/test/gtest_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/views/view.h"
 
@@ -34,6 +35,36 @@ class SettableSizeView : public View {
   DISALLOW_COPY_AND_ASSIGN(SettableSizeView);
 };
 
+// A test view that wants to alter its preferred size and re-layout when it gets
+// added to the View hierarchy.
+class LayoutOnAddView : public SettableSizeView {
+ public:
+  LayoutOnAddView() : SettableSizeView(gfx::Size(10, 10)) {}
+
+  void set_target_size(const gfx::Size& target_size) {
+    target_size_ = target_size;
+  }
+
+  // View:
+  void ViewHierarchyChanged(
+      const ViewHierarchyChangedDetails& details) override {
+    if (GetPreferredSize() == target_size_)
+      return;
+
+    // Contrive a realistic thing that a View might what to do, but which would
+    // break the layout machinery. Note an override of OnNativeThemeChanged()
+    // would be more compelling, but there is no Widget in this test harness.
+    set_pref(target_size_);
+    PreferredSizeChanged();
+    parent()->Layout();
+  }
+
+ private:
+  gfx::Size target_size_;
+
+  DISALLOW_COPY_AND_ASSIGN(LayoutOnAddView);
+};
+
 // A view with fixed circumference that trades height for width.
 class FlexibleView : public View {
  public:
@@ -737,4 +768,29 @@ TEST_F(GridLayoutTest, MinimumPreferredSize) {
   RemoveAll();
 }
 
+// Test that attempting a Layout() while nested in AddView() causes a DCHECK.
+// GridLayout must guard against this as it hasn't yet updated the internal
+// structures it uses to calculate Layout, so will give bogus results.
+TEST_F(GridLayoutTest, LayoutOnAddDeath) {
+  // Don't use the |layout| data member from the test harness, otherwise
+  // SetLayoutManager() can take not take ownership.
+  GridLayout* grid_layout = new GridLayout(&host);
+  host.SetLayoutManager(grid_layout);
+  ColumnSet* set = grid_layout->AddColumnSet(0);
+  set->AddColumn(GridLayout::FILL, GridLayout::FILL, 0, GridLayout::USE_PREF, 0,
+                 0);
+  grid_layout->StartRow(0, 0);
+  LayoutOnAddView view;
+  EXPECT_DCHECK_DEATH(grid_layout->AddView(&view));
+  // Death tests use fork(), so nothing should be added here.
+  EXPECT_FALSE(view.parent());
+
+  // If the View has nothing to change, adding should succeed.
+  view.set_target_size(view.GetPreferredSize());
+  grid_layout->AddView(&view);
+  EXPECT_TRUE(view.parent());
+
+  RemoveAll();
+}
+
 }  // namespace views