Commit 4e2773bd authored by Lukasz Anforowicz's avatar Lukasz Anforowicz Committed by Commit Bot

Add NOTREACHED for InitiatorLockCompatibility::kNoLock + fix unit tests.

This CL adds a NOTREACHED assertion to the
InitiatorLockCompatibility::kNoLock case in
CorsURLLoaderFactory::IsValidRequest (in preparation for calling
mojo::ReportBadMessage and returning false in a follow-up CL).

Presence of the new NOTREACHED assertion requires fixing unit tests, so
that they more accurately simulate the behavior of the product code.  In
particular:

- A DCHECK has been added to CorsURLLoaderTest::ResetFactory to help
  ensure that all tests use a non-nullopt |request_initiator_site_lock|
  when simulating a renderer-side factory.

- CorsURLLoaderTest::SetUp started passing non-null default initiator to
  CorsURLLoaderTest::ResetFactory.  https://example.com was chosen as
  the default, because it is the most commonly used URL in other tests.
  This necessitated changing some tests to use the matching scheme
  (https rather than http).

- Some tests were forced to specify a non-nullopt
  network::ResourceRequest::request_initiator (this is no longer okay,
  because if kNoLock case cannot be reached after this CL, then
  kNoInitiator case would crash the test).
- Tests that really had to use a nullopt |request_initiator| were
  switched to simulating a browser-side factory (one with a nullopt
  |request_initiator_site_lock| and with kBrowserProcessId).

Fixed: 1115222
Change-Id: I8c6e6603ba4626cf5c306ae5ca286fe1f1fb48e5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2350232
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: default avatarTakashi Toyoshima <toyoshim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#798890}
parent 03476cf7
......@@ -387,11 +387,7 @@ bool CorsURLLoaderFactory::IsValidRequest(const ResourceRequest& request,
// a renderer process. Once issues blocking https://crbug.com/1114906 are
// fixed, the case below should return |false| and call
// mojo::ReportBadMessage.
//
// TODO(lukasza): https://crbug.com/1114906: Add NOTREACHED below, after
// tweaking unit tests to correctly simulate product behavior. Broken
// tests: services_unittests: CorsURLLoader*Test and
// *RawRequestAccessControl*.
NOTREACHED();
break;
case InitiatorLockCompatibility::kNoInitiator:
......
......@@ -70,6 +70,8 @@ class CorsURLLoaderFactoryTest : public testing::Test {
auto factory_params = network::mojom::URLLoaderFactoryParams::New();
factory_params->process_id = kProcessId;
factory_params->request_initiator_site_lock =
url::Origin::Create(GURL("http://localhost"));
auto resource_scheduler_client =
base::MakeRefCounted<ResourceSchedulerClient>(
kProcessId, kRouteId, &resource_scheduler_,
......
......@@ -935,6 +935,8 @@ class NetworkServiceTestWithService : public testing::Test {
mojom::URLLoaderFactoryParamsPtr params =
mojom::URLLoaderFactoryParams::New();
params->process_id = process_id;
params->request_initiator_site_lock =
url::Origin::Create(GURL("https://initiator.example.com"));
params->is_corb_enabled = false;
network_context_->CreateURLLoaderFactory(
loader_factory.BindNewPipeAndPassReceiver(), std::move(params));
......
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