Commit 0a074c79 authored by Christopher Thompson's avatar Christopher Thompson Committed by Commit Bot

Fix DCHECK crash in PageInfoBubbleViewTest

This fixes a flaky test crash in PageInfoBubbleViewTest where
content::BrowserSideNavigationSetUp() was called but we weren't calling
the corresponding content::BrowserSideNavigationTearDown(). This can
cause a DCHECK to trigger [1] under certain orderings of test
invocations. The flakiness isn't bad enough to be problematic yet [2]
(especially since it only triggers on Debug bots), but it's better to
preemptively fix it.

[1] https://cs.chromium.org/chromium/src/content/browser/loader/navigation_url_loader.cc?l=55&rcl=42b97a68450fe6fb772120b6767d5687af3a52df
[2] https://test-results.appspot.com/dashboards/flakiness_dashboard.html#testType=unit_tests&tests=PageInfoBubbleViewTest

All other callers of content::BrowserSideNavigationSetUp() correctly
call the TearDown() function during test fixture tear down.

To reproduce the flaky crash, you can run:

unit_tests --gtest_filter="PageInfoBubbleViewTest.*" --gtest_shuffle \
    --gtest_repeat=-1

This can also be used to verify that the crash no longer triggers with
this CL.

Change-Id: I031e3de8a4b46bb58fcb20d752a83289f1372bc5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1885077Reviewed-by: default avatarMustafa Emre Acer <meacer@chromium.org>
Commit-Queue: Christopher Thompson <cthomp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#710048}
parent 64596be3
...@@ -196,6 +196,7 @@ class PageInfoBubbleViewTest : public testing::Test { ...@@ -196,6 +196,7 @@ class PageInfoBubbleViewTest : public testing::Test {
// testing::Test: // testing::Test:
void SetUp() override { void SetUp() override {
content::BrowserSideNavigationSetUp();
views_helper_.test_views_delegate()->set_layout_provider( views_helper_.test_views_delegate()->set_layout_provider(
ChromeLayoutProvider::CreateLayoutProvider()); ChromeLayoutProvider::CreateLayoutProvider());
views::Widget::InitParams parent_params; views::Widget::InitParams parent_params;
...@@ -210,7 +211,10 @@ class PageInfoBubbleViewTest : public testing::Test { ...@@ -210,7 +211,10 @@ class PageInfoBubbleViewTest : public testing::Test {
web_contents); web_contents);
} }
void TearDown() override { parent_window_->CloseNow(); } void TearDown() override {
parent_window_->CloseNow();
content::BrowserSideNavigationTearDown();
}
protected: protected:
ScopedWebContentsTestHelper web_contents_helper_; ScopedWebContentsTestHelper web_contents_helper_;
...@@ -683,7 +687,6 @@ TEST_F(PageInfoBubbleViewTest, ChangingFlashSettingForSiteIsRemembered) { ...@@ -683,7 +687,6 @@ TEST_F(PageInfoBubbleViewTest, ChangingFlashSettingForSiteIsRemembered) {
// Tests opening the bubble between navigation start and finish. The bubble // Tests opening the bubble between navigation start and finish. The bubble
// should be updated to reflect the secure state after the navigation commits. // should be updated to reflect the secure state after the navigation commits.
TEST_F(PageInfoBubbleViewTest, OpenPageInfoBubbleAfterNavigationStart) { TEST_F(PageInfoBubbleViewTest, OpenPageInfoBubbleAfterNavigationStart) {
content::BrowserSideNavigationSetUp();
SecurityStateTabHelper::CreateForWebContents( SecurityStateTabHelper::CreateForWebContents(
web_contents_helper_.web_contents()); web_contents_helper_.web_contents());
std::unique_ptr<content::NavigationSimulator> navigation = std::unique_ptr<content::NavigationSimulator> navigation =
...@@ -728,7 +731,6 @@ TEST_F(PageInfoBubbleViewTest, EnsureCloseCallback) { ...@@ -728,7 +731,6 @@ TEST_F(PageInfoBubbleViewTest, EnsureCloseCallback) {
} }
TEST_F(PageInfoBubbleViewTest, CertificateButtonShowsEvCertDetails) { TEST_F(PageInfoBubbleViewTest, CertificateButtonShowsEvCertDetails) {
content::BrowserSideNavigationSetUp();
SecurityStateTabHelper::CreateForWebContents( SecurityStateTabHelper::CreateForWebContents(
web_contents_helper_.web_contents()); web_contents_helper_.web_contents());
std::unique_ptr<content::NavigationSimulator> navigation = std::unique_ptr<content::NavigationSimulator> navigation =
......
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