Commit af0f1fcd authored by Hiroki Nakagawa's avatar Hiroki Nakagawa Committed by Chromium LUCI CQ

Prerender: Protect PrerenderBrowserTest::request_count_by_path_ with base::Lock

PrerenderBrowserTest::request_count_by_path_ is accessed from
MonitorResourceRequest() and GetRequestCount(). These functions are
called from different threads: the UI thread and
EmbeddedTestServer::io_thread_. Therefore, the field should have been
protected by a lock.

Bug: 1158659
Change-Id: I491f7a60b24c4652c4278f46129976eccb81fecd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2592289Reviewed-by: default avatarMatt Falkenhagen <falken@chromium.org>
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#836981}
parent 175cc670
...@@ -2,10 +2,13 @@ ...@@ -2,10 +2,13 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
#include "base/synchronization/lock.h"
#include "base/test/scoped_feature_list.h" #include "base/test/scoped_feature_list.h"
#include "base/thread_annotations.h"
#include "content/browser/prerender/prerender_host_registry.h" #include "content/browser/prerender/prerender_host_registry.h"
#include "content/browser/storage_partition_impl.h" #include "content/browser/storage_partition_impl.h"
#include "content/public/browser/browser_context.h" #include "content/public/browser/browser_context.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/test/browser_test.h" #include "content/public/test/browser_test.h"
#include "content/public/test/browser_test_utils.h" #include "content/public/test/browser_test_utils.h"
#include "content/public/test/content_browser_test.h" #include "content/public/test/content_browser_test.h"
...@@ -33,6 +36,8 @@ class PrerenderBrowserTest : public ContentBrowserTest, ...@@ -33,6 +36,8 @@ class PrerenderBrowserTest : public ContentBrowserTest,
~PrerenderBrowserTest() override = default; ~PrerenderBrowserTest() override = default;
void SetUpOnMainThread() override { void SetUpOnMainThread() override {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
// Make sure the feature param is correctly set before testing. // Make sure the feature param is correctly set before testing.
if (IsActivationDisabled()) { if (IsActivationDisabled()) {
ASSERT_EQ(blink::features::kPrerender2Param.Get(), ASSERT_EQ(blink::features::kPrerender2Param.Get(),
...@@ -52,14 +57,19 @@ class PrerenderBrowserTest : public ContentBrowserTest, ...@@ -52,14 +57,19 @@ class PrerenderBrowserTest : public ContentBrowserTest,
} }
void TearDownOnMainThread() override { void TearDownOnMainThread() override {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
EXPECT_TRUE(ssl_server_.ShutdownAndWaitUntilComplete()); EXPECT_TRUE(ssl_server_.ShutdownAndWaitUntilComplete());
} }
void MonitorResourceRequest(const net::test_server::HttpRequest& request) { void MonitorResourceRequest(const net::test_server::HttpRequest& request) {
// This should be called on `EmbeddedTestServer::io_thread_`.
DCHECK(!BrowserThread::CurrentlyOn(BrowserThread::UI));
base::AutoLock auto_lock(lock_);
request_count_by_path_[request.GetURL().PathForRequest()]++; request_count_by_path_[request.GetURL().PathForRequest()]++;
} }
PrerenderHostRegistry& GetPrerenderHostRegistry() { PrerenderHostRegistry& GetPrerenderHostRegistry() {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
auto* storage_partition = static_cast<StoragePartitionImpl*>( auto* storage_partition = static_cast<StoragePartitionImpl*>(
BrowserContext::GetDefaultStoragePartition( BrowserContext::GetDefaultStoragePartition(
shell()->web_contents()->GetBrowserContext())); shell()->web_contents()->GetBrowserContext()));
...@@ -69,6 +79,7 @@ class PrerenderBrowserTest : public ContentBrowserTest, ...@@ -69,6 +79,7 @@ class PrerenderBrowserTest : public ContentBrowserTest,
// Adds <link rel=prerender> in the current main frame and waits until the // Adds <link rel=prerender> in the current main frame and waits until the
// completion of prerendering. // completion of prerendering.
void AddPrerender(const GURL& prerendering_url) { void AddPrerender(const GURL& prerendering_url) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
// Start watching new web contents to be created for prerendering. // Start watching new web contents to be created for prerendering.
content::TestNavigationObserver observer(prerendering_url); content::TestNavigationObserver observer(prerendering_url);
observer.StartWatchingNewWebContents(); observer.StartWatchingNewWebContents();
...@@ -87,6 +98,7 @@ class PrerenderBrowserTest : public ContentBrowserTest, ...@@ -87,6 +98,7 @@ class PrerenderBrowserTest : public ContentBrowserTest,
// destroyed during activation and results in crashes. // destroyed during activation and results in crashes.
// See https://crbug.com/1154501 for the MPArch migration. // See https://crbug.com/1154501 for the MPArch migration.
void NavigateWithLocation(const GURL& url) { void NavigateWithLocation(const GURL& url) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
content::TestNavigationObserver observer(shell()->web_contents()); content::TestNavigationObserver observer(shell()->web_contents());
EXPECT_TRUE( EXPECT_TRUE(
ExecJs(shell()->web_contents(), JsReplace("location = $1", url))); ExecJs(shell()->web_contents(), JsReplace("location = $1", url)));
...@@ -95,10 +107,13 @@ class PrerenderBrowserTest : public ContentBrowserTest, ...@@ -95,10 +107,13 @@ class PrerenderBrowserTest : public ContentBrowserTest,
} }
GURL GetUrl(const std::string& path) { GURL GetUrl(const std::string& path) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
return ssl_server_.GetURL("a.test", path); return ssl_server_.GetURL("a.test", path);
} }
int GetRequestCount(const GURL& url) { int GetRequestCount(const GURL& url) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
base::AutoLock auto_lock(lock_);
return request_count_by_path_[url.PathForRequest()]; return request_count_by_path_[url.PathForRequest()];
} }
...@@ -111,9 +126,12 @@ class PrerenderBrowserTest : public ContentBrowserTest, ...@@ -111,9 +126,12 @@ class PrerenderBrowserTest : public ContentBrowserTest,
// Counts of requests sent to the server. Keyed by path (not by full URL) // Counts of requests sent to the server. Keyed by path (not by full URL)
// because the host part of the requests is translated ("a.test" to // because the host part of the requests is translated ("a.test" to
// "127.0.0.1") before the server handles them. // "127.0.0.1") before the server handles them.
std::map<std::string, int> request_count_by_path_; // This is accessed from the UI thread and `EmbeddedTestServer::io_thread_`.
std::map<std::string, int> request_count_by_path_ GUARDED_BY(lock_);
base::test::ScopedFeatureList feature_list_; base::test::ScopedFeatureList feature_list_;
base::Lock lock_;
}; };
INSTANTIATE_TEST_SUITE_P(All, INSTANTIATE_TEST_SUITE_P(All,
......
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