Use GridLayout for DialogClientView's button row.

ui/views/window/dialog_client_view_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ui/views/window/dialog_client_view.h"
 
 #include "base/macros.h"
 #include "base/strings/utf_string_conversions.h"
 #include "build/build_config.h"
 #include "ui/base/ui_base_types.h"
@@ skipping 25 matching lines @@
     widget_->Init(params);
     EXPECT_EQ(this, GetContentsView());
   }
 
   void TearDown() override {
     widget_->CloseNow();
     WidgetTest::TearDown();
   }
 
   // DialogDelegateView:
+  gfx::Size GetPreferredSize() const override { return preferred_size_; }
+  gfx::Size GetMinimumSize() const override { return min_size_; }
+  gfx::Size GetMaximumSize() const override { return max_size_; }
   ClientView* CreateClientView(Widget* widget) override {
     client_view_ = new DialogClientView(widget, this);
     return client_view_;
   }
 
   bool ShouldUseCustomFrame() const override { return false; }
 
   void DeleteDelegate() override {
     // DialogDelegateView would delete this, but |this| is owned by the test.
   }
@@ skipping 39 matching lines @@
     EXPECT_FALSE(next_extra_view_);
     next_extra_view_ = base::WrapUnique(view);
     client_view_->UpdateDialogButtons();
     EXPECT_FALSE(next_extra_view_);
   }
 
   // Sets the extra view padding.
   void SetExtraViewPadding(int padding) {
     DCHECK(!extra_view_padding_);
     extra_view_padding_.reset(new int(padding));
-    client_view_->Layout();
+    client_view_->UpdateDialogButtons();
+  }
+
+  void SetSizeConstraints(const gfx::Size& min_size,
+                          const gfx::Size& preferred_size,
+                          const gfx::Size& max_size) {
+    min_size_ = min_size;
+    preferred_size_ = preferred_size;
+    max_size_ = max_size;
   }
 
   View* FocusableViewAfter(View* view) {
     const bool dont_loop = false;
     const bool reverse = false;
     return GetFocusManager()->GetNextFocusableView(view, GetWidget(), reverse,
                                                    dont_loop);
   }
 
   DialogClientView* client_view() { return client_view_; }
 
  private:
   // The dialog Widget.
   Widget* widget_ = nullptr;
 
   // The DialogClientView that's being tested. Owned by |widget_|.
   DialogClientView* client_view_;
 
   // The bitmask of buttons to show in the dialog.
   int dialog_buttons_ = ui::DIALOG_BUTTON_NONE;
 
   // Set and cleared in SetExtraView().
   std::unique_ptr<View> next_extra_view_;
 
   std::unique_ptr<int> extra_view_padding_;
 
+  gfx::Size preferred_size_;
+  gfx::Size min_size_;
+  gfx::Size max_size_;
+
   DISALLOW_COPY_AND_ASSIGN(DialogClientViewTest);
 };
 
 TEST_F(DialogClientViewTest, UpdateButtons) {
   // This dialog should start with no buttons.
   EXPECT_EQ(GetDialogButtons(), ui::DIALOG_BUTTON_NONE);
   EXPECT_EQ(NULL, client_view()->ok_button());
   EXPECT_EQ(NULL, client_view()->cancel_button());
   const int height_without_buttons = GetUpdatedClientBounds().height();
 
@@ skipping 39 matching lines @@
 }
 
 // Test that views inside the dialog client view have the correct focus order.
 TEST_F(DialogClientViewTest, SetupFocusChain) {
 #if defined(OS_WIN) || defined(OS_CHROMEOS)
   const bool kIsOkButtonOnLeftSide = true;
 #else
   const bool kIsOkButtonOnLeftSide = false;
 #endif
 
+  GetContentsView()->SetFocusBehavior(View::FocusBehavior::ALWAYS);
   // Initially the dialog client view only contains the content view.
-  EXPECT_EQ(nullptr, GetContentsView()->GetNextFocusableView());
+  EXPECT_EQ(GetContentsView(), FocusableViewAfter(GetContentsView()));
 
   // Add OK and cancel buttons.
   SetDialogButtons(ui::DIALOG_BUTTON_OK | ui::DIALOG_BUTTON_CANCEL);
 
   if (kIsOkButtonOnLeftSide) {
     EXPECT_EQ(client_view()->ok_button(),
-              GetContentsView()->GetNextFocusableView());
+              FocusableViewAfter(GetContentsView()));
     EXPECT_EQ(client_view()->cancel_button(),
-              client_view()->ok_button()->GetNextFocusableView());
-    EXPECT_EQ(nullptr, client_view()->cancel_button()->GetNextFocusableView());
+              FocusableViewAfter(client_view()->ok_button()));
+    EXPECT_EQ(GetContentsView(),
+              FocusableViewAfter(client_view()->cancel_button()));
   } else {
     EXPECT_EQ(client_view()->cancel_button(),
-              GetContentsView()->GetNextFocusableView());
+              FocusableViewAfter(GetContentsView()));
     EXPECT_EQ(client_view()->ok_button(),
-              client_view()->cancel_button()->GetNextFocusableView());
-    EXPECT_EQ(nullptr, client_view()->ok_button()->GetNextFocusableView());
+              FocusableViewAfter(client_view()->cancel_button()));
+    EXPECT_EQ(GetContentsView(),
+              FocusableViewAfter(client_view()->ok_button()));
   }
 
   // Add extra view and remove OK button.
   View* extra_view = new StaticSizedView(gfx::Size(200, 200));
   extra_view->SetFocusBehavior(View::FocusBehavior::ALWAYS);
   SetExtraView(extra_view);
   SetDialogButtons(ui::DIALOG_BUTTON_CANCEL);
 
-  EXPECT_EQ(extra_view, GetContentsView()->GetNextFocusableView());
-  EXPECT_EQ(client_view()->cancel_button(), extra_view->GetNextFocusableView());
-  EXPECT_EQ(nullptr, client_view()->cancel_button()->GetNextFocusableView());
+  EXPECT_EQ(extra_view, FocusableViewAfter(GetContentsView()));
+  EXPECT_EQ(client_view()->cancel_button(), FocusableViewAfter(extra_view));
+  EXPECT_EQ(GetContentsView(), FocusableViewAfter(client_view()));
 
   // Add a dummy view to the contents view. Consult the FocusManager for the
   // traversal order since it now spans different levels of the view hierarchy.
   View* dummy_view = new StaticSizedView(gfx::Size(200, 200));
   dummy_view->SetFocusBehavior(View::FocusBehavior::ALWAYS);
+  GetContentsView()->SetFocusBehavior(View::FocusBehavior::NEVER);
   GetContentsView()->AddChildView(dummy_view);
   EXPECT_EQ(dummy_view, FocusableViewAfter(client_view()->cancel_button()));
   EXPECT_EQ(extra_view, FocusableViewAfter(dummy_view));
   EXPECT_EQ(client_view()->cancel_button(), FocusableViewAfter(extra_view));
 
   // Views are added to the contents view, not the client view, so the focus
   // chain within the client view is not affected.
   EXPECT_EQ(nullptr, client_view()->cancel_button()->GetNextFocusableView());
 }
 
 // Test that the contents view gets its preferred size in the basic dialog
 // configuration.
 TEST_F(DialogClientViewTest, ContentsSize) {
   CheckContentsIsSetToPreferredSize();
   EXPECT_EQ(GetContentsView()->size(), client_view()->size());
   // There's nothing in the contents view (i.e. |this|), so it should be 0x0.
   EXPECT_EQ(gfx::Size(), client_view()->size());
 }
 
 // Test the effect of the button strip on layout.
 TEST_F(DialogClientViewTest, LayoutWithButtons) {
   SetDialogButtons(ui::DIALOG_BUTTON_OK | ui::DIALOG_BUTTON_CANCEL);
   CheckContentsIsSetToPreferredSize();
 
   EXPECT_LT(GetContentsView()->bounds().bottom(),
             client_view()->bounds().bottom());
-  gfx::Size no_extra_view_size = client_view()->bounds().size();
+  const gfx::Size no_extra_view_size = client_view()->bounds().size();
 
   View* extra_view = new StaticSizedView(gfx::Size(200, 200));
   SetExtraView(extra_view);
   CheckContentsIsSetToPreferredSize();
   EXPECT_GT(client_view()->bounds().height(), no_extra_view_size.height());
-  int width_of_dialog = client_view()->bounds().width();
-  int width_of_extra_view = extra_view->bounds().width();
+  const int width_of_dialog_small_padding = client_view()->width();
 
   // Try with an adjusted padding for the extra view.
   SetExtraViewPadding(250);
   CheckContentsIsSetToPreferredSize();
-  EXPECT_GT(client_view()->bounds().width(), width_of_dialog);
+  EXPECT_GT(client_view()->bounds().width(), width_of_dialog_small_padding);
 
-  // Visibility of extra view is respected.
+  const gfx::Size with_extra_view_size = client_view()->size();
+  EXPECT_NE(no_extra_view_size, with_extra_view_size);
+
+  // Hiding the extra view removes it as well as the extra padding.
   extra_view->SetVisible(false);
   CheckContentsIsSetToPreferredSize();
-  EXPECT_EQ(no_extra_view_size.height(), client_view()->bounds().height());
-  EXPECT_EQ(no_extra_view_size.width(), client_view()->bounds().width());
+  EXPECT_EQ(no_extra_view_size, client_view()->size());
 
-  // Try with a reduced-size dialog.
+  // Making it visible again adds it all back.
   extra_view->SetVisible(true);
-  client_view()->SetBoundsRect(gfx::Rect(gfx::Point(0, 0), no_extra_view_size));
-  client_view()->Layout();
-  EXPECT_GT(width_of_extra_view, extra_view->bounds().width());
+  CheckContentsIsSetToPreferredSize();
+  EXPECT_EQ(with_extra_view_size, client_view()->size());
+
+  // Leave |extra_view| hidden. It should still have a parent, to ensure it is
+  // owned by a View hierarchy and gets deleted.
+  extra_view->SetVisible(false);
+  EXPECT_TRUE(extra_view->parent());
+}
+
+// Ensure the minimum, maximum and preferred sizes of the contents view are
+// respected by the client view, and that the client view includes the button
+// row in its minimum and preferred size calculations.
+TEST_F(DialogClientViewTest, MinMaxPreferredSize) {
+  SetDialogButtons(ui::DIALOG_BUTTON_OK | ui::DIALOG_BUTTON_CANCEL);
+  const gfx::Size buttons_size = client_view()->GetPreferredSize();
+  EXPECT_FALSE(buttons_size.IsEmpty());
+
+  // When the contents view has no preference, just fit the buttons. The
+  // maximum size should be unconstrained in both directions.
+  EXPECT_EQ(buttons_size, client_view()->GetMinimumSize());
+  EXPECT_EQ(gfx::Size(), client_view()->GetMaximumSize());
+
+  // Ensure buttons are between these widths, for the constants below.
+  EXPECT_LT(20, buttons_size.width());
+  EXPECT_GT(300, buttons_size.width());
+
+  // With no buttons, client view should match the contents view.
+  SetDialogButtons(ui::DIALOG_BUTTON_NONE);
+  SetSizeConstraints(gfx::Size(10, 15), gfx::Size(20, 25), gfx::Size(300, 350));
+  EXPECT_EQ(gfx::Size(10, 15), client_view()->GetMinimumSize());
+  EXPECT_EQ(gfx::Size(20, 25), client_view()->GetPreferredSize());
+  EXPECT_EQ(gfx::Size(300, 350), client_view()->GetMaximumSize());
+
+  // With buttons, size should increase vertically only.
+  SetDialogButtons(ui::DIALOG_BUTTON_OK | ui::DIALOG_BUTTON_CANCEL);
+  EXPECT_EQ(gfx::Size(buttons_size.width(), 15 + buttons_size.height()),
+            client_view()->GetMinimumSize());
+  EXPECT_EQ(gfx::Size(buttons_size.width(), 25 + buttons_size.height()),
+            client_view()->GetPreferredSize());
+  EXPECT_EQ(gfx::Size(300, 350 + buttons_size.height()),
+            client_view()->GetMaximumSize());
+
+  // If the contents view gets bigger, it should take over the width.
+  SetSizeConstraints(gfx::Size(400, 450), gfx::Size(500, 550),
+                     gfx::Size(600, 650));
+  EXPECT_EQ(gfx::Size(400, 450 + buttons_size.height()),
+            client_view()->GetMinimumSize());
+  EXPECT_EQ(gfx::Size(500, 550 + buttons_size.height()),
+            client_view()->GetPreferredSize());
+  EXPECT_EQ(gfx::Size(600, 650 + buttons_size.height()),
+            client_view()->GetMaximumSize());
 }
 
 }  // namespace views

[Bret, 2017/03/01 00:29:50]

no new test for linking the button sizes?

[tapted, 2017/03/01 10:44:58]

Done.