Commit a5d3b30a authored by Alexander Semashko's avatar Alexander Semashko Committed by Commit Bot

Prevent use-after-free of ContentBrowserClient.

I observed the use after free in
WebContentsImplTest.LoadResourceWithEmptySecurityInfo. It was triggered
by some downstream code that is not present in chromium, but the root
cause is that the test sets ContentBrowserClient that is destroyed at
the end of the test body. This CL extends the lifetime of this object in
WebContentsImplTest.

Bug: 
Change-Id: I8d6b553a20b497c346b47aeb043dedcd84e94abb
Reviewed-on: https://chromium-review.googlesource.com/563380Reviewed-by: default avatarJohn Abd-El-Malek <jam@chromium.org>
Commit-Queue: Alexander Semashko <ahest@yandex-team.ru>
Cr-Commit-Position: refs/heads/master@{#488680}
parent ea7eb66c
...@@ -229,9 +229,12 @@ class TestInterstitialPageStateGuard : public TestInterstitialPage::Delegate { ...@@ -229,9 +229,12 @@ class TestInterstitialPageStateGuard : public TestInterstitialPage::Delegate {
class WebContentsImplTestBrowserClient : public TestContentBrowserClient { class WebContentsImplTestBrowserClient : public TestContentBrowserClient {
public: public:
WebContentsImplTestBrowserClient() WebContentsImplTestBrowserClient()
: assign_site_for_url_(false) {} : assign_site_for_url_(false),
original_browser_client_(SetBrowserClientForTesting(this)) {}
~WebContentsImplTestBrowserClient() override {} ~WebContentsImplTestBrowserClient() override {
SetBrowserClientForTesting(original_browser_client_);
}
bool ShouldAssignSiteForURL(const GURL& url) override { bool ShouldAssignSiteForURL(const GURL& url) override {
return assign_site_for_url_; return assign_site_for_url_;
...@@ -243,6 +246,7 @@ class WebContentsImplTestBrowserClient : public TestContentBrowserClient { ...@@ -243,6 +246,7 @@ class WebContentsImplTestBrowserClient : public TestContentBrowserClient {
private: private:
bool assign_site_for_url_; bool assign_site_for_url_;
ContentBrowserClient* original_browser_client_;
}; };
class WebContentsImplTest : public RenderViewHostImplTestHarness { class WebContentsImplTest : public RenderViewHostImplTestHarness {
...@@ -833,9 +837,6 @@ TEST_F(WebContentsImplTest, NavigateFromSitelessUrl) { ...@@ -833,9 +837,6 @@ TEST_F(WebContentsImplTest, NavigateFromSitelessUrl) {
DeleteContents(); DeleteContents();
EXPECT_EQ(orig_rvh_delete_count, 1); EXPECT_EQ(orig_rvh_delete_count, 1);
EXPECT_EQ(pending_rvh_delete_count, 1); EXPECT_EQ(pending_rvh_delete_count, 1);
// Since the ChromeBlobStorageContext posts a task to the BrowserThread, we
// must run out the loop so the thread bundle is destroyed after this happens.
base::RunLoop().RunUntilIdle();
} }
// Regression test for http://crbug.com/386542 - variation of // Regression test for http://crbug.com/386542 - variation of
...@@ -884,9 +885,6 @@ TEST_F(WebContentsImplTest, NavigateFromRestoredSitelessUrl) { ...@@ -884,9 +885,6 @@ TEST_F(WebContentsImplTest, NavigateFromRestoredSitelessUrl) {
// Cleanup. // Cleanup.
DeleteContents(); DeleteContents();
// Since the ChromeBlobStorageContext posts a task to the BrowserThread, we
// must run out the loop so the thread bundle is destroyed after this happens.
base::RunLoop().RunUntilIdle();
} }
// Complement for NavigateFromRestoredSitelessUrl, verifying that when a regular // Complement for NavigateFromRestoredSitelessUrl, verifying that when a regular
...@@ -933,9 +931,6 @@ TEST_F(WebContentsImplTest, NavigateFromRestoredRegularUrl) { ...@@ -933,9 +931,6 @@ TEST_F(WebContentsImplTest, NavigateFromRestoredRegularUrl) {
// Cleanup. // Cleanup.
DeleteContents(); DeleteContents();
// Since the ChromeBlobStorageContext posts a task to the BrowserThread, we
// must run out the loop so the thread bundle is destroyed after this happens.
base::RunLoop().RunUntilIdle();
} }
// Test that we can find an opener RVH even if it's pending. // Test that we can find an opener RVH even if it's pending.
......
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