Fix layout of BubbleFrameView when there's only a close button in the

ui/views/window/dialog_delegate_unittest.cc

Index: ui/views/window/dialog_delegate_unittest.cc
diff --git a/ui/views/window/dialog_delegate_unittest.cc b/ui/views/window/dialog_delegate_unittest.cc
index d11cfddde8f6aabf0595da8212e837a7e92e24b4..a5f4ad63435e673ea786eceb2145e0ba3d6c0506 100644
--- a/ui/views/window/dialog_delegate_unittest.cc
+++ b/ui/views/window/dialog_delegate_unittest.cc
@@ -28,15 +28,7 @@ namespace {
 
 class TestDialog : public DialogDelegateView {
  public:
-  TestDialog()
-      : input_(new views::Textfield()),
-        canceled_(false),
-        accepted_(false),
-        closed_(false),
-        closeable_(false),
-        should_handle_escape_(false) {
-    AddChildView(input_);
-  }
+  TestDialog() : input_(new views::Textfield()) { AddChildView(input_); }
   ~TestDialog() override {}
 
   void Init() {
@@ -52,6 +44,8 @@ class TestDialog : public DialogDelegateView {
     return !title_.empty();
   }
 
+  bool ShouldShowCloseButton() const override { return show_close_button_; }
+
   // DialogDelegateView overrides:
   bool Cancel() override {
     canceled_ = true;
@@ -66,7 +60,6 @@ class TestDialog : public DialogDelegateView {
     return closeable_;
   }
 
-  // DialogDelegateView overrides:
   gfx::Size GetPreferredSize() const override { return gfx::Size(200, 200); }
   bool AcceleratorPressed(const ui::Accelerator& accelerator) override {
     return should_handle_escape_;
@@ -92,6 +85,9 @@ class TestDialog : public DialogDelegateView {
   }
 
   void set_title(const base::string16& title) { title_ = title; }
+  void set_show_close_button(bool show_close) {
+    show_close_button_ = show_close;
+  }
   void set_should_handle_escape(bool should_handle_escape) {
     should_handle_escape_ = should_handle_escape;
   }
@@ -100,13 +96,14 @@ class TestDialog : public DialogDelegateView {
 
  private:
   views::Textfield* input_;
-  bool canceled_;
-  bool accepted_;
-  bool closed_;
+  bool canceled_ = false;
+  bool accepted_ = false;
+  bool closed_ = false;
   // Prevent the dialog from closing, for repeated ok and cancel button clicks.
   bool closeable_;
   base::string16 title_;
-  bool should_handle_escape_;
+  bool show_close_button_ = true;
+  bool should_handle_escape_ = false;
 
   DISALLOW_COPY_AND_ASSIGN(TestDialog);
 };
@@ -118,9 +115,8 @@ class DialogTest : public ViewsTestBase {
 
   void SetUp() override {
     ViewsTestBase::SetUp();
-    dialog_ = new TestDialog();
-    dialog_->Init();
-    DialogDelegate::CreateDialogWidget(dialog_, GetContext(), nullptr)->Show();
+    InitializeDialog();
+    ShowDialog();
   }
 
   void TearDown() override {
@@ -128,6 +124,18 @@ class DialogTest : public ViewsTestBase {
     ViewsTestBase::TearDown();
   }
 
+  void InitializeDialog() {
+    if (dialog_)
+      dialog_->TearDown();
+
+    dialog_ = new TestDialog();
+    dialog_->Init();
+  }
+
+  void ShowDialog() {
+    DialogDelegate::CreateDialogWidget(dialog_, GetContext(), nullptr)->Show();
+  }
+
   void SimulateKeyEvent(const ui::KeyEvent& event) {
     ui::KeyEvent event_copy = event;
     if (dialog()->GetFocusManager()->OnKeyEvent(event_copy))
@@ -205,8 +213,37 @@ TEST_F(DialogTest, HitTest_HiddenTitle) {
   const int border = frame->bubble_border()->GetBorderThickness();
 
   struct {
-    const int point;
-    const int hit;
+    int point;
+    int hit;
+  } cases[] = {
+      {border, HTSYSMENU},
+      {border + 10, HTSYSMENU},
+      {border + 20, HTNOWHERE},
+      {border + 50, HTCLIENT /* Space is reserved for the close button. */},
+      {border + 60, HTCLIENT},
+      {1000, HTNOWHERE},
+  };
+
+  for (size_t i = 0; i < arraysize(cases); ++i) {
+    gfx::Point point(cases[i].point, cases[i].point);
+    EXPECT_EQ(cases[i].hit, frame->NonClientHitTest(point))
+        << " case " << i << " with border: " << border << ", at point "
+        << cases[i].point;
+  }
+}
+
+TEST_F(DialogTest, HitTest_HiddenTitleNoCloseButton) {
+  InitializeDialog();
+  dialog()->set_show_close_button(false);
+  ShowDialog();

[Peter Kasting, 2017/04/13 23:47:35]

Cool, this route is more readable than the old code.  Can we rewrite
BoundsAccommodateTitle to make use of this too?  I'm thinking:

* Capture the relevant metrics (with and without title) from the dialog, check
that without title is shorter than with
* Then use init/show to make a new dialog and set its title before showing, and
check that it's the same as the previous with-title dialog

[Evan Stade, 2017/04/13 23:51:24]

honestly, would rather not let this change snowball any further.

[Peter Kasting, 2017/04/13 23:53:35]

OK.  I just figured you introduced this in part because of the needs of that
function, so it made sense to make use of it there.  If it would add too much
hassle, that's cool too.

+
+  const NonClientView* view = dialog()->GetWidget()->non_client_view();
+  BubbleFrameView* frame = static_cast<BubbleFrameView*>(view->frame_view());
+  const int border = frame->bubble_border()->GetBorderThickness();
+
+  struct {
+    int point;
+    int hit;
   } cases[] = {
     { border,      HTSYSMENU },
     { border + 10, HTSYSMENU },