Port BubbleDelegate tests to BubbleDialogDelegate

ui/views/bubble/bubble_dialog_delegate_unittest.cc

Index: ui/views/bubble/bubble_dialog_delegate_unittest.cc
diff --git a/ui/views/bubble/bubble_delegate_unittest.cc b/ui/views/bubble/bubble_dialog_delegate_unittest.cc
similarity index 66%
copy from ui/views/bubble/bubble_delegate_unittest.cc
copy to ui/views/bubble/bubble_dialog_delegate_unittest.cc
index f7656f061511f617a6db4a26f9840ebefc6ee956..27f6b2519b3eb9d3a4bc179292bed8a712476dd9 100644
--- a/ui/views/bubble/bubble_delegate_unittest.cc
+++ b/ui/views/bubble/bubble_dialog_delegate_unittest.cc
@@ -1,13 +1,14 @@
-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Copyright 2016 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/bubble/bubble_dialog_delegate.h"
+
 #include <stddef.h>
 
 #include "base/macros.h"
 #include "ui/base/hit_test.h"
 #include "ui/events/event_utils.h"
-#include "ui/views/bubble/bubble_delegate.h"
 #include "ui/views/bubble/bubble_frame_view.h"
 #include "ui/views/controls/button/label_button.h"
 #include "ui/views/test/test_widget_observer.h"
@@ -19,25 +20,21 @@ namespace views {
 
 namespace {
 
-class TestBubbleDelegateView : public BubbleDelegateView {
+class TestBubbleDialogDelegateView : public BubbleDialogDelegateView {
  public:
-  TestBubbleDelegateView(View* anchor_view)
-      : BubbleDelegateView(anchor_view, BubbleBorder::TOP_LEFT),
+  TestBubbleDialogDelegateView(View* anchor_view)
+      : BubbleDialogDelegateView(anchor_view, BubbleBorder::TOP_LEFT),
         view_(new View()) {
     view_->SetFocusable(true);
     AddChildView(view_);
   }
-  ~TestBubbleDelegateView() override {}
+  ~TestBubbleDialogDelegateView() override {}
 
-  void SetAnchorRectForTest(gfx::Rect rect) {
-    SetAnchorRect(rect);
-  }
+  void SetAnchorRectForTest(gfx::Rect rect) { SetAnchorRect(rect); }
 
-  void SetAnchorViewForTest(View* view) {
-    SetAnchorView(view);
-  }
+  void SetAnchorViewForTest(View* view) { SetAnchorView(view); }
 
-  // BubbleDelegateView overrides:
+  // BubbleDialogDelegateView overrides:
   View* GetInitiallyFocusedView() override { return view_; }
   gfx::Size GetPreferredSize() const override { return gfx::Size(200, 200); }
 
@@ -45,16 +42,18 @@ class TestBubbleDelegateView : public BubbleDelegateView {
     return GetBubbleFrameView();
   }
 
+  using BubbleDialogDelegateView::GetBubbleFrameView;

[msw, 2016/03/17 17:28:26]

This is redundant with GetBubbleFrameViewForTest; can you make them consistent?
If you go with 'using', please do the same for SetAnchor[Rect|View]ForTest().

[Evan Stade, 2016/03/17 22:41:34]

Done.

+
  private:
   View* view_;
 
-  DISALLOW_COPY_AND_ASSIGN(TestBubbleDelegateView);
+  DISALLOW_COPY_AND_ASSIGN(TestBubbleDialogDelegateView);
 };
 
-class BubbleDelegateTest : public ViewsTestBase {
+class BubbleDialogDelegateTest : public ViewsTestBase {
  public:
-  BubbleDelegateTest() {}
-  ~BubbleDelegateTest() override {}
+  BubbleDialogDelegateTest() {}
+  ~BubbleDialogDelegateTest() override {}
 
   // Creates a test widget that owns its native widget.
   Widget* CreateTestWidget() {

[msw, 2016/03/17 17:28:26]

nit: this could be a static local function, then we don't need the subclass.

[Evan Stade, 2016/03/17 22:41:34]

CreateParams is a (non-static) ViewsTestBase method

@@ -66,17 +65,30 @@ class BubbleDelegateTest : public ViewsTestBase {
   }
 
  private:
-  DISALLOW_COPY_AND_ASSIGN(BubbleDelegateTest);
+  DISALLOW_COPY_AND_ASSIGN(BubbleDialogDelegateTest);
+};
+
+/*

[msw, 2016/03/17 17:28:26]

You probably meant to remove this.

[Evan Stade, 2016/03/17 22:41:34]

right you are

+class TestBubbleDialogDelegateView : public BubbleDialogDelegateView {
+ public
+  TestBubbleDialogDelegateView(View* anchor_view, BubbleBorder border)
+      : BubbleDialogDelegateView(anchor_view, border) {}
+  ~TestBubbleDialogDelegateView() override {}
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(TestBubbleDialogDelegateView);
 };
+*/
 
 }  // namespace
 
-TEST_F(BubbleDelegateTest, CreateDelegate) {
+TEST_F(BubbleDialogDelegateTest, CreateDelegate) {
   scoped_ptr<Widget> anchor_widget(CreateTestWidget());

[msw, 2016/03/17 17:28:26]

Every test does this same pattern (anchor widget, new TestBubbleBlahBlah), maybe
make that part of a Setup override or ctor for the test subclass?

[Evan Stade, 2016/03/17 22:41:34]

that seems tricky because most tests do something to the bubble delegate before
creating the bubble widget (the common pattern has a step in the middle that's
unique to each test).

-  BubbleDelegateView* bubble_delegate = new BubbleDelegateView(
-      anchor_widget->GetContentsView(), BubbleBorder::NONE);
+  TestBubbleDialogDelegateView* bubble_delegate =
+      new TestBubbleDialogDelegateView(anchor_widget->GetContentsView());
   bubble_delegate->set_color(SK_ColorGREEN);
-  Widget* bubble_widget = BubbleDelegateView::CreateBubble(bubble_delegate);
+  Widget* bubble_widget =
+      BubbleDialogDelegateView::CreateBubble(bubble_delegate);
   EXPECT_EQ(bubble_delegate, bubble_widget->widget_delegate());
   EXPECT_EQ(bubble_widget, bubble_delegate->GetWidget());
   test::TestWidgetObserver bubble_observer(bubble_widget);
@@ -91,13 +103,14 @@ TEST_F(BubbleDelegateTest, CreateDelegate) {
   EXPECT_TRUE(bubble_observer.widget_closed());
 }
 
-TEST_F(BubbleDelegateTest, CloseAnchorWidget) {
+TEST_F(BubbleDialogDelegateTest, CloseAnchorWidget) {
   scoped_ptr<Widget> anchor_widget(CreateTestWidget());
-  BubbleDelegateView* bubble_delegate = new BubbleDelegateView(
-      anchor_widget->GetContentsView(), BubbleBorder::NONE);
+  BubbleDialogDelegateView* bubble_delegate =
+      new TestBubbleDialogDelegateView(anchor_widget->GetContentsView());
   // Preventing close on deactivate should not prevent closing with the anchor.
   bubble_delegate->set_close_on_deactivate(false);
-  Widget* bubble_widget = BubbleDelegateView::CreateBubble(bubble_delegate);
+  Widget* bubble_widget =
+      BubbleDialogDelegateView::CreateBubble(bubble_delegate);
   EXPECT_EQ(bubble_delegate, bubble_widget->widget_delegate());
   EXPECT_EQ(bubble_widget, bubble_delegate->GetWidget());
   EXPECT_EQ(anchor_widget.get(), bubble_delegate->anchor_widget());
@@ -121,16 +134,17 @@ TEST_F(BubbleDelegateTest, CloseAnchorWidget) {
 // This test checks that the bubble delegate is capable to handle an early
 // destruction of the used anchor view. (Animations and delayed closure of the
 // bubble will call upon the anchor view to get its location).
-TEST_F(BubbleDelegateTest, CloseAnchorViewTest) {
+TEST_F(BubbleDialogDelegateTest, CloseAnchorViewTest) {
   // Create an anchor widget and add a view to be used as an anchor view.
   scoped_ptr<Widget> anchor_widget(CreateTestWidget());
   scoped_ptr<View> anchor_view(new View());
   anchor_widget->GetContentsView()->AddChildView(anchor_view.get());
-  TestBubbleDelegateView* bubble_delegate = new TestBubbleDelegateView(
-      anchor_view.get());
+  TestBubbleDialogDelegateView* bubble_delegate =
+      new TestBubbleDialogDelegateView(anchor_view.get());
   // Prevent flakes by avoiding closing on activation changes.
   bubble_delegate->set_close_on_deactivate(false);
-  Widget* bubble_widget = BubbleDelegateView::CreateBubble(bubble_delegate);
+  Widget* bubble_widget =
+      BubbleDialogDelegateView::CreateBubble(bubble_delegate);
 
   // Check that the anchor view is correct and set up an anchor view rect.
   // Make sure that this rect will get ignored (as long as the anchor view is
@@ -154,12 +168,12 @@ TEST_F(BubbleDelegateTest, CloseAnchorViewTest) {
 }
 
 // Testing that a move of the anchor view will lead to new bubble locations.
-TEST_F(BubbleDelegateTest, TestAnchorRectMovesWithViewTest) {
+TEST_F(BubbleDialogDelegateTest, TestAnchorRectMovesWithViewTest) {
   // Create an anchor widget and add a view to be used as anchor view.
   scoped_ptr<Widget> anchor_widget(CreateTestWidget());
-  TestBubbleDelegateView* bubble_delegate = new TestBubbleDelegateView(
-      anchor_widget->GetContentsView());
-  BubbleDelegateView::CreateBubble(bubble_delegate);
+  TestBubbleDialogDelegateView* bubble_delegate =
+      new TestBubbleDialogDelegateView(anchor_widget->GetContentsView());
+  BubbleDialogDelegateView::CreateBubble(bubble_delegate);
 
   anchor_widget->GetContentsView()->SetBounds(10, 10, 100, 100);
   const gfx::Rect view_rect = bubble_delegate->GetAnchorRect();
@@ -169,10 +183,10 @@ TEST_F(BubbleDelegateTest, TestAnchorRectMovesWithViewTest) {
   EXPECT_NE(view_rect.ToString(), view_rect_2.ToString());
 }
 
-TEST_F(BubbleDelegateTest, ResetAnchorWidget) {
+TEST_F(BubbleDialogDelegateTest, ResetAnchorWidget) {
   scoped_ptr<Widget> anchor_widget(CreateTestWidget());
-  BubbleDelegateView* bubble_delegate = new BubbleDelegateView(
-      anchor_widget->GetContentsView(), BubbleBorder::NONE);
+  BubbleDialogDelegateView* bubble_delegate =
+      new TestBubbleDialogDelegateView(anchor_widget->GetContentsView());
 
   // Make sure the bubble widget is parented to a widget other than the anchor
   // widget so that closing the anchor widget does not close the bubble widget.
@@ -180,7 +194,8 @@ TEST_F(BubbleDelegateTest, ResetAnchorWidget) {
   bubble_delegate->set_parent_window(parent_widget->GetNativeView());
   // Preventing close on deactivate should not prevent closing with the parent.
   bubble_delegate->set_close_on_deactivate(false);
-  Widget* bubble_widget = BubbleDelegateView::CreateBubble(bubble_delegate);
+  Widget* bubble_widget =
+      BubbleDialogDelegateView::CreateBubble(bubble_delegate);
   EXPECT_EQ(bubble_delegate, bubble_widget->widget_delegate());
   EXPECT_EQ(bubble_widget, bubble_delegate->GetWidget());
   EXPECT_EQ(anchor_widget.get(), bubble_delegate->anchor_widget());
@@ -209,21 +224,23 @@ TEST_F(BubbleDelegateTest, ResetAnchorWidget) {
   EXPECT_TRUE(bubble_observer.widget_closed());
 }
 
-TEST_F(BubbleDelegateTest, InitiallyFocusedView) {
+TEST_F(BubbleDialogDelegateTest, InitiallyFocusedView) {
   scoped_ptr<Widget> anchor_widget(CreateTestWidget());
-  BubbleDelegateView* bubble_delegate = new BubbleDelegateView(
-      anchor_widget->GetContentsView(), BubbleBorder::NONE);
-  Widget* bubble_widget = BubbleDelegateView::CreateBubble(bubble_delegate);
+  BubbleDialogDelegateView* bubble_delegate =
+      new TestBubbleDialogDelegateView(anchor_widget->GetContentsView());
+  Widget* bubble_widget =
+      BubbleDialogDelegateView::CreateBubble(bubble_delegate);
+  bubble_widget->Show();
   EXPECT_EQ(bubble_delegate->GetInitiallyFocusedView(),
             bubble_widget->GetFocusManager()->GetFocusedView());
   bubble_widget->CloseNow();
 }
 
-TEST_F(BubbleDelegateTest, NonClientHitTest) {
+TEST_F(BubbleDialogDelegateTest, NonClientHitTest) {
   scoped_ptr<Widget> anchor_widget(CreateTestWidget());
-  TestBubbleDelegateView* bubble_delegate =
-      new TestBubbleDelegateView(anchor_widget->GetContentsView());
-  BubbleDelegateView::CreateBubble(bubble_delegate);
+  TestBubbleDialogDelegateView* bubble_delegate =
+      new TestBubbleDialogDelegateView(anchor_widget->GetContentsView());
+  BubbleDialogDelegateView::CreateBubble(bubble_delegate);
   BubbleFrameView* frame = bubble_delegate->GetBubbleFrameView();
   const int border = frame->bubble_border()->GetBorderThickness();
 
@@ -231,9 +248,7 @@ TEST_F(BubbleDelegateTest, NonClientHitTest) {
     const int point;
     const int hit;
   } cases[] = {
-    { border,      HTNOWHERE },
-    { border + 50, HTCLIENT  },
-    { 1000,        HTNOWHERE },
+      {border, HTNOWHERE}, {border + 50, HTCLIENT}, {1000, HTNOWHERE},
   };
 
   for (size_t i = 0; i < arraysize(cases); ++i) {
@@ -243,11 +258,12 @@ TEST_F(BubbleDelegateTest, NonClientHitTest) {
   }
 }
 
-TEST_F(BubbleDelegateTest, VisibleWhenAnchorWidgetBoundsChanged) {
+TEST_F(BubbleDialogDelegateTest, VisibleWhenAnchorWidgetBoundsChanged) {
   scoped_ptr<Widget> anchor_widget(CreateTestWidget());
-  BubbleDelegateView* bubble_delegate = new BubbleDelegateView(
-      anchor_widget->GetContentsView(), BubbleBorder::NONE);
-  Widget* bubble_widget = BubbleDelegateView::CreateBubble(bubble_delegate);
+  BubbleDialogDelegateView* bubble_delegate =
+      new TestBubbleDialogDelegateView(anchor_widget->GetContentsView());
+  Widget* bubble_widget =
+      BubbleDialogDelegateView::CreateBubble(bubble_delegate);
   test::TestWidgetObserver bubble_observer(bubble_widget);
   EXPECT_FALSE(bubble_observer.widget_closed());
 
@@ -259,64 +275,60 @@ TEST_F(BubbleDelegateTest, VisibleWhenAnchorWidgetBoundsChanged) {
 }
 
 // Test that setting WidgetDelegate::set_can_activate() to false makes the
-// widget created via BubbleDelegateView::CreateBubble() not activatable.
-TEST_F(BubbleDelegateTest, NotActivatable) {
+// widget created via BubbleDialogDelegateView::CreateBubble() not activatable.
+TEST_F(BubbleDialogDelegateTest, NotActivatable) {
   scoped_ptr<Widget> anchor_widget(CreateTestWidget());
-  BubbleDelegateView* bubble_delegate = new BubbleDelegateView(
-      anchor_widget->GetContentsView(), BubbleBorder::NONE);
+  BubbleDialogDelegateView* bubble_delegate =
+      new TestBubbleDialogDelegateView(anchor_widget->GetContentsView());
   bubble_delegate->set_can_activate(false);
-  Widget* bubble_widget = BubbleDelegateView::CreateBubble(bubble_delegate);
+  Widget* bubble_widget =
+      BubbleDialogDelegateView::CreateBubble(bubble_delegate);
   bubble_widget->Show();
   EXPECT_FALSE(bubble_widget->CanActivate());
 }
 
-TEST_F(BubbleDelegateTest, CloseReasons) {
+TEST_F(BubbleDialogDelegateTest, CloseReasons) {
   {
     scoped_ptr<Widget> anchor_widget(CreateTestWidget());
-    BubbleDelegateView* bubble_delegate = new BubbleDelegateView(
-        anchor_widget->GetContentsView(), BubbleBorder::NONE);
+    BubbleDialogDelegateView* bubble_delegate =
+        new TestBubbleDialogDelegateView(anchor_widget->GetContentsView());
     bubble_delegate->set_close_on_deactivate(true);
-    Widget* bubble_widget = BubbleDelegateView::CreateBubble(bubble_delegate);
+    Widget* bubble_widget =
+        BubbleDialogDelegateView::CreateBubble(bubble_delegate);
     anchor_widget->Show();
     bubble_widget->Show();
     anchor_widget->Activate();
     EXPECT_TRUE(bubble_widget->IsClosed());
-    EXPECT_EQ(BubbleDelegateView::CloseReason::DEACTIVATION,

[msw, 2016/03/17 17:28:26]

Are close reasons no longer important? Rename this test?

[Evan Stade, 2016/03/17 22:41:34]

Done.

-              bubble_delegate->close_reason());
   }
 
   {
     scoped_ptr<Widget> anchor_widget(CreateTestWidget());

[msw, 2016/03/17 17:28:26]

These should probably be three separate tests...

[Evan Stade, 2016/03/17 22:41:34]

I've seen both styles used. I'm sort of ambivalent. I'm happy to keep logically
related test cases in the same test.

-    BubbleDelegateView* bubble_delegate = new BubbleDelegateView(
-        anchor_widget->GetContentsView(), BubbleBorder::NONE);
-    bubble_delegate->set_close_on_esc(true);
-    Widget* bubble_widget = BubbleDelegateView::CreateBubble(bubble_delegate);
+    BubbleDialogDelegateView* bubble_delegate =
+        new TestBubbleDialogDelegateView(anchor_widget->GetContentsView());
+    Widget* bubble_widget =
+        BubbleDialogDelegateView::CreateBubble(bubble_delegate);
     bubble_widget->Show();
-    // Cast as a test hack to access AcceleratorPressed() (which is protected
-    // in BubbleDelegate).
-    static_cast<View*>(bubble_delegate)
-        ->AcceleratorPressed(ui::Accelerator(ui::VKEY_ESCAPE, ui::EF_NONE));
+
+    ui::KeyEvent escape_event(ui::ET_KEY_PRESSED, ui::VKEY_ESCAPE, ui::EF_NONE);
+    bubble_widget->OnKeyEvent(&escape_event);
     EXPECT_TRUE(bubble_widget->IsClosed());
-    EXPECT_EQ(BubbleDelegateView::CloseReason::ESCAPE,
-              bubble_delegate->close_reason());
   }
 
   {
     scoped_ptr<Widget> anchor_widget(CreateTestWidget());
-    TestBubbleDelegateView* bubble_delegate =
-        new TestBubbleDelegateView(anchor_widget->GetContentsView());
-    Widget* bubble_widget = BubbleDelegateView::CreateBubble(bubble_delegate);
+    TestBubbleDialogDelegateView* bubble_delegate =
+        new TestBubbleDialogDelegateView(anchor_widget->GetContentsView());
+    Widget* bubble_widget =
+        BubbleDialogDelegateView::CreateBubble(bubble_delegate);
     bubble_widget->Show();
     BubbleFrameView* frame_view = bubble_delegate->GetBubbleFrameViewForTest();
     LabelButton* close_button = frame_view->close_;
     ASSERT_TRUE(close_button);
     frame_view->ButtonPressed(
         close_button,
-        ui::MouseEvent(ui::ET_MOUSE_PRESSED, gfx::Point(0, 0), gfx::Point(0, 0),
+        ui::MouseEvent(ui::ET_MOUSE_PRESSED, gfx::Point(), gfx::Point(),
                        ui::EventTimeForNow(), ui::EF_NONE, ui::EF_NONE));
     EXPECT_TRUE(bubble_widget->IsClosed());
-    EXPECT_EQ(BubbleDelegateView::CloseReason::CLOSE_BUTTON,
-              bubble_delegate->close_reason());
   }
 }