Remove unused AwInterceptedRequestHandler code and add documentation for...

Remove unused AwInterceptedRequestHandler code and add documentation for AwProxyingURLLoaderFactory.

This patch:
- removes unused AwInterceptedRequestHandler class and related code.
- adds documentation for the AwProxyingURLLoaderFactory specifying it's
  functionality, the logic it contains, handled callbacks and threading
  considerations.

BUG=841556

Change-Id: I1c62b6db3d7c4d4915b72abdb07954f5642ce425
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1710574
Commit-Queue: Tim Volodine <timvolodine@chromium.org>
Reviewed-by: Richard Coles <torne@chromium.org>
Reviewed-by: Clark DuVall <cduvall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#680068}

android_webview/browser/aw_content_browser_client.cc

@@ -994,7 +994,7 @@ bool AwContentBrowserClient::HandleExternalProtocol(
     if (content::BrowserThread::CurrentlyOn(content::BrowserThread::IO)) {
       // Manages its own lifetime.
       new android_webview::AwProxyingURLLoaderFactory(
-          0 /* process_id */, std::move(request), nullptr, nullptr,
+          0 /* process_id */, std::move(request), nullptr,
           true /* intercept_only */);
     } else {
       base::PostTaskWithTraits(
@@ -1003,7 +1003,7 @@ bool AwContentBrowserClient::HandleExternalProtocol(
               [](network::mojom::URLLoaderFactoryRequest request) {
                 // Manages its own lifetime.
                 new android_webview::AwProxyingURLLoaderFactory(
-                    0 /* process_id */, std::move(request), nullptr, nullptr,
+                    0 /* process_id */, std::move(request), nullptr,
                     true /* intercept_only */);
               },
               std::move(request)));
@@ -1069,8 +1069,7 @@ bool AwContentBrowserClient::WillCreateURLLoaderFactory(
       FROM_HERE, {content::BrowserThread::IO},
       base::BindOnce(&AwProxyingURLLoaderFactory::CreateProxy, process_id,
                      std::move(proxied_receiver),
-                     std::move(target_factory_info),
-                     nullptr /* AwInterceptedRequestHandler */));
+                     std::move(target_factory_info)));
   return true;
 }
 
@@ -1088,8 +1087,7 @@ void AwContentBrowserClient::WillCreateURLLoaderFactoryForAppCacheSubresource(
       FROM_HERE, {content::BrowserThread::IO},
       base::BindOnce(&AwProxyingURLLoaderFactory::CreateProxy,
                      render_process_id, std::move(factory_receiver),
-                     std::move(pending_proxy),
-                     nullptr /* AwInterceptedRequestHandler */));
+                     std::move(pending_proxy)));
 }
 
 uint32_t AwContentBrowserClient::GetWebSocketOptions(

android_webview/browser/network_service/aw_proxying_url_loader_factory.cc

@@ -711,13 +711,10 @@ AwProxyingURLLoaderFactory::AwProxyingURLLoaderFactory(
     int process_id,
     network::mojom::URLLoaderFactoryRequest loader_request,
     network::mojom::URLLoaderFactoryPtrInfo target_factory_info,
-    std::unique_ptr<AwInterceptedRequestHandler> request_handler,
     bool intercept_only)
     : process_id_(process_id),
-      request_handler_(std::move(request_handler)),
       intercept_only_(intercept_only),
       weak_factory_(this) {
-  // actual creation of the factory
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
   DCHECK(!(intercept_only_ && target_factory_info));
   if (target_factory_info) {
@@ -738,14 +735,12 @@ AwProxyingURLLoaderFactory::~AwProxyingURLLoaderFactory() {}
 void AwProxyingURLLoaderFactory::CreateProxy(
     int process_id,
     network::mojom::URLLoaderFactoryRequest loader_request,
-    network::mojom::URLLoaderFactoryPtrInfo target_factory_info,
-    std::unique_ptr<AwInterceptedRequestHandler> request_handler) {
+    network::mojom::URLLoaderFactoryPtrInfo target_factory_info) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
 
   // will manage its own lifetime
   new AwProxyingURLLoaderFactory(process_id, std::move(loader_request),
-                                 std::move(target_factory_info),
-                                 std::move(request_handler), false);
+                                 std::move(target_factory_info), false);
 }
 
 void AwProxyingURLLoaderFactory::CreateLoaderAndStart(

android_webview/browser/network_service/aw_proxying_url_loader_factory.h

@@ -22,11 +22,32 @@
 
 namespace android_webview {
 
-class AwInterceptedRequestHandler {};
-
-// URL Loader Factory for android webview, for supporting request/response
-// interception, processing and callback invocation. Currently contains basic
-// pass-through implementation.
+// URL Loader Factory for Android WebView. This is the entry point for handling
+// Android WebView callbacks (i.e. error, interception and other callbacks) and
+// loading of android specific schemes and overridden responses.
+//
+// This class contains centralized logic for:
+//  - request interception and blocking,
+//  - setting load flags and headers,
+//  - loading requests depending on the scheme (e.g. different delegates are
+//    used for loading android assets/resources as compared to overridden
+//    responses).
+//  - handling errors (e.g. no input stream, redirect or safebrowsing related
+//    errors).
+//
+// In particular handles the following Android WebView callbacks:
+//  - shouldInterceptRequest
+//  - onReceivedError
+//  - onReceivedHttpError
+//  - onReceivedLoginRequest
+//
+// Threading:
+//  Currently the factory and the associated loader assume they live on the IO
+//  thread. This is also required by the shouldInterceptRequest callback (which
+//  should be called on a non-UI thread). The other callbacks (i.e.
+//  onReceivedError, onReceivedHttpError and onReceivedLoginRequest) are posted
+//  on the UI thread.
+//
 class AwProxyingURLLoaderFactory : public network::mojom::URLLoaderFactory {
  public:
   // Create a factory that will create specialized URLLoaders for Android
@@ -38,7 +59,6 @@ class AwProxyingURLLoaderFactory : public network::mojom::URLLoaderFactory {
       int process_id,
       network::mojom::URLLoaderFactoryRequest loader_request,
       network::mojom::URLLoaderFactoryPtrInfo target_factory_info,
-      std::unique_ptr<AwInterceptedRequestHandler> request_handler,
       bool intercept_only);
 
   ~AwProxyingURLLoaderFactory() override;
@@ -47,8 +67,7 @@ class AwProxyingURLLoaderFactory : public network::mojom::URLLoaderFactory {
   static void CreateProxy(
       int process_id,
       network::mojom::URLLoaderFactoryRequest loader,
-      network::mojom::URLLoaderFactoryPtrInfo target_factory_info,
-      std::unique_ptr<AwInterceptedRequestHandler> request_handler);
+      network::mojom::URLLoaderFactoryPtrInfo target_factory_info);
 
   void CreateLoaderAndStart(network::mojom::URLLoaderRequest loader,
                             int32_t routing_id,
@@ -69,10 +88,6 @@ class AwProxyingURLLoaderFactory : public network::mojom::URLLoaderFactory {
   mojo::BindingSet<network::mojom::URLLoaderFactory> proxy_bindings_;
   network::mojom::URLLoaderFactoryPtr target_factory_;
 
-  // TODO(timvolodine): consider functionality to have multiple interception
-  // handlers operating in sequence.
-  std::unique_ptr<AwInterceptedRequestHandler> request_handler_;
-
   // When true the loader resulting from this factory will only execute
   // intercept callback (shouldInterceptRequest). If that returns without
   // a response, the loader will abort loading.