[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_web_resource_response.cc b/android_webview/browser/net/aw_request_interceptor.cc
similarity index 43%
rename from android_webview/browser/aw_web_resource_response.cc
rename to android_webview/browser/net/aw_request_interceptor.cc
index 44f2aa87abdc9d2a3ea170d1c7c3cfc6d400cac5..e97c867cc655da7f83eb1be70d117714729f4b31 100644
--- a/android_webview/browser/aw_web_resource_response.cc
+++ b/android_webview/browser/net/aw_request_interceptor.cc
@@ -1,18 +1,26 @@
-// Copyright 2014 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // 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_web_resource_response.h"
+#include "android_webview/browser/net/aw_request_interceptor.h"
 
+#include "android_webview/browser/aw_contents_io_thread_client.h"
 #include "android_webview/browser/input_stream.h"
 #include "android_webview/browser/net/android_stream_reader_url_request_job.h"
+#include "android_webview/browser/net/aw_web_resource_response.h"
 #include "base/strings/string_number_conversions.h"
+#include "base/supports_user_data.h"
+#include "content/public/browser/browser_thread.h"
+#include "content/public/browser/resource_request_info.h"
 #include "net/http/http_response_headers.h"
+#include "net/url_request/url_request_job.h"
 
 namespace android_webview {
 
 namespace {
 
+const void* const kRequestAlreadyHasJobDataKey = &kRequestAlreadyHasJobDataKey;
+
 class StreamReaderJobDelegateImpl
     : public AndroidStreamReaderURLRequestJob::Delegate {
  public:
@@ -65,22 +73,84 @@ class StreamReaderJobDelegateImpl
   scoped_ptr<AwWebResourceResponse> aw_web_resource_response_;
 };
 
+class ShouldInterceptRequestAdaptor
+    : public AndroidStreamReaderURLRequestJob::DelegateObtainer {
+ public:
+  explicit ShouldInterceptRequestAdaptor(
+      scoped_ptr<AwContentsIoThreadClient> io_thread_client)
+      : io_thread_client_(io_thread_client.Pass()) {}
+   ~ShouldInterceptRequestAdaptor() override {}
+
+  void ObtainDelegate(net::URLRequest* request,
+                      Callback callback) override {
+    callback_ = callback;
+    io_thread_client_->ShouldInterceptRequestAsync(

[mmenke, 2015/10/01 18:34:40]

Is it a problem if io_thread_client_ is deleted while we're waiting for this
call?

[mnaganov (inactive), 2015/10/01 19:51:04]

ShouldInterceptRequestAdaptor owns io_thread_client_.

[mmenke, 2015/10/01 20:06:53]

Right, and the Job owns ShouldInterceptRequestAdaptor, so if the job is deleted,
so is the ShouldInterceptRequestAdaptor.  I was just wondering if the client is
happy to be deleted in the middle of a call.

[boliu, 2015/10/01 20:28:36]

That case looks fine :)

+        request,
+        base::Bind(&ShouldInterceptRequestAdaptor::WebResourceResponseObtained,
+                   // The lifetime of the DelegateObtainer is managed by
+                   // AndroidStreamReaderURLRequestJob.
+                   base::Unretained(this)));

[mmenke, 2015/10/01 18:34:40]

This should be a weak_ptr, no?  It is a bit unfortunate that we need to
double-weakptr this, but the job may just be cancelled (So we need the other
one), or it may be destroyed (So we need this one).

Other option would be to make the callback itself an argument to
WebResourceResponseObtained, and make that method static, and depend on callback
using a weak ptr, though that seems uglier to me.

[mmenke, 2015/10/01 18:41:35]

On second thought, if IOThreadClient handles being deleted while we're waiting
on the callback safely, and doesn't invoke it's callback, then we're fined with
Unretained here.

[boliu, 2015/10/01 19:13:56]

I don't think IOThreadClient does that. So this does need to be a weakptr.

I was checking if request pointer might become dangling too. It's safe because
it's only used inside ShouldInterceptRequestAsync call itself, not in the
callback. Worth a comment somewhere though.

[mnaganov (inactive), 2015/10/01 19:51:04]

Right, I have also realized that over the lunch. Using WeakPtr here seems like
the easiest option. Thanks for noticing this!

Re: request -- sure, added a comment.

+  }
+
+ private:
+  void WebResourceResponseObtained(
+      scoped_ptr<AwWebResourceResponse> response) {
+    if (response) {
+      callback_.Run(
+          make_scoped_ptr(new StreamReaderJobDelegateImpl(response.Pass()))
+              .Pass());
+    } else {
+      callback_.Run(nullptr);
+    }
+  }
+
+  scoped_ptr<AwContentsIoThreadClient> io_thread_client_;
+  Callback callback_;
+
+  DISALLOW_COPY_AND_ASSIGN(ShouldInterceptRequestAdaptor);
+};
+
 }  // namespace
 
-// static
-net::URLRequestJob* AwWebResourceResponse::CreateJobFor(
-    scoped_ptr<AwWebResourceResponse> aw_web_resource_response,
+AwRequestInterceptor::AwRequestInterceptor() {}
+
+AwRequestInterceptor::~AwRequestInterceptor() {}
+
+net::URLRequestJob* AwRequestInterceptor::MaybeInterceptRequest(
     net::URLRequest* request,
-    net::NetworkDelegate* network_delegate) {
-  DCHECK(aw_web_resource_response);
-  DCHECK(request);
-  DCHECK(network_delegate);
+    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 (!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)
+    return nullptr;
+
+  GURL referrer(request->referrer());
+  if (referrer.is_valid() &&
+      (!request->is_pending() || request->is_redirecting())) {
+    request->SetExtraRequestHeaderByName(net::HttpRequestHeaders::kReferer,
+                                         referrer.spec(), true);
+  }
+  request->SetUserData(kRequestAlreadyHasJobDataKey,
+                       new base::SupportsUserData::Data());
   return new AndroidStreamReaderURLRequestJob(
-      request,
-      network_delegate,
+      request, network_delegate,
       make_scoped_ptr(
-          new StreamReaderJobDelegateImpl(aw_web_resource_response.Pass())));
+          new ShouldInterceptRequestAdaptor(io_thread_client.Pass()))
+          .Pass(),

[mmenke, 2015/10/01 18:34:40]

nit:  .Pass() isn't needed with make_scoped_ptr, in general, even if you're
passing as a more general scoped_ptr type.

[mnaganov (inactive), 2015/10/01 19:51:04]

Neat, thanks!

+      true);
 }
 
 }  // namespace android_webview