[AW NS] Implement shouldInterceptRequest callback, null response case.

Implement shouldInterceptRequest callback in Android WebView when Network Service is enabled. This CL implements the so-called null response case, i.e. when the embedder does not actualy override network traffic with custom responses. This patch also contains some modifications to the AwContentsIoThreadClient class in order to accomodate both code paths (the current code path operating with net::URLRequest and the new code path using network::ResourceRequest). Fixed tests: -AwContentsClientShouldInterceptRequestTest.testCalledForIframe -AwContentsClientShouldInterceptRequestTest.testCalledForImage -AwContentsClientShouldInterceptRequestTest.testCalledForNonexistentAsset -AwContentsClientShouldInterceptRequestTest.testCalledForNonexistentContentUrl -AwContentsClientShouldInterceptRequestTest.testCalledForNonexistentFiles -AwContentsClientShouldInterceptRequestTest.testCalledForNonexistentResource -AwContentsClientShouldInterceptRequestTest.testCalledWithCorrectHasUserGestureParam -AwContentsClientShouldInterceptRequestTest.testCalledWithCorrectHeadersParam -AwContentsClientShouldInterceptRequestTest.testCalledWithCorrectIsMainFrameParam -AwContentsClientShouldInterceptRequestTest.testCalledWithCorrectMethodParam -AwContentsClientShouldInterceptRequestTest.testCalledWithCorrectUrlParam -AwContentsClientShouldInterceptRequestTest.testDeadlock -AwContentsClientShouldInterceptRequestTest.testDoesNotCrashOnInvalidData -AwContentsClientShouldInterceptRequestTest.testOnLoadResourceCalledWithCorrectUrl BUG=893566,841556 Cq-Include-Trybots: master.tryserver.chromium.android:android_mojo Change-Id: I6872a71670d33cc6726771ffb5341df4904dfd38 Reviewed-on: https://chromium-review.googlesource.com/c/1345029 Commit-Queue: Tim Volodine <[email protected]> Reviewed-by: John Abd-El-Malek <[email protected]> Reviewed-by: Clark DuVall <[email protected]> Reviewed-by: Richard Coles <[email protected]> Cr-Commit-Position: refs/heads/master@{#610686}
feiyue1206 · Nov 23, 2018 · 978fb98 · 978fb98
1 parent d32b6bf
commit 978fb98
Show file tree

Hide file tree

Showing 5 changed files with 40 additions and 20 deletions.
diff --git a/android_webview/browser/aw_contents_io_thread_client.cc b/android_webview/browser/aw_contents_io_thread_client.cc
@@ -29,6 +29,7 @@
 #include "jni/AwContentsIoThreadClient_jni.h"
 #include "net/base/data_url.h"
 #include "net/url_request/url_request.h"
+#include "services/network/public/cpp/resource_request.h"
 
 using base::android::AttachCurrentThread;
 using base::android::ConvertUTF8ToJavaString;
@@ -341,7 +342,7 @@ AwContentsIoThreadClient::CacheMode AwContentsIoThreadClient::GetCacheMode()
 namespace {
 
 std::unique_ptr<AwWebResourceResponse> RunShouldInterceptRequest(
-    const AwWebResourceRequest& request,
+    AwWebResourceRequest request,
     JavaObjectWeakGlobalRef ref) {
   base::ScopedBlockingCall scoped_blocking_call(base::BlockingType::MAY_BLOCK);
 
@@ -372,7 +373,7 @@ std::unique_ptr<AwWebResourceResponse> ReturnNull() {
 }  // namespace
 
 void AwContentsIoThreadClient::ShouldInterceptRequestAsync(
-    const net::URLRequest* request,
+    AwWebResourceRequest request,
     ShouldInterceptRequestResultCallback callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   base::OnceCallback<std::unique_ptr<AwWebResourceResponse>()> get_response =
@@ -385,7 +386,7 @@ void AwContentsIoThreadClient::ShouldInterceptRequestAsync(
   }
   if (!bg_thread_client_object_.is_null()) {
     get_response = base::BindOnce(
-        &RunShouldInterceptRequest, AwWebResourceRequest(*request),
+        &RunShouldInterceptRequest, std::move(request),
         JavaObjectWeakGlobalRef(env, bg_thread_client_object_.obj()));
   }
   base::PostTaskAndReplyWithResult(sequenced_task_runner_.get(), FROM_HERE,

diff --git a/android_webview/browser/aw_contents_io_thread_client.h b/android_webview/browser/aw_contents_io_thread_client.h
@@ -27,6 +27,7 @@ class URLRequest;
 namespace android_webview {
 
 class AwWebResourceResponse;
+struct AwWebResourceRequest;
 
 // This class provides a means of calling Java methods on an instance that has
 // a 1:1 relationship with a WebContents instance directly from the IO thread.
@@ -111,7 +112,7 @@ class AwContentsIoThreadClient {
   using ShouldInterceptRequestResultCallback =
       base::OnceCallback<void(std::unique_ptr<AwWebResourceResponse>)>;
   void ShouldInterceptRequestAsync(
-      const net::URLRequest* request,
+      AwWebResourceRequest request,
       ShouldInterceptRequestResultCallback callback);
 
   // Retrieve the AllowContentAccess setting value of this AwContents.

diff --git a/android_webview/browser/aw_proxying_url_loader_factory.cc b/android_webview/browser/aw_proxying_url_loader_factory.cc
@@ -8,6 +8,7 @@
 
 #include "android_webview/browser/aw_contents_client_bridge.h"
 #include "android_webview/browser/aw_contents_io_thread_client.h"
+#include "android_webview/browser/net/aw_web_resource_response.h"
 #include "android_webview/browser/net_helpers.h"
 #include "android_webview/browser/renderer_host/auto_login_parser.h"
 #include "base/strings/stringprintf.h"
@@ -71,6 +72,10 @@ class InterceptedRequest : public network::mojom::URLLoader,
   void PauseReadingBodyFromNet() override;
   void ResumeReadingBodyFromNet() override;
 
+  void ContinueAfterIntercept();
+  void InterceptResponseReceived(
+      std::unique_ptr<AwWebResourceResponse> response);
+
  private:
   std::unique_ptr<AwContentsIoThreadClient> GetIoThreadClient();
   void OnRequestError(const network::URLLoaderCompletionStatus& status);
@@ -131,8 +136,31 @@ void InterceptedRequest::Restart() {
   // TODO(timvolodine): add async check shouldOverrideUrlLoading and
   // shouldInterceptRequest.
 
-  request_.load_flags = GetCacheModeForClient(GetIoThreadClient().get());
+  std::unique_ptr<AwContentsIoThreadClient> io_thread_client =
+      GetIoThreadClient();
+  DCHECK(io_thread_client);
+  request_.load_flags = GetCacheModeForClient(io_thread_client.get());
+
+  // TODO: verify the case when WebContents::RenderFrameDeleted is called
+  // before network request is intercepted (i.e. if that's possible and
+  // whether it can result in any issues).
+  io_thread_client->ShouldInterceptRequestAsync(
+      AwWebResourceRequest(request_),
+      base::BindOnce(&InterceptedRequest::InterceptResponseReceived,
+                     weak_factory_.GetWeakPtr()));
+}
+
+void InterceptedRequest::InterceptResponseReceived(
+    std::unique_ptr<AwWebResourceResponse> response) {
+  if (response) {
+    // TODO(timvolodine): handle the case where response contains data,
+    // i.e. is actually overridden, crbug.com/893566.
+  } else {
+    ContinueAfterIntercept();
+  }
+}
 
+void InterceptedRequest::ContinueAfterIntercept() {
   if (!target_loader_ && target_factory_) {
     network::mojom::URLLoaderClientPtr proxied_client;
     proxied_client_binding_.Bind(mojo::MakeRequest(&proxied_client));

diff --git a/android_webview/browser/net/aw_request_interceptor.cc b/android_webview/browser/net/aw_request_interceptor.cc
@@ -10,6 +10,7 @@
 #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_request.h"
 #include "android_webview/browser/net/aw_web_resource_response.h"
 #include "base/memory/weak_ptr.h"
 #include "base/strings/string_number_conversions.h"
@@ -90,7 +91,7 @@ class ShouldInterceptRequestAdaptor
     callback_ = std::move(callback);
     io_thread_client_->ShouldInterceptRequestAsync(
         // The request is only used while preparing the call, not retained.
-        request,
+        AwWebResourceRequest(*request),
         base::BindOnce(
             &ShouldInterceptRequestAdaptor::WebResourceResponseObtained,
             // The lifetime of the DelegateObtainer is managed by

diff --git a/testing/buildbot/filters/mojo.fyi.network_webview_instrumentation_test_apk.filter b/testing/buildbot/filters/mojo.fyi.network_webview_instrumentation_test_apk.filter
@@ -33,25 +33,12 @@
 # https://crbug.com/893566
 -org.chromium.android_webview.test.AwContentsClientShouldInterceptRequestTest.testBaseUrlOfLoadDataSentInRefererHeader
 -org.chromium.android_webview.test.AwContentsClientShouldInterceptRequestTest.testCalledForExistingFiles
--org.chromium.android_webview.test.AwContentsClientShouldInterceptRequestTest.testCalledForIframe
--org.chromium.android_webview.test.AwContentsClientShouldInterceptRequestTest.testCalledForImage
--org.chromium.android_webview.test.AwContentsClientShouldInterceptRequestTest.testCalledForNonexistentAsset
--org.chromium.android_webview.test.AwContentsClientShouldInterceptRequestTest.testCalledForNonexistentContentUrl
--org.chromium.android_webview.test.AwContentsClientShouldInterceptRequestTest.testCalledForNonexistentFiles
--org.chromium.android_webview.test.AwContentsClientShouldInterceptRequestTest.testCalledForNonexistentResource
 -org.chromium.android_webview.test.AwContentsClientShouldInterceptRequestTest.testCalledForUnsupportedSchemes
--org.chromium.android_webview.test.AwContentsClientShouldInterceptRequestTest.testCalledWithCorrectHasUserGestureParam
--org.chromium.android_webview.test.AwContentsClientShouldInterceptRequestTest.testCalledWithCorrectHeadersParam
--org.chromium.android_webview.test.AwContentsClientShouldInterceptRequestTest.testCalledWithCorrectIsMainFrameParam
--org.chromium.android_webview.test.AwContentsClientShouldInterceptRequestTest.testCalledWithCorrectMethodParam
 -org.chromium.android_webview.test.AwContentsClientShouldInterceptRequestTest.testCalledWithCorrectRefererHeader
--org.chromium.android_webview.test.AwContentsClientShouldInterceptRequestTest.testCalledWithCorrectUrlParam
 -org.chromium.android_webview.test.AwContentsClientShouldInterceptRequestTest.testCanInterceptMainFrame
 -org.chromium.android_webview.test.AwContentsClientShouldInterceptRequestTest.testContentIdIframe
 -org.chromium.android_webview.test.AwContentsClientShouldInterceptRequestTest.testContentIdImage
--org.chromium.android_webview.test.AwContentsClientShouldInterceptRequestTest.testDeadlock
 -org.chromium.android_webview.test.AwContentsClientShouldInterceptRequestTest.testDoesNotCrashOnEmptyStream
--org.chromium.android_webview.test.AwContentsClientShouldInterceptRequestTest.testDoesNotCrashOnInvalidData
 -org.chromium.android_webview.test.AwContentsClientShouldInterceptRequestTest.testDoesNotCrashOnSlowStream
 -org.chromium.android_webview.test.AwContentsClientShouldInterceptRequestTest.testDoesNotCrashOnThrowingStream
 -org.chromium.android_webview.test.AwContentsClientShouldInterceptRequestTest.testHttpResponseClientViaHeader
@@ -65,7 +52,9 @@
 -org.chromium.android_webview.test.AwContentsClientShouldInterceptRequestTest.testNotCalledForExistingResource
 -org.chromium.android_webview.test.AwContentsClientShouldInterceptRequestTest.testNotCalledForHttpRedirect
 -org.chromium.android_webview.test.AwContentsClientShouldInterceptRequestTest.testNullInputStreamCausesErrorForMainFrame
--org.chromium.android_webview.test.AwContentsClientShouldInterceptRequestTest.testOnLoadResourceCalledWithCorrectUrl
+-org.chromium.android_webview.test.AwContentsClientShouldInterceptRequestTest.testNullHttpResponseHeaders
+-org.chromium.android_webview.test.AwContentsClientShouldInterceptRequestTest.testDoesNotChangeReportedUrl
+-org.chromium.android_webview.test.AwContentsClientShouldInterceptRequestTest.testNoOnReceivedErrorCallback
 
 # https://crbug.com/893568
 -org.chromium.android_webview.test.AwContentsTest.testCanInjectHeaders