Remove DialogDelegate::OnClosed() which is redundant with

chrome/browser/ui/views/extensions/extension_install_dialog_view_browsertest.cc

 // Copyright 2014 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 "chrome/browser/ui/views/extensions/extension_install_dialog_view.h"
 
 #include <utility>
 
 #include "base/macros.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/extensions/extension_browsertest.h"
 #include "chrome/browser/extensions/extension_icon_manager.h"
 #include "chrome/browser/extensions/extension_install_prompt.h"
 #include "chrome/browser/extensions/extension_install_prompt_test_helper.h"
 #include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/webui/extensions/extension_settings_handler.h"
 #include "chrome/common/extensions/extension_test_util.h"
 #include "components/constrained_window/constrained_window_views.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/test/test_utils.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/permissions/permission_message_provider.h"
 #include "extensions/common/permissions/permissions_data.h"
 #include "extensions/common/test_util.h"
 #include "ui/views/controls/scroll_view.h"
 #include "ui/views/view.h"
 #include "ui/views/widget/widget.h"
+#include "ui/views/window/dialog_client_view.h"
 
 using extensions::PermissionIDSet;
 using extensions::PermissionMessage;
 using extensions::PermissionMessages;
 
 class ExtensionInstallDialogViewTestBase : public ExtensionBrowserTest {
  protected:
   explicit ExtensionInstallDialogViewTestBase(
       ExtensionInstallPrompt::PromptType prompt_type);
   ~ExtensionInstallDialogViewTestBase() override {}
@@ skipping 106 matching lines @@
 }
 
 class ExtensionInstallDialogViewTest
     : public ExtensionInstallDialogViewTestBase {
  protected:
   ExtensionInstallDialogViewTest()
       : ExtensionInstallDialogViewTestBase(
             ExtensionInstallPrompt::INSTALL_PROMPT) {}
   ~ExtensionInstallDialogViewTest() override {}
 
+  views::DialogDelegateView* CreateAndShowPrompt(
+      ExtensionInstallPromptTestHelper* helper) {
+    scoped_ptr<ExtensionInstallDialogView> dialog(
+        new ExtensionInstallDialogView(profile(), web_contents(),
+                                       helper->GetCallback(), CreatePrompt()));
+    views::DialogDelegateView* delegate_view = dialog.get();
+
+    views::Widget* modal_dialog = views::DialogDelegate::CreateDialogWidget(
+        dialog.release(), nullptr, browser()->window()->GetNativeWindow());
+    modal_dialog->Show();

[msw, 2016/02/11 18:08:58]

I suppose it's potentially necessary to create the Widget, in order to keep the
changed below working, but is it necessary to actually show the dialog?
Otherwise, I wonder if showing a dialog here might actually interfere with other
tests running simultaneously (but any tests that depend on focus/activation
should be interative_ui_tests).

[Evan Stade, 2016/02/11 20:02:11]

i don't think it would interfere with other tests, it's only window-modal

+
+    return delegate_view;
+  }
+
  private:
   DISALLOW_COPY_AND_ASSIGN(ExtensionInstallDialogViewTest);
 };
 
 // Verifies that the delegate is notified when the user selects to accept or
 // cancel the install.
 IN_PROC_BROWSER_TEST_F(ExtensionInstallDialogViewTest, NotifyDelegate) {
   {
+    // User presses install.
     ExtensionInstallPromptTestHelper helper;
-    scoped_ptr<ExtensionInstallDialogView> dialog(
-        new ExtensionInstallDialogView(profile(), web_contents(),
-                                       helper.GetCallback(),
-                                       CreatePrompt()));
-    views::DialogDelegateView* delegate_view = dialog.get();
-
-    delegate_view->Accept();
-    delegate_view->OnClosed();
-    dialog.reset();
-
+    views::DialogDelegateView* delegate_view = CreateAndShowPrompt(&helper);
+    delegate_view->GetDialogClientView()->AcceptWindow();

[msw, 2016/02/11 18:08:58]

Can you stick with DialogDelegateView::Accept/Cancel + DeleteDelegate in these
three cases, and avoid creating the Widget?

[Evan Stade, 2016/02/11 20:02:11]

that seems like worse coverage. The test seems like it's trying to simulate user
actions, and this code is closer to doing that, instead of re-implementing what
happens after the user takes an action.

     EXPECT_EQ(ExtensionInstallPrompt::Result::ACCEPTED, helper.result());
   }
-
   {
-    // The user cancels the install.
+    // User presses cancel.
     ExtensionInstallPromptTestHelper helper;
-    scoped_ptr<ExtensionInstallDialogView> dialog(
-        new ExtensionInstallDialogView(profile(), web_contents(),
-                                       helper.GetCallback(),
-                                       CreatePrompt()));
-    views::DialogDelegateView* delegate_view = dialog.get();
-
-    delegate_view->Cancel();
-    delegate_view->OnClosed();
-    dialog.reset();
-
+    views::DialogDelegateView* delegate_view = CreateAndShowPrompt(&helper);
+    delegate_view->GetDialogClientView()->CancelWindow();
     EXPECT_EQ(ExtensionInstallPrompt::Result::USER_CANCELED, helper.result());
   }
-
   {
-    // Corner case: Dialog is closed without the user explicitly choosing to
-    // proceed or cancel.
+    // Dialog is closed without the user explicitly choosing to proceed or
+    // cancel.
     ExtensionInstallPromptTestHelper helper;
-    scoped_ptr<ExtensionInstallDialogView> dialog(
-        new ExtensionInstallDialogView(profile(), web_contents(),
-                                       helper.GetCallback(),
-                                       CreatePrompt()));
-    dialog.reset();
-
+    views::DialogDelegateView* delegate_view = CreateAndShowPrompt(&helper);
+    delegate_view->GetWidget()->Close();
     // TODO(devlin): Should this be ABORTED?
     EXPECT_EQ(ExtensionInstallPrompt::Result::USER_CANCELED, helper.result());
   }
 }