Commit 5b7eb518 authored by Miriam Gershenson's avatar Miriam Gershenson Committed by Commit Bot

Fix Cronet load flag initialization

Load flags were being read from the CronetURLRequestContextAdapter
on the main thread, which could potentially happen before they were
initialized on the network thread if a UrlRequest is created immediately
after the CronetEngine. Now they are read at request start time on the
network thread, which must be after context initialization.

BUG=743232

Change-Id: I3f9c5cd1cde906555c5ba68b4a45a9a3ee71b014
Reviewed-on: https://chromium-review.googlesource.com/574827Reviewed-by: default avatarPaul Jensen <pauljensen@chromium.org>
Commit-Queue: Miriam Gershenson <mgersh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487227}
parent 5c498c5a
...@@ -91,7 +91,7 @@ CronetURLRequestAdapter::CronetURLRequestAdapter( ...@@ -91,7 +91,7 @@ CronetURLRequestAdapter::CronetURLRequestAdapter(
initial_url_(url), initial_url_(url),
initial_priority_(priority), initial_priority_(priority),
initial_method_("GET"), initial_method_("GET"),
load_flags_(context->default_load_flags()), load_flags_(net::LOAD_NORMAL),
enable_metrics_(jenable_metrics == JNI_TRUE), enable_metrics_(jenable_metrics == JNI_TRUE),
metrics_reported_(false) { metrics_reported_(false) {
DCHECK(!context_->IsOnNetworkThread()); DCHECK(!context_->IsOnNetworkThread());
...@@ -304,7 +304,7 @@ void CronetURLRequestAdapter::StartOnNetworkThread() { ...@@ -304,7 +304,7 @@ void CronetURLRequestAdapter::StartOnNetworkThread() {
<< " priority: " << RequestPriorityToString(initial_priority_); << " priority: " << RequestPriorityToString(initial_priority_);
url_request_ = context_->GetURLRequestContext()->CreateRequest( url_request_ = context_->GetURLRequestContext()->CreateRequest(
initial_url_, net::DEFAULT_PRIORITY, this); initial_url_, net::DEFAULT_PRIORITY, this);
url_request_->SetLoadFlags(load_flags_); url_request_->SetLoadFlags(load_flags_ | context_->default_load_flags());
url_request_->set_method(initial_method_); url_request_->set_method(initial_method_);
url_request_->SetExtraRequestHeaders(initial_request_headers_); url_request_->SetExtraRequestHeaders(initial_request_headers_);
url_request_->SetPriority(initial_priority_); url_request_->SetPriority(initial_priority_);
......
...@@ -962,6 +962,11 @@ void CronetURLRequestContextAdapter::GetCertVerifierDataOnNetworkThread() { ...@@ -962,6 +962,11 @@ void CronetURLRequestContextAdapter::GetCertVerifierDataOnNetworkThread() {
base::android::ConvertUTF8ToJavaString(env, encoded_data).obj()); base::android::ConvertUTF8ToJavaString(env, encoded_data).obj());
} }
int CronetURLRequestContextAdapter::default_load_flags() const {
DCHECK(is_context_initialized_);
return default_load_flags_;
}
base::Thread* CronetURLRequestContextAdapter::GetFileThread() { base::Thread* CronetURLRequestContextAdapter::GetFileThread() {
DCHECK(GetNetworkTaskRunner()->BelongsToCurrentThread()); DCHECK(GetNetworkTaskRunner()->BelongsToCurrentThread());
if (!file_thread_) { if (!file_thread_) {
......
...@@ -108,7 +108,7 @@ class CronetURLRequestContextAdapter ...@@ -108,7 +108,7 @@ class CronetURLRequestContextAdapter
const base::android::JavaParamRef<jobject>& jcaller); const base::android::JavaParamRef<jobject>& jcaller);
// Default net::LOAD flags used to create requests. // Default net::LOAD flags used to create requests.
int default_load_flags() const { return default_load_flags_; } int default_load_flags() const;
// Called on init Java thread to initialize URLRequestContext. // Called on init Java thread to initialize URLRequestContext.
void InitRequestContextOnInitThread(); void InitRequestContextOnInitThread();
......
...@@ -544,11 +544,15 @@ public class CronetUrlRequestContextTest extends CronetTestBase { ...@@ -544,11 +544,15 @@ public class CronetUrlRequestContextTest extends CronetTestBase {
private CronetEngine createCronetEngineWithCache(int cacheType) { private CronetEngine createCronetEngineWithCache(int cacheType) {
CronetEngine.Builder builder = new CronetEngine.Builder(getContext()); CronetEngine.Builder builder = new CronetEngine.Builder(getContext());
if (cacheType == CronetEngine.Builder.HTTP_CACHE_DISK) { if (cacheType == CronetEngine.Builder.HTTP_CACHE_DISK
|| cacheType == CronetEngine.Builder.HTTP_CACHE_DISK_NO_HTTP) {
builder.setStoragePath(getTestStorage(getContext())); builder.setStoragePath(getTestStorage(getContext()));
} }
builder.enableHttpCache(cacheType, 100 * 1024); builder.enableHttpCache(cacheType, 100 * 1024);
assertTrue(NativeTestServer.startNativeTestServer(getContext())); // Don't check the return value here, because startNativeTestServer() returns false when the
// NativeTestServer is already running and this method needs to be called twice without
// shutting down the NativeTestServer in between.
NativeTestServer.startNativeTestServer(getContext());
return builder.build(); return builder.build();
} }
...@@ -961,13 +965,20 @@ public class CronetUrlRequestContextTest extends CronetTestBase { ...@@ -961,13 +965,20 @@ public class CronetUrlRequestContextTest extends CronetTestBase {
@Feature({"Cronet"}) @Feature({"Cronet"})
@OnlyRunNativeCronet @OnlyRunNativeCronet
public void testEnableHttpCacheDiskNoHttp() throws Exception { public void testEnableHttpCacheDiskNoHttp() throws Exception {
// TODO(pauljensen): This should be testing HTTP_CACHE_DISK_NO_HTTP.
CronetEngine cronetEngine = CronetEngine cronetEngine =
createCronetEngineWithCache(CronetEngine.Builder.HTTP_CACHE_DISABLED); createCronetEngineWithCache(CronetEngine.Builder.HTTP_CACHE_DISK_NO_HTTP);
String url = NativeTestServer.getFileURL("/cacheable.txt"); String url = NativeTestServer.getFileURL("/cacheable.txt");
checkRequestCaching(cronetEngine, url, false); checkRequestCaching(cronetEngine, url, false);
checkRequestCaching(cronetEngine, url, false); checkRequestCaching(cronetEngine, url, false);
checkRequestCaching(cronetEngine, url, false); checkRequestCaching(cronetEngine, url, false);
// Make a new CronetEngine and try again to make sure the response didn't get cached on the
// first request. See https://crbug.com/743232.
cronetEngine.shutdown();
cronetEngine = createCronetEngineWithCache(CronetEngine.Builder.HTTP_CACHE_DISK_NO_HTTP);
checkRequestCaching(cronetEngine, url, false);
checkRequestCaching(cronetEngine, url, false);
checkRequestCaching(cronetEngine, url, false);
} }
@SmallTest @SmallTest
......
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