Make URLRequestContextAdapter initialization asynchronous

Currently we place a 2-second on URLRequestContextAdapter initialization, so
that context can be initialized when embedders make requests. This CL changes
so that tasks that depend on context being initialized will be run after the
context is initialized properly, and URLRequestContextAdapter initialization
won't block.

BUG=

Review URL: https://codereview.chromium.org/617393005

Cr-Commit-Position: refs/heads/master@{#299145}

components/cronet/android/java/src/org/chromium/net/ChromiumUrlRequestContext.java

@@ -5,7 +5,6 @@
 package org.chromium.net;
 
 import android.content.Context;
-import android.os.ConditionVariable;
 import android.os.Process;
 import android.util.Log;
 
@@ -27,8 +26,6 @@ public class ChromiumUrlRequestContext {
      */
     private long mChromiumUrlRequestContextAdapter;
 
-    private final ConditionVariable mStarted = new ConditionVariable();
-
     /**
      * Constructor.
      *
@@ -37,11 +34,9 @@ public class ChromiumUrlRequestContext {
             String config) {
         mChromiumUrlRequestContextAdapter = nativeCreateRequestContextAdapter(
                 context, userAgent, getLoggingLevel(), config);
-        if (mChromiumUrlRequestContextAdapter == 0)
+        if (mChromiumUrlRequestContextAdapter == 0) {
             throw new NullPointerException("Context Adapter creation failed");
-
-        // TODO(mef): Revisit the need of block here.
-        mStarted.block(2000);
+        }
     }
 
     /**
@@ -90,7 +85,6 @@ public class ChromiumUrlRequestContext {
     private void initNetworkThread() {
         Thread.currentThread().setName("ChromiumNet");
         Process.setThreadPriority(Process.THREAD_PRIORITY_BACKGROUND);
-        mStarted.open();
     }
 
     @Override

components/cronet/android/url_request_adapter.cc

@@ -9,6 +9,7 @@
 #include "base/bind.h"
 #include "base/location.h"
 #include "base/logging.h"
+#include "base/single_thread_task_runner.h"
 #include "base/strings/string_number_conversions.h"
 #include "components/cronet/android/url_request_context_adapter.h"
 #include "components/cronet/android/wrapped_channel_upload_element_reader.h"
@@ -41,6 +42,7 @@ URLRequestAdapter::URLRequestAdapter(URLRequestContextAdapter* context,
 }
 
 URLRequestAdapter::~URLRequestAdapter() {
+  DCHECK(OnNetworkThread());
   CHECK(url_request_ == NULL);
 }
 
@@ -77,7 +79,7 @@ void URLRequestAdapter::AppendChunk(const char* bytes, int bytes_len,
   VLOG(1) << "AppendChunk, len: " << bytes_len << ", last: " << is_last_chunk;
   scoped_ptr<char[]> buf(new char[bytes_len]);
   memcpy(buf.get(), bytes, bytes_len);
-  context_->GetNetworkTaskRunner()->PostTask(
+  context_->PostTaskToNetworkThread(
       FROM_HERE,
       base::Bind(&URLRequestAdapter::OnAppendChunk,
                  base::Unretained(this),
@@ -108,7 +110,7 @@ std::string URLRequestAdapter::GetNegotiatedProtocol() const {
 }
 
 void URLRequestAdapter::Start() {
-  context_->GetNetworkTaskRunner()->PostTask(
+  context_->PostTaskToNetworkThread(
       FROM_HERE,
       base::Bind(&URLRequestAdapter::OnInitiateConnection,
                  base::Unretained(this)));
@@ -116,10 +118,12 @@ void URLRequestAdapter::Start() {
 
 void URLRequestAdapter::OnAppendChunk(const scoped_ptr<char[]> bytes,
                                       int bytes_len, bool is_last_chunk) {
+  DCHECK(OnNetworkThread());
   url_request_->AppendChunkToUpload(bytes.get(), bytes_len, is_last_chunk);
 }
 
 void URLRequestAdapter::OnInitiateConnection() {
+  DCHECK(OnNetworkThread());
   if (canceled_) {
     return;
   }
@@ -159,12 +163,13 @@ void URLRequestAdapter::Cancel() {
 
   canceled_ = true;
 
-  context_->GetNetworkTaskRunner()->PostTask(
+  context_->PostTaskToNetworkThread(
       FROM_HERE,
       base::Bind(&URLRequestAdapter::OnCancelRequest, base::Unretained(this)));
 }
 
 void URLRequestAdapter::OnCancelRequest() {
+  DCHECK(OnNetworkThread());
   VLOG(1) << "Canceling chromium request: " << url_.possibly_invalid_spec();
 
   if (url_request_ != NULL) {
@@ -175,18 +180,21 @@ void URLRequestAdapter::OnCancelRequest() {
 }
 
 void URLRequestAdapter::Destroy() {
-  context_->GetNetworkTaskRunner()->PostTask(
+  context_->PostTaskToNetworkThread(
       FROM_HERE, base::Bind(&URLRequestAdapter::OnDestroyRequest, this));
 }
 
 // static
 void URLRequestAdapter::OnDestroyRequest(URLRequestAdapter* self) {
+  DCHECK(self->OnNetworkThread());
   VLOG(1) << "Destroying chromium request: "
           << self->url_.possibly_invalid_spec();
   delete self;
 }
 
+// static
 void URLRequestAdapter::OnResponseStarted(net::URLRequest* request) {
+  DCHECK(OnNetworkThread());
   if (request->status().status() != net::URLRequestStatus::SUCCESS) {
     OnRequestFailed();
     return;
@@ -204,6 +212,7 @@ void URLRequestAdapter::OnResponseStarted(net::URLRequest* request) {
 
 // Reads all available data or starts an asynchronous read.
 void URLRequestAdapter::Read() {
+  DCHECK(OnNetworkThread());
   while (true) {
     if (read_buffer_->RemainingCapacity() == 0) {
       int new_capacity = read_buffer_->capacity() + kBufferSizeIncrement;
@@ -241,6 +250,7 @@ void URLRequestAdapter::Read() {
 
 void URLRequestAdapter::OnReadCompleted(net::URLRequest* request,
                                         int bytes_read) {
+  DCHECK(OnNetworkThread());
   VLOG(1) << "Asynchronously read: " << bytes_read << " bytes";
   if (bytes_read < 0) {
     OnRequestFailed();
@@ -255,12 +265,14 @@ void URLRequestAdapter::OnReadCompleted(net::URLRequest* request,
 }
 
 void URLRequestAdapter::OnBytesRead(int bytes_read) {
+  DCHECK(OnNetworkThread());
   read_buffer_->set_offset(read_buffer_->offset() + bytes_read);
   bytes_read_ += bytes_read;
   total_bytes_read_ += bytes_read;
 }
 
 void URLRequestAdapter::OnRequestSucceeded() {
+  DCHECK(OnNetworkThread());
   if (canceled_) {
     return;
   }
@@ -272,6 +284,7 @@ void URLRequestAdapter::OnRequestSucceeded() {
 }
 
 void URLRequestAdapter::OnRequestFailed() {
+  DCHECK(OnNetworkThread());
   if (canceled_) {
     return;
   }
@@ -283,10 +296,12 @@ void URLRequestAdapter::OnRequestFailed() {
 }
 
 void URLRequestAdapter::OnRequestCanceled() {
+  DCHECK(OnNetworkThread());
   OnRequestCompleted();
 }
 
 void URLRequestAdapter::OnRequestCompleted() {
+  DCHECK(OnNetworkThread());
   VLOG(1) << "Completed: " << url_.possibly_invalid_spec();
   url_request_.reset();
 
@@ -295,7 +310,12 @@ void URLRequestAdapter::OnRequestCompleted() {
 }
 
 unsigned char* URLRequestAdapter::Data() const {
+  DCHECK(OnNetworkThread());
   return reinterpret_cast<unsigned char*>(read_buffer_->StartOfBuffer());
 }
 
+bool URLRequestAdapter::OnNetworkThread() const {
+  return context_->GetNetworkTaskRunner()->BelongsToCurrentThread();
+}
+
 }  // namespace cronet

components/cronet/android/url_request_adapter.h

@@ -117,6 +117,8 @@ class URLRequestAdapter : public net::URLRequest::Delegate {
   virtual void OnReadCompleted(net::URLRequest* request,
                                int bytes_read) override;
 
+  bool OnNetworkThread() const;
+
  private:
   static void OnDestroyRequest(URLRequestAdapter* self);
 

components/cronet/android/url_request_context_adapter.cc

@@ -135,6 +135,7 @@ void URLRequestContextAdapter::Initialize(
 
 void URLRequestContextAdapter::InitializeURLRequestContext(
     scoped_ptr<URLRequestContextConfig> config) {
+  DCHECK(GetNetworkTaskRunner()->BelongsToCurrentThread());
   // TODO(mmenke):  Add method to have the builder enable SPDY.
   net::URLRequestContextBuilder context_builder;
   context_builder.set_network_delegate(new BasicNetworkDelegate());
@@ -178,6 +179,11 @@ void URLRequestContextAdapter::InitializeURLRequestContext(
           net::AlternateProtocol::QUIC,
           1.0f);
     }
+    is_context_initialized_ = true;
+    while (!tasks_waiting_for_context_.empty()) {
+      tasks_waiting_for_context_.front().Run();
+      tasks_waiting_for_context_.pop();
+    }
   }
 
   if (VLOG_IS_ON(2)) {
@@ -189,12 +195,34 @@ void URLRequestContextAdapter::InitializeURLRequestContext(
   delegate_->OnContextInitialized(this);
 }
 
+void URLRequestContextAdapter::PostTaskToNetworkThread(
+    const tracked_objects::Location& posted_from,
+    const RunAfterContextInitTask& callback) {
+  GetNetworkTaskRunner()->PostTask(
+      posted_from,
+      base::Bind(
+          &URLRequestContextAdapter::RunTaskAfterContextInitOnNetworkThread,
+          this,
+          callback));
+}
+
+void URLRequestContextAdapter::RunTaskAfterContextInitOnNetworkThread(
+    const RunAfterContextInitTask& callback) {
+  DCHECK(GetNetworkTaskRunner()->BelongsToCurrentThread());
+  if (is_context_initialized_) {
+    callback.Run();
+    return;
+  }
+  tasks_waiting_for_context_.push(callback);
+}
+
 URLRequestContextAdapter::~URLRequestContextAdapter() {
+  DCHECK(GetNetworkTaskRunner()->BelongsToCurrentThread());
   if (net_log_observer_) {
     context_->net_log()->RemoveThreadSafeObserver(net_log_observer_.get());
     net_log_observer_.reset();
   }
-  StopNetLog();
+  StopNetLogHelper();
   // TODO(mef): Ensure that |network_thread_| is destroyed properly.
 }
 
@@ -204,6 +232,7 @@ const std::string& URLRequestContextAdapter::GetUserAgent(
 }
 
 net::URLRequestContext* URLRequestContextAdapter::GetURLRequestContext() {
+  DCHECK(GetNetworkTaskRunner()->BelongsToCurrentThread());
   if (!context_) {
     LOG(ERROR) << "URLRequestContext is not set up";
   }
@@ -216,6 +245,20 @@ URLRequestContextAdapter::GetNetworkTaskRunner() const {
 }
 
 void URLRequestContextAdapter::StartNetLogToFile(const std::string& file_name) {
+  PostTaskToNetworkThread(
+      FROM_HERE,
+      base::Bind(
+          &URLRequestContextAdapter::StartNetLogToFileHelper, this, file_name));
+}
+
+void URLRequestContextAdapter::StopNetLog() {
+  PostTaskToNetworkThread(
+      FROM_HERE, base::Bind(&URLRequestContextAdapter::StopNetLogHelper, this));
+}
+
+void URLRequestContextAdapter::StartNetLogToFileHelper(
+    const std::string& file_name) {
+  DCHECK(GetNetworkTaskRunner()->BelongsToCurrentThread());
   // Do nothing if already logging to a file.
   if (net_log_logger_)
     return;
@@ -230,7 +273,8 @@ void URLRequestContextAdapter::StartNetLogToFile(const std::string& file_name) {
   net_log_logger_->StartObserving(context_->net_log());
 }
 
-void URLRequestContextAdapter::StopNetLog() {
+void URLRequestContextAdapter::StopNetLogHelper() {
+  DCHECK(GetNetworkTaskRunner()->BelongsToCurrentThread());
   if (net_log_logger_) {
     net_log_logger_->StopObserving();
     net_log_logger_.reset();

components/cronet/android/url_request_context_adapter.h

@@ -5,9 +5,12 @@
 #ifndef COMPONENTS_CRONET_ANDROID_URL_REQUEST_CONTEXT_ADAPTER_H_
 #define COMPONENTS_CRONET_ANDROID_URL_REQUEST_CONTEXT_ADAPTER_H_
 
+#include <queue>
 #include <string>
 
+#include "base/callback.h"
 #include "base/compiler_specific.h"
+#include "base/location.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
@@ -24,6 +27,7 @@ class NetLogLogger;
 namespace cronet {
 
 struct URLRequestContextConfig;
+typedef base::Callback<void(void)> RunAfterContextInitTask;
 
 // Implementation of the Chromium NetLog observer interface.
 class NetLogObserver : public net::NetLog::ThreadSafeObserver {
@@ -56,6 +60,16 @@ class URLRequestContextAdapter : public net::URLRequestContextGetter {
                            std::string user_agent);
   void Initialize(scoped_ptr<URLRequestContextConfig> config);
 
+  // Posts a task that might depend on the context being initialized
+  // to the network thread.
+  void PostTaskToNetworkThread(const tracked_objects::Location& posted_from,
+                               const RunAfterContextInitTask& callback);
+
+  // Runs a task that might depend on the context being initialized.
+  // This method should only be run on the network thread.
+  void RunTaskAfterContextInitOnNetworkThread(
+      const RunAfterContextInitTask& callback);
+
   const std::string& GetUserAgent(const GURL& url) const;
 
   // net::URLRequestContextGetter implementation:
@@ -75,11 +89,22 @@ class URLRequestContextAdapter : public net::URLRequestContextGetter {
   scoped_ptr<NetLogObserver> net_log_observer_;
   scoped_ptr<net::NetLogLogger> net_log_logger_;
 
+  // A queue of tasks that need to be run after context has been initialized.
+  std::queue<RunAfterContextInitTask> tasks_waiting_for_context_;
+  bool is_context_initialized_;
+
   virtual ~URLRequestContextAdapter();
 
   // Initializes |context_| on the Network thread.
   void InitializeURLRequestContext(scoped_ptr<URLRequestContextConfig> config);
 
+  // Helper function to start writing NetLog data to file. This should only be
+  // run after context is initialized.
+  void StartNetLogToFileHelper(const std::string& file_name);
+  // Helper function to stop writing NetLog data to file. This should only be
+  // run after context is initialized.
+  void StopNetLogHelper();
+
   DISALLOW_COPY_AND_ASSIGN(URLRequestContextAdapter);
 };