Commit 208b89b3 authored by Elly Fong-Jones's avatar Elly Fong-Jones Committed by Commit Bot

extensions: robustify some ExtensionInstall tests

These tests end with an asynchronous close of the dialog's widget.
This means that, depending on the current weather, either the view
under test (owned by the widget) will be destroyed before or after
the test ends. This *currently* doesn't usually cause problems,
because Widget::Close calls DialogDelegate::Cancel, which sets the
object's state so that ~ExtensionInstallDialogView doesn't do
anything. However, if ::Cancel is ever not called, these tests will
start to flakily pass or fail depending on whether the async close
finishes before or after the test ends.

This change causes the tests to wait for the close to complete, to
make their execution deterministic.

It also removes some dead code. As part of "fixing" issue 851167,
r566186 added a MAYBE stanza for one of these tests, but never
actually used it.

Bug: 851167
Change-Id: Ifeac4c7791a79ea8a7c9fab7c398ea87bd62646a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2136866Reviewed-by: default avatarDevlin <rdevlin.cronin@chromium.org>
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#756820}
parent 50c6e900
......@@ -39,6 +39,7 @@
#include "ui/base/l10n/l10n_util.h"
#include "ui/views/bubble/bubble_frame_view.h"
#include "ui/views/controls/scroll_view.h"
#include "ui/views/test/widget_test.h"
#include "ui/views/view.h"
#include "ui/views/widget/widget.h"
......@@ -47,6 +48,16 @@ using extensions::PermissionMessage;
using extensions::PermissionMessages;
using extensions::PermissionSet;
namespace {
void CloseAndWait(views::Widget* widget) {
views::test::WidgetDestroyedWaiter waiter(widget);
widget->Close();
waiter.Wait();
}
} // namespace
class ExtensionInstallDialogViewTestBase
: public extensions::ExtensionBrowserTest {
protected:
......@@ -176,15 +187,6 @@ class ExtensionInstallDialogViewTest
DISALLOW_COPY_AND_ASSIGN(ExtensionInstallDialogViewTest);
};
// Verifies that the delegate is notified when the user selects to accept or
// cancel the install.
//
// Crashes flakily on Mac. See http://crbug.com/851167
#if defined(OS_MACOSX)
#define MAYBE_NotifyDelegate DISABLED_NotifyDelegate
#else
#define MAYBE_NotifyDelegate NotifyDelegate
#endif
IN_PROC_BROWSER_TEST_F(ExtensionInstallDialogViewTest, NotifyDelegate) {
{
// User presses install.
......@@ -205,7 +207,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionInstallDialogViewTest, NotifyDelegate) {
// cancel.
ExtensionInstallPromptTestHelper helper;
views::DialogDelegateView* delegate_view = CreateAndShowPrompt(&helper);
delegate_view->GetWidget()->Close();
CloseAndWait(delegate_view->GetWidget());
// TODO(devlin): Should this be ABORTED?
EXPECT_EQ(ExtensionInstallPrompt::Result::USER_CANCELED, helper.result());
}
......@@ -232,7 +234,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionInstallDialogViewTest, InstallButtonDelay) {
// Ensure default button (cancel) has focus.
EXPECT_TRUE(delegate_view->GetInitiallyFocusedView()->HasFocus());
delegate_view->Close();
CloseAndWait(delegate_view->GetWidget());
}
class ExtensionInstallDialogViewInteractiveBrowserTest
......@@ -483,8 +485,7 @@ void ExtensionInstallDialogRatingsSectionTest::TestRatingsSectionA11y(
EXPECT_EQ(ax::mojom::Role::kIgnored, node_data.role);
}
modal_dialog->Close();
base::RunLoop().RunUntilIdle();
CloseAndWait(modal_dialog);
}
IN_PROC_BROWSER_TEST_F(ExtensionInstallDialogRatingsSectionTest,
......@@ -535,5 +536,5 @@ IN_PROC_BROWSER_TEST_F(ExtensionInstallDialogWithWithholdPermissionsUI,
EXPECT_TRUE(extra_view);
EXPECT_EQ("Checkbox", std::string(extra_view->GetClassName()));
delegate_view->Close();
CloseAndWait(delegate_view->GetWidget());
}
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment