views: break deps on DialogClientView, part 1 of n

This change begins breaking dependencies outside //ui/views on
DialogClientView. The overarching goal here is to make DialogClientView
private to //ui/views, with DialogDelegate presenting the only interface
for clients of Views to manage/interact with dialogs. This will also
make it easier to move some logic (ie: the AcceptDialog/CancelDialog
wrappers this change adds) entirely into DialogDelegate later on.

Bug: 1011446
Change-Id: I49c1b435ff57faf41beae7b51c6a5f430f0631cb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1912805Reviewed-by: Peter Boström <pbos@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#715269}

chrome/browser/chromeos/power/idle_action_warning_dialog_view.cc

@@ -21,7 +21,6 @@
 #include "ui/views/controls/label.h"
 #include "ui/views/layout/fill_layout.h"
 #include "ui/views/widget/widget.h"
-#include "ui/views/window/dialog_client_view.h"
 
 namespace chromeos {
 
@@ -58,7 +57,7 @@ IdleActionWarningDialogView::IdleActionWarningDialogView(
 
 void IdleActionWarningDialogView::CloseDialog() {
   update_timer_.Stop();
-  GetDialogClientView()->CancelWindow();
+  CancelDialog();
 }
 
 void IdleActionWarningDialogView::Update(base::TimeTicks idle_action_time) {

chrome/browser/chromeos/ui/echo_dialog_view.cc

@@ -19,7 +19,6 @@
 #include "ui/views/controls/styled_label.h"
 #include "ui/views/layout/fill_layout.h"
 #include "ui/views/widget/widget.h"
-#include "ui/views/window/dialog_client_view.h"
 
 namespace {
 

chrome/browser/ui/views/chrome_cleaner_reboot_dialog_win.cc

@@ -21,7 +21,6 @@
 #include "ui/views/controls/button/label_button.h"
 #include "ui/views/layout/layout_provider.h"
 #include "ui/views/widget/widget.h"
-#include "ui/views/window/dialog_client_view.h"
 
 namespace chrome {
 
@@ -91,8 +90,7 @@ base::string16 ChromeCleanerRebootDialog::GetWindowTitle() const {
 views::View* ChromeCleanerRebootDialog::GetInitiallyFocusedView() {
   // Set focus away from the Restart/OK button to prevent accidental prompt
   // acceptance if the user is typing as the dialog appears.
-  const views::DialogClientView* dcv = GetDialogClientView();
-  return dcv ? dcv->cancel_button() : nullptr;
+  return GetCancelButton();
 }
 
 // DialogDelegate overrides.

chrome/browser/ui/views/content_setting_bubble_contents.cc

@@ -49,7 +49,6 @@
 #include "ui/views/layout/box_layout.h"
 #include "ui/views/layout/grid_layout.h"
 #include "ui/views/native_cursor.h"
-#include "ui/views/window/dialog_client_view.h"
 
 namespace {
 
@@ -682,7 +681,6 @@ void ContentSettingBubbleContents::ButtonPressed(views::Button* sender,
 
     // Toggling the check state may change the dialog button text.
     DialogModelChanged();
-    GetDialogClientView()->Layout();
   } else if (sender == learn_more_button_) {
     GetWidget()->Close();
     content_setting_bubble_model_->OnLearnMoreClicked();

chrome/browser/ui/views/sync/one_click_signin_dialog_view_unittest.cc

@@ -19,7 +19,6 @@
 #include "ui/views/controls/button/label_button.h"
 #include "ui/views/widget/widget.h"
 #include "ui/views/widget/widget_observer.h"
-#include "ui/views/window/dialog_client_view.h"
 
 class OneClickSigninDialogViewTest : public ChromeViewsTestBase,
                                      public views::WidgetObserver {
@@ -134,14 +133,14 @@ TEST_F(OneClickSigninDialogViewTest, HideDialog) {
 
 TEST_F(OneClickSigninDialogViewTest, OkButton) {
   OneClickSigninDialogView* view = ShowOneClickSigninDialog();
-  view->GetDialogClientView()->ResetViewShownTimeStampForTesting();
+  view->ResetViewShownTimeStampForTesting();
 
   gfx::Point center(10, 10);
   const ui::MouseEvent event(ui::ET_MOUSE_PRESSED, center, center,
                              ui::EventTimeForNow(), ui::EF_LEFT_MOUSE_BUTTON,
                              ui::EF_LEFT_MOUSE_BUTTON);
-  view->GetDialogClientView()->ok_button()->OnMousePressed(event);
-  view->GetDialogClientView()->ok_button()->OnMouseReleased(event);
+  view->GetOkButton()->OnMousePressed(event);
+  view->GetOkButton()->OnMouseReleased(event);
 
   WaitForClose();
   EXPECT_FALSE(OneClickSigninDialogView::IsShowing());
@@ -151,14 +150,14 @@ TEST_F(OneClickSigninDialogViewTest, OkButton) {
 
 TEST_F(OneClickSigninDialogViewTest, UndoButton) {
   OneClickSigninDialogView* view = ShowOneClickSigninDialog();
-  view->GetDialogClientView()->ResetViewShownTimeStampForTesting();
+  view->ResetViewShownTimeStampForTesting();
 
   gfx::Point center(10, 10);
   const ui::MouseEvent event(ui::ET_MOUSE_PRESSED, center, center,
                              ui::EventTimeForNow(), ui::EF_LEFT_MOUSE_BUTTON,
                              ui::EF_LEFT_MOUSE_BUTTON);
-  view->GetDialogClientView()->cancel_button()->OnMousePressed(event);
-  view->GetDialogClientView()->cancel_button()->OnMouseReleased(event);
+  view->GetCancelButton()->OnMousePressed(event);
+  view->GetCancelButton()->OnMouseReleased(event);
 
   WaitForClose();
   EXPECT_FALSE(OneClickSigninDialogView::IsShowing());

chrome/browser/ui/views/sync/profile_signin_confirmation_dialog_views.cc

@@ -42,7 +42,6 @@
 #include "ui/views/layout/layout_provider.h"
 #include "ui/views/views_delegate.h"
 #include "ui/views/widget/widget.h"
-#include "ui/views/window/dialog_client_view.h"
 
 ProfileSigninConfirmationDialogViews::ProfileSigninConfirmationDialogViews(
     Browser* browser,

chrome/browser/ui/views/task_manager_view.cc

@@ -32,7 +32,6 @@
 #include "ui/views/layout/fill_layout.h"
 #include "ui/views/view.h"
 #include "ui/views/widget/widget.h"
-#include "ui/views/window/dialog_client_view.h"
 
 #if defined(OS_CHROMEOS)
 #include "ash/public/cpp/shelf_item.h"

chrome/browser/ui/views/toolbar/toolbar_actions_bar_bubble_views_unittest.cc

@@ -25,7 +25,6 @@
 #include "ui/views/controls/link.h"
 #include "ui/views/test/test_widget_observer.h"
 #include "ui/views/widget/widget.h"
-#include "ui/views/window/dialog_client_view.h"
 
 namespace {
 const int kIconSize = 16;
@@ -77,7 +76,7 @@ class ToolbarActionsBarBubbleViewsTest : public ChromeViewsTestBase {
   }
 
   void ClickButton(views::Button* button) {
-    bubble()->GetDialogClientView()->ResetViewShownTimeStampForTesting();
+    bubble()->ResetViewShownTimeStampForTesting();
 
     ASSERT_TRUE(button);
     const gfx::Point point(10, 10);
@@ -113,10 +112,9 @@ TEST_F(ToolbarActionsBarBubbleViewsTest, TestBubbleLayoutActionButton) {
                                                ActionString());
   ShowBubble(&delegate);
 
-  EXPECT_TRUE(bubble()->GetDialogClientView()->ok_button());
-  EXPECT_EQ(ActionString(),
-            bubble()->GetDialogClientView()->ok_button()->GetText());
-  EXPECT_FALSE(bubble()->GetDialogClientView()->cancel_button());
+  EXPECT_TRUE(bubble()->GetOkButton());
+  EXPECT_EQ(ActionString(), bubble()->GetOkButton()->GetText());
+  EXPECT_FALSE(bubble()->GetCancelButton());
 
   CloseBubble();
 }
@@ -133,8 +131,8 @@ TEST_F(ToolbarActionsBarBubbleViewsTest, TestBubbleLayoutNoButtons) {
   ShowBubble(&delegate);
 
   EXPECT_EQ(nullptr, bubble()->GetExtraView());
-  EXPECT_FALSE(bubble()->GetDialogClientView()->ok_button());
-  EXPECT_FALSE(bubble()->GetDialogClientView()->cancel_button());
+  EXPECT_FALSE(bubble()->GetOkButton());
+  EXPECT_FALSE(bubble()->GetCancelButton());
   EXPECT_FALSE(bubble()->learn_more_button());
 
   CloseBubble();
@@ -147,12 +145,10 @@ TEST_F(ToolbarActionsBarBubbleViewsTest,
   delegate.set_dismiss_button_text(DismissString());
   ShowBubble(&delegate);
 
-  EXPECT_TRUE(bubble()->GetDialogClientView()->ok_button());
-  EXPECT_EQ(ActionString(),
-            bubble()->GetDialogClientView()->ok_button()->GetText());
-  EXPECT_TRUE(bubble()->GetDialogClientView()->cancel_button());
-  EXPECT_EQ(DismissString(),
-            bubble()->GetDialogClientView()->cancel_button()->GetText());
+  EXPECT_TRUE(bubble()->GetOkButton());
+  EXPECT_EQ(ActionString(), bubble()->GetOkButton()->GetText());
+  EXPECT_TRUE(bubble()->GetCancelButton());
+  EXPECT_EQ(DismissString(), bubble()->GetCancelButton()->GetText());
 
   EXPECT_FALSE(bubble()->learn_more_button());
   EXPECT_FALSE(bubble()->item_list());
@@ -174,12 +170,10 @@ TEST_F(ToolbarActionsBarBubbleViewsTest,
 
   ShowBubble(&delegate);
 
-  EXPECT_TRUE(bubble()->GetDialogClientView()->ok_button());
-  EXPECT_EQ(ActionString(),
-            bubble()->GetDialogClientView()->ok_button()->GetText());
-  EXPECT_TRUE(bubble()->GetDialogClientView()->cancel_button());
-  EXPECT_EQ(DismissString(),
-            bubble()->GetDialogClientView()->cancel_button()->GetText());
+  EXPECT_TRUE(bubble()->GetOkButton());
+  EXPECT_EQ(ActionString(), bubble()->GetOkButton()->GetText());
+  EXPECT_TRUE(bubble()->GetCancelButton());
+  EXPECT_EQ(DismissString(), bubble()->GetCancelButton()->GetText());
   EXPECT_TRUE(bubble()->learn_more_button());
   EXPECT_EQ(LearnMoreString(),
             bubble()->learn_more_button()->GetTooltipText(gfx::Point(0, 0)));
@@ -194,10 +188,9 @@ TEST_F(ToolbarActionsBarBubbleViewsTest, TestBubbleLayoutListView) {
   delegate.set_item_list_text(ItemListString());
   ShowBubble(&delegate);
 
-  EXPECT_TRUE(bubble()->GetDialogClientView()->ok_button());
-  EXPECT_EQ(ActionString(),
-            bubble()->GetDialogClientView()->ok_button()->GetText());
-  EXPECT_FALSE(bubble()->GetDialogClientView()->cancel_button());
+  EXPECT_TRUE(bubble()->GetOkButton());
+  EXPECT_EQ(ActionString(), bubble()->GetOkButton()->GetText());
+  EXPECT_FALSE(bubble()->GetCancelButton());
   EXPECT_FALSE(bubble()->learn_more_button());
   EXPECT_TRUE(bubble()->item_list());
   EXPECT_EQ(ItemListString(), bubble()->item_list()->GetText());
@@ -210,10 +203,9 @@ TEST_F(ToolbarActionsBarBubbleViewsTest, TestBubbleLayoutNoBodyText) {
       HeadingString(), base::string16(), ActionString());
   ShowBubble(&delegate);
 
-  EXPECT_TRUE(bubble()->GetDialogClientView()->ok_button());
-  EXPECT_EQ(ActionString(),
-            bubble()->GetDialogClientView()->ok_button()->GetText());
-  EXPECT_FALSE(bubble()->GetDialogClientView()->cancel_button());
+  EXPECT_TRUE(bubble()->GetOkButton());
+  EXPECT_EQ(ActionString(), bubble()->GetOkButton()->GetText());
+  EXPECT_FALSE(bubble()->GetCancelButton());
   EXPECT_FALSE(bubble()->learn_more_button());
   EXPECT_FALSE(bubble()->body_text());
   EXPECT_FALSE(bubble()->item_list());
@@ -228,12 +220,11 @@ TEST_F(ToolbarActionsBarBubbleViewsTest, TestBubbleDefaultDialogButtons) {
   delegate.set_default_dialog_button(ui::DIALOG_BUTTON_OK);
   ShowBubble(&delegate);
 
-  ASSERT_TRUE(bubble()->GetDialogClientView()->ok_button());
-  EXPECT_TRUE(bubble()->GetDialogClientView()->ok_button()->GetIsDefault());
+  ASSERT_TRUE(bubble()->GetOkButton());
+  EXPECT_TRUE(bubble()->GetOkButton()->GetIsDefault());
 
-  ASSERT_TRUE(bubble()->GetDialogClientView()->cancel_button());
-  EXPECT_FALSE(
-      bubble()->GetDialogClientView()->cancel_button()->GetIsDefault());
+  ASSERT_TRUE(bubble()->GetCancelButton());
+  EXPECT_FALSE(bubble()->GetCancelButton()->GetIsDefault());
 
   CloseBubble();
 }
@@ -255,7 +246,7 @@ TEST_F(ToolbarActionsBarBubbleViewsTest, TestShowAndCloseBubble) {
   EXPECT_TRUE(delegate.shown());
   EXPECT_FALSE(delegate.close_action());
 
-  bubble->GetDialogClientView()->CancelWindow();
+  bubble->CancelDialog();
   base::RunLoop().RunUntilIdle();
   ASSERT_TRUE(delegate.close_action());
   EXPECT_EQ(ToolbarActionsBarBubbleDelegate::CLOSE_DISMISS_USER_ACTION,
@@ -273,7 +264,7 @@ TEST_F(ToolbarActionsBarBubbleViewsTest, TestClickActionButton) {
 
   EXPECT_FALSE(delegate.close_action());
 
-  ClickButton(bubble()->GetDialogClientView()->ok_button());
+  ClickButton(bubble()->GetOkButton());
   ASSERT_TRUE(delegate.close_action());
   EXPECT_EQ(ToolbarActionsBarBubbleDelegate::CLOSE_EXECUTE,
             *delegate.close_action());
@@ -290,7 +281,7 @@ TEST_F(ToolbarActionsBarBubbleViewsTest, TestClickDismissButton) {
 
   EXPECT_FALSE(delegate.close_action());
 
-  ClickButton(bubble()->GetDialogClientView()->cancel_button());
+  ClickButton(bubble()->GetCancelButton());
   ASSERT_TRUE(delegate.close_action());
   EXPECT_EQ(ToolbarActionsBarBubbleDelegate::CLOSE_DISMISS_USER_ACTION,
             *delegate.close_action());

chrome/browser/ui/views/webauthn/authenticator_request_dialog_view.cc

@@ -24,7 +24,6 @@
 #include "ui/views/border.h"
 #include "ui/views/layout/fill_layout.h"
 #include "ui/views/vector_icons.h"
-#include "ui/views/window/dialog_client_view.h"
 
 namespace {
 
@@ -184,14 +183,14 @@ views::View* AuthenticatorRequestDialogView::GetInitiallyFocusedView() {
 
   if (sheet()->model()->IsAcceptButtonVisible() &&
       sheet()->model()->IsAcceptButtonEnabled()) {
-    return GetDialogClientView()->ok_button();
+    return GetOkButton();
   }
 
   if (ShouldOtherTransportsButtonBeVisible())
     return other_transports_button_;
 
   if (sheet()->model()->IsCancelButtonVisible())
-    return GetDialogClientView()->cancel_button();
+    return GetCancelButton();
 
   return nullptr;
 }
@@ -336,8 +335,8 @@ void AuthenticatorRequestDialogView::UpdateUIForCurrentSheet() {
 
   // Force re-layout of the entire dialog client view, which includes the sheet
   // content as well as the button row on the bottom.
-  DCHECK(GetDialogClientView());
-  GetDialogClientView()->Layout();
+  // TODO(ellyjones): Why is this necessary?
+  GetWidget()->GetRootView()->Layout();
 
   // The accessibility title is also sourced from the |sheet_|'s step title.
   GetWidget()->UpdateWindowTitle();

ui/views/window/dialog_client_view.h

@@ -30,6 +30,10 @@ class Widget;
 //   +------------------------------+
 //   | [Extra View]   [OK] [Cancel] |
 //   +------------------------------+
+//
+// You should not need to directly depend on or use DialogClientView; it is
+// being made internal to //ui/views. Access it through the public interfaces on
+// DialogDelegate.
 class VIEWS_EXPORT DialogClientView : public ClientView,
                                       public ButtonListener,
                                       public DialogObserver {

ui/views/window/dialog_delegate.cc

@@ -294,6 +294,18 @@ std::unique_ptr<View> DialogDelegate::DisownExtraView() {
   return std::move(extra_view_);
 }
 
+void DialogDelegate::CancelDialog() {
+  GetDialogClientView()->CancelWindow();
+}
+
+void DialogDelegate::AcceptDialog() {
+  GetDialogClientView()->AcceptWindow();
+}
+
+void DialogDelegate::ResetViewShownTimeStampForTesting() {
+  GetDialogClientView()->ResetViewShownTimeStampForTesting();
+}
+
 DialogDelegate::~DialogDelegate() {
   UMA_HISTOGRAM_LONG_TIMES("Dialog.DialogDelegate.Duration",
                            base::TimeTicks::Now() - creation_time_);

ui/views/window/dialog_delegate.h

@@ -231,6 +231,18 @@ class VIEWS_EXPORT DialogDelegate : public WidgetDelegate {
   //    lazy layout system in View::InvalidateLayout
   std::unique_ptr<View> DisownExtraView();
 
+  // Externally or accept the dialog. These methods:
+  // 1) Invoke the DialogDelegate's Cancel or Accept methods
+  // 2) Depending on their return value, close the dialog's widget.
+  // Neither of these methods can be called before the dialog has been
+  // initialized.
+  void CancelDialog();
+  void AcceptDialog();
+
+  // Reset the dialog's shown timestamp, for tests that are subject to the
+  // "unintended interaction" detection mechanism.
+  void ResetViewShownTimeStampForTesting();
+
  protected:
   ~DialogDelegate() override;