UpdateWindowTitle before SetInitialBounds in Widget::Init.

Reverse the ordering of these Widget::Init operations.
BubbleFrameView uses title/close presence for sizing.
(requires title/close to be set before bounds init)

DialogDelegate::CreateNewStyleFrameView did this explicitly.
(that changed in http://crrev.com/214637, seems less hacky)

Add a unit test, and cleanup a related test.

BUG=267343
TEST=Dialogs are sized properly, especially non-tab/content-modal dialogs (add wifi, import lock, screen sharing, etc.); see the bug and its dups for examples.
R=sky@chromium.org,ben@chromium.org

Review URL: https://chromiumcodereview.appspot.com/21967002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@215805 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 51 insertions, 25 deletions

diff --git a/ui/views/widget/widget.cc b/ui/views/widget/widget.cc
index 0b357f8..e97c950 100644
--- a/ui/views/widget/widget.cc
+++ b/ui/views/widget/widget.cc
@@ -348,12 +348,14 @@ void Widget::Init(const InitParams& in_params) {
     non_client_view_->set_client_view(widget_delegate_->CreateClientView(this));
     non_client_view_->SetOverlayView(widget_delegate_->CreateOverlayView());
     SetContentsView(non_client_view_);
+    // Initialize the window's title before setting the window's initial bounds;
+    // the frame view's preferred height may depend on the presence of a title.
+    UpdateWindowTitle();
     SetInitialBounds(params.bounds);
     if (params.show_state == ui::SHOW_STATE_MAXIMIZED)
       Maximize();
     else if (params.show_state == ui::SHOW_STATE_MINIMIZED)
       Minimize();
-    UpdateWindowTitle();
   } else if (params.delegate) {
     SetContentsView(params.delegate->GetContentsView());
     SetInitialBoundsForFramelessWindow(params.bounds);
diff --git a/ui/views/window/dialog_delegate_unittest.cc b/ui/views/window/dialog_delegate_unittest.cc
index d7471e7..ba891f4 100644
--- a/ui/views/window/dialog_delegate_unittest.cc
+++ b/ui/views/window/dialog_delegate_unittest.cc
@@ -37,8 +37,9 @@ class TestDialog : public DialogDelegateView, public ButtonListener {
     return closeable_;
   }
 
-  // View override:
+  // DialogDelegateView overrides:
   virtual gfx::Size GetPreferredSize() OVERRIDE { return gfx::Size(200, 200); }
+  virtual string16 GetWindowTitle() const OVERRIDE { return title_; }
 
   // ButtonListener override:
   virtual void ButtonPressed(Button* sender, const ui::Event& event) OVERRIDE {
@@ -73,12 +74,15 @@ class TestDialog : public DialogDelegateView, public ButtonListener {
     GetWidget()->Close();
   }
 
+  void set_title(const string16& title) { title_ = title; }
+
  private:
   bool canceled_;
   bool accepted_;
   // Prevent the dialog from closing, for repeated ok and cancel button clicks.
   bool closeable_;
   Button* last_pressed_button_;
+  string16 title_;
 
   DISALLOW_COPY_AND_ASSIGN(TestDialog);
 };
@@ -200,31 +204,51 @@ TEST_F(DialogTest, RemoveDefaultButton) {
 }
 
 TEST_F(DialogTest, HitTest) {
-  const NonClientView* view = dialog()->GetWidget()->non_client_view();
+  if (!DialogDelegate::UseNewStyle())
+    return;
 
-  if (DialogDelegate::UseNewStyle()) {
-    // Ensure that the new style's BubbleFrameView hit-tests as expected.
-    BubbleFrameView* frame = static_cast<BubbleFrameView*>(view->frame_view());
-    const int border = frame->bubble_border()->GetBorderThickness();
-
-    struct {
-      const int point;
-      const int hit;
-    } cases[] = {
-      { border,      HTSYSMENU },
-      { border + 10, HTSYSMENU },
-      { border + 20, HTCAPTION },
-      { border + 40, HTCLIENT  },
-      { border + 50, HTCLIENT  },
-      { 1000,        HTNOWHERE },
-    };
-
-    for (size_t i = 0; i < ARRAYSIZE_UNSAFE(cases); ++i) {
-      gfx::Point point(cases[i].point, cases[i].point);
-      EXPECT_EQ(cases[i].hit, frame->NonClientHitTest(point))
-          << " with border: " << border << ", at point " << cases[i].point;
-    }
+  // Ensure that the new style's BubbleFrameView hit-tests as expected.
+  const NonClientView* view = dialog()->GetWidget()->non_client_view();
+  BubbleFrameView* frame = static_cast<BubbleFrameView*>(view->frame_view());
+  const int border = frame->bubble_border()->GetBorderThickness();
+
+  struct {
+    const int point;
+    const int hit;
+  } cases[] = {
+    { border,      HTSYSMENU },
+    { border + 10, HTSYSMENU },
+    { border + 20, HTCAPTION },
+    { border + 40, HTCLIENT  },
+    { border + 50, HTCLIENT  },
+    { 1000,        HTNOWHERE },
+  };
+
+  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(cases); ++i) {
+    gfx::Point point(cases[i].point, cases[i].point);
+    EXPECT_EQ(cases[i].hit, frame->NonClientHitTest(point))
+        << " with border: " << border << ", at point " << cases[i].point;
   }
 }
 
+TEST_F(DialogTest, InitialBoundsAccommodateTitle) {
+  if (!DialogDelegate::UseNewStyle())
+    return;
+
+  TestDialog* titled_dialog(new TestDialog());
+  titled_dialog->set_title(ASCIIToUTF16("Title"));
+  DialogDelegate::CreateDialogWidget(titled_dialog, GetContext(), NULL);
+
+  // Titled dialogs have taller initial frame bounds than untitled dialogs.
+  EXPECT_GT(titled_dialog->GetWidget()->GetWindowBoundsInScreen().height(),
+            dialog()->GetWidget()->GetWindowBoundsInScreen().height());
+
+  // Giving the default test dialog a title will make the bounds the same.
+  dialog()->set_title(ASCIIToUTF16("Title"));
+  dialog()->GetWidget()->UpdateWindowTitle();
+  View* frame = dialog()->GetWidget()->non_client_view()->frame_view();
+  EXPECT_EQ(titled_dialog->GetWidget()->GetWindowBoundsInScreen().height(),
+            frame->GetPreferredSize().height());
+}
+
 }  // namespace views