[Android WebView] Call shouldInterceptRequest on a background thread

android_webview/browser/net/aw_request_interceptor.cc

Index: android_webview/browser/net/aw_request_interceptor.cc
diff --git a/android_webview/browser/aw_request_interceptor.cc b/android_webview/browser/net/aw_request_interceptor.cc
similarity index 38%
rename from android_webview/browser/aw_request_interceptor.cc
rename to android_webview/browser/net/aw_request_interceptor.cc
index ee39a6594e896936918ecb56ba87a3ae1ac93d57..f7d96034e95842bfb2e2e8ee3aa22dca87b76d5a 100644
--- a/android_webview/browser/aw_request_interceptor.cc
+++ b/android_webview/browser/net/aw_request_interceptor.cc
@@ -2,30 +2,20 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "android_webview/browser/aw_request_interceptor.h"
+#include "android_webview/browser/net/aw_request_interceptor.h"
 
 #include "android_webview/browser/aw_contents_io_thread_client.h"
-#include "android_webview/browser/aw_web_resource_response.h"
-#include "base/android/jni_string.h"
-#include "base/memory/scoped_ptr.h"
+#include "android_webview/browser/net/aw_stream_reader_job_delegate_impl.h"
+#include "base/supports_user_data.h"
 #include "content/public/browser/browser_thread.h"
-#include "content/public/browser/render_view_host.h"
 #include "content/public/browser/resource_request_info.h"
-#include "net/url_request/url_request.h"
-#include "net/url_request/url_request_context.h"
-#include "net/url_request/url_request_context_getter.h"
 #include "net/url_request/url_request_job.h"
 
-using content::BrowserThread;
-using content::RenderViewHost;
-using content::ResourceRequestInfo;
-
 namespace android_webview {
 
 namespace {
 
-const void* const kRequestAlreadyQueriedDataKey =
-    &kRequestAlreadyQueriedDataKey;
+const void* const kRequestAlreadyHasJobDataKey = &kRequestAlreadyHasJobDataKey;
 
 }  // namespace
 
@@ -35,20 +25,26 @@ AwRequestInterceptor::AwRequestInterceptor() {
 AwRequestInterceptor::~AwRequestInterceptor() {
 }
 
-scoped_ptr<AwWebResourceResponse>
-AwRequestInterceptor::QueryForAwWebResourceResponse(
-    net::URLRequest* request) const {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+net::URLRequestJob* AwRequestInterceptor::MaybeInterceptRequest(
+    net::URLRequest* request,
+    net::NetworkDelegate* network_delegate) const {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+
+  // MaybeInterceptRequest can be called multiple times for the same request.
+  if (request->GetUserData(kRequestAlreadyHasJobDataKey))
+    return nullptr;
+
   int render_process_id, render_frame_id;
-  if (!ResourceRequestInfo::GetRenderFrameForRequest(
-      request, &render_process_id, &render_frame_id))
-    return scoped_ptr<AwWebResourceResponse>();
+  if (!content::ResourceRequestInfo::GetRenderFrameForRequest(
+          request, &render_process_id, &render_frame_id)) {
+    return nullptr;
+  }
 
   scoped_ptr<AwContentsIoThreadClient> io_thread_client =
       AwContentsIoThreadClient::FromID(render_process_id, render_frame_id);
 
-  if (!io_thread_client.get())
-    return scoped_ptr<AwWebResourceResponse>();
+  if (!io_thread_client)
+    return nullptr;
 
   GURL referrer(request->referrer());
   if (referrer.is_valid() &&
@@ -56,35 +52,13 @@ AwRequestInterceptor::QueryForAwWebResourceResponse(
     request->SetExtraRequestHeaderByName(net::HttpRequestHeaders::kReferer,
                                          referrer.spec(), true);
   }
-  return io_thread_client->ShouldInterceptRequest(request).Pass();
-}
-
-net::URLRequestJob* AwRequestInterceptor::MaybeInterceptRequest(
-    net::URLRequest* request,
-    net::NetworkDelegate* network_delegate) const {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-
-  // See if we've already found out the aw_web_resource_response for this
-  // request.
-  // This is done not only for efficiency reasons, but also for correctness
-  // as it is possible for the Interceptor chain to be invoked more than once
-  // in which case we don't want to query the embedder multiple times.
-  // Note: The Interceptor chain is not invoked more than once if we create a
-  // URLRequestJob in this method, so this is only caching negative hits.
-  if (request->GetUserData(kRequestAlreadyQueriedDataKey))
-    return NULL;
-  request->SetUserData(kRequestAlreadyQueriedDataKey,
+  AndroidStreamReaderURLRequestJob* job =
+      new AndroidStreamReaderURLRequestJob(request, network_delegate);
+  request->SetUserData(kRequestAlreadyHasJobDataKey,
                        new base::SupportsUserData::Data());
-
-  scoped_ptr<AwWebResourceResponse> aw_web_resource_response =
-      QueryForAwWebResourceResponse(request);
-
-  if (!aw_web_resource_response)
-    return NULL;
-
-  // The newly created job will own the AwWebResourceResponse.
-  return AwWebResourceResponse::CreateJobFor(
-      aw_web_resource_response.Pass(), request, network_delegate);
+  io_thread_client->ShouldInterceptRequestAsync(
+      request, job->GetWebResourceResponseCallback());

[mmenke, 2015/09/30 21:13:34]

Up to you, but I think this makes more sense in
AndroidStreamReaderURLRequestJob::Start() - then you can also get rid of
has_been_started_, merge MaybeStart() with WebResourceResponObtained, and inline
GetWebResourceResponseCallback().

Note that you don't really gain any performance from doing this call sooner,
since we call Start() almost immediately after creating the job, anyways.  I
think the simpler code has fewer potential race conditions to worry about.

[mnaganov (inactive), 2015/10/01 18:04:30]

Thanks for suggestion! I've made that. This also allows
AndroidStreamReaderURLRequestJob not to know about
AwStreamReaderJobDelegateImpl, and instead deal with something that returns a
Delegate asynchronously, I called this DelegateObtainer.

+  return job;
 }
 
 }  // namespace android_webview