Remove the BeginSaveFile and BeginDownload methods from ResourceDispatcherHostImpl

content/browser/loader/resource_dispatcher_host_impl.h

 // 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.
 
 // This is the browser side of the resource dispatcher, it receives requests
 // from the child process (i.e. [Renderer, Plugin, Worker]ProcessHost), and
 // dispatches them to URLRequests. It then forwards the messages from the
 // URLRequests back to the correct process for handling.
 //
 // See http://dev.chromium.org/developers/design-documents/multi-process-resource-loading
 
 #ifndef CONTENT_BROWSER_LOADER_RESOURCE_DISPATCHER_HOST_IMPL_H_
 #define CONTENT_BROWSER_LOADER_RESOURCE_DISPATCHER_HOST_IMPL_H_
 
 #include <stdint.h>
 
 #include <map>
 #include <memory>
 #include <string>
 #include <vector>
 
 #include "base/gtest_prod_util.h"
 #include "base/macros.h"
 #include "base/observer_list.h"
 #include "base/time/time.h"
 #include "base/timer/timer.h"
-#include "content/browser/download/download_resource_handler.h"
 #include "content/browser/download/save_types.h"
 #include "content/browser/loader/global_routing_id.h"
 #include "content/browser/loader/resource_loader.h"
 #include "content/browser/loader/resource_loader_delegate.h"
 #include "content/browser/loader/resource_scheduler.h"
 #include "content/common/content_export.h"
 #include "content/common/url_loader.mojom.h"
 #include "content/public/browser/global_request_id.h"
 #include "content/public/browser/resource_dispatcher_host.h"
 #include "content/public/common/request_context_type.h"
@@ skipping 20 matching lines @@
 
 namespace content {
 class AppCacheService;
 class AsyncRevalidationManager;
 class CertStore;
 class LoaderDelegate;
 class NavigationURLLoaderImplCore;
 class RenderFrameHostImpl;
 class ResourceContext;
 class ResourceDispatcherHostDelegate;
+class ResourceHandler;
 class ResourceMessageDelegate;
 class ResourceMessageFilter;
 class ResourceRequestInfoImpl;
 class ServiceWorkerNavigationHandleCore;
 struct CommonNavigationParams;
-struct DownloadSaveInfo;
 struct NavigationRequestInfo;
 struct Referrer;
 struct ResourceRequest;
 
 namespace mojom {
 class URLLoader;
 }  // namespace mojom
 
+using CreateDownloadHandlerIntercept =
+    base::Callback<std::unique_ptr<ResourceHandler>(net::URLRequest*)>;
+
 class CONTENT_EXPORT ResourceDispatcherHostImpl
     : public ResourceDispatcherHost,
       public ResourceLoaderDelegate {
  public:
   ResourceDispatcherHostImpl();
   ~ResourceDispatcherHostImpl() override;
 
   // Returns the current ResourceDispatcherHostImpl. May return NULL if it
   // hasn't been created yet.
   static ResourceDispatcherHostImpl* Get();
@@ skipping 30 matching lines @@
 
   // Force cancels any pending requests for the given |context|. This is
   // necessary to ensure that before |context| goes away, all requests
   // for it are dead.
   void CancelRequestsForContext(ResourceContext* context);
 
   // Returns true if the message was a resource message that was processed.
   bool OnMessageReceived(const IPC::Message& message,
                          ResourceMessageFilter* filter);
 
-  DownloadInterruptReason BeginDownload(
-      std::unique_ptr<net::URLRequest> request,
-      const Referrer& referrer,
-      bool is_content_initiated,
-      ResourceContext* context,
-      int render_process_id,
-      int render_view_route_id,
-      int render_frame_route_id,
-      bool do_not_prompt_for_login);
-
-  // Initiates a save file from the browser process (as opposed to a resource
-  // request from the renderer or another child process).
-  void BeginSaveFile(const GURL& url,
-                     const Referrer& referrer,
-                     SaveItemId save_item_id,
-                     SavePackageId save_package_id,
-                     int child_id,
-                     int render_view_route_id,
-                     int render_frame_route_id,
-                     ResourceContext* context);
-
   // Cancels the given request if it still exists.
   void CancelRequest(int child_id, int request_id);
 
   // Marks the request, with its current |response|, as "parked". This
   // happens if a request is redirected cross-site and needs to be
   // resumed by a new process.
   void MarkAsTransferredNavigation(
       const GlobalRequestID& id,
       const scoped_refptr<ResourceResponse>& response);
 
@@ skipping 74 matching lines @@
   // dialog boxes.
   bool allow_cross_origin_auth_prompt();
 
   ResourceDispatcherHostDelegate* delegate() {
     return delegate_;
   }
 
   // Must be called after the ResourceRequestInfo has been created
   // and associated with the request.
   // This is marked virtual so it can be overriden in testing.
+  // TODO(ananta)
+  // This method should be removed or moved outside this class.
   virtual std::unique_ptr<ResourceHandler> CreateResourceHandlerForDownload(
       net::URLRequest* request,
       bool is_content_initiated,
       bool must_download);
 
   // Called to determine whether the response to |request| should be intercepted
   // and handled as a stream. Streams are used to pass direct access to a
   // resource response to another application (e.g. a web page) without being
   // handled by the browser itself. If the request should be intercepted as a
   // stream, a StreamResourceHandler is returned which provides access to the
@@ skipping 45 matching lines @@
 
   // Called when loading a request with mojo.
   void OnRequestResourceWithMojo(
       int routing_id,
       int request_id,
       const ResourceRequest& request,
       mojo::InterfaceRequest<mojom::URLLoader> mojo_request,
       mojom::URLLoaderClientPtr url_loader_client,
       ResourceMessageFilter* filter);
 
+  // This enum contains possible return values for the BeginURLRequest()
+  // function declared below.
+  enum URLRequestStatus {

[mmenke, 2016/08/22 14:40:08]

Given that we already have a net::URLRequestState class that the loader/
directory uses extensively, I don't think we want an enum by that same name.  Do
we even need an enum, or could we just return a bool indicating if starting the
request suceeded or failed?  If it failed for some other reason, I'd prefer we
go through the normal failure path, anyways (Create a request and then fail it
with an error).  Shutdown is a bit magical, in that creating a request at that
point in time may be problematic.

[ananta, 2016/08/22 19:14:57]

Left it as void. It does not return anything useful at the moment.

+    URL_REQUEST_FAILED_SHUTDOWN = 0,
+    URL_REQUEST_ISSUED = 1,
+    URL_REQUEST_LAST = URL_REQUEST_ISSUED,
+  };
+
+  // Helper function for initiating a URL request. The |is_download| and
+  // |is_content_initiated and |do_not_prompt_for_login| parameters are
+  // specific to download requests.
+  // TODO(ananta)
+  // Look into a better way of passing these parameters in.
+  // Returns values defined in the URLRequestStatus enum above.
+  URLRequestStatus BeginURLRequest(std::unique_ptr<net::URLRequest> request,
+                                   std::unique_ptr<ResourceHandler> handler,
+                                   const Referrer& referrer,
+                                   bool is_download,
+                                   bool is_content_initiated,
+                                   bool do_not_prompt_for_login,
+                                   int render_process_host_id,
+                                   int render_view_routing_id,
+                                   int render_frame_routing_id,
+                                   ResourceContext* context);
+
+  bool is_shutdown() const { return is_shutdown_; }

[Randy Smith (Not in Mondays), 2016/08/21 23:32:57]

I don't think this function is needed anymore?

[ananta, 2016/08/22 19:14:57]

Needed. We check the shutdown state in the DownloadManagerImpl as in one of the
previous patchsets.

+
+  // Registers the |intercept| as the callback to be invoked when the download
+  // handler is created for handling downloads.
+  // TODO(ananta)
+  // Investigate if we can find a better way to do this.
+  void RegisterCreateDownloadHandlerInterceptor(
+      CreateDownloadHandlerIntercept intercept);
+
  private:
   friend class ResourceDispatcherHostTest;
 
   FRIEND_TEST_ALL_PREFIXES(ResourceDispatcherHostTest,
                            TestBlockedRequestsProcessDies);
   FRIEND_TEST_ALL_PREFIXES(ResourceDispatcherHostTest,
                            CalculateApproximateMemoryCost);
   FRIEND_TEST_ALL_PREFIXES(ResourceDispatcherHostTest,
                            DetachableResourceTimesOut);
   FRIEND_TEST_ALL_PREFIXES(ResourceDispatcherHostTest,
@@ skipping 49 matching lines @@
   void DidFinishLoading(ResourceLoader* loader) override;
   std::unique_ptr<net::ClientCertStore> CreateClientCertStore(
       ResourceLoader* loader) override;
 
   // An init helper that runs on the IO thread.
   void OnInit();
 
   // A shutdown helper that runs on the IO thread.
   void OnShutdown();
 
-  // Helper function for regular and download requests.
+  // Helper function for URL requests.
   void BeginRequestInternal(std::unique_ptr<net::URLRequest> request,
                             std::unique_ptr<ResourceHandler> handler);
 
   void StartLoading(ResourceRequestInfoImpl* info,
                     std::unique_ptr<ResourceLoader> loader);
 
   // We keep track of how much memory each request needs and how many requests
   // are issued by each renderer. These are known as OustandingRequestStats.
   // Memory limits apply to all requests sent to us by the renderers. There is a
   // limit for each renderer. File descriptor limits apply to requests that are
@@ skipping 216 matching lines @@
   // ResourceDispatcherHostImpl should service this request.  A request might
   // be disallowed if the renderer is not authorized to retrieve the request
   // URL or if the renderer is attempting to upload an unauthorized file.
   bool ShouldServiceRequest(int process_type,
                             int child_id,
                             const ResourceRequest& request_data,
                             const net::HttpRequestHeaders& headers,
                             ResourceMessageFilter* filter,
                             ResourceContext* resource_context);
 
+  // Notifies the ResourceDispatcherHostDelegate about a download having
+  // started. The function returns the |handler| passed in, if the download
+  // is not throttled. If the download is to be throttled (Decided by the
+  // delegate) the function returns a ThrottlingResourceHandler to handle the
+  // download.
+  std::unique_ptr<ResourceHandler> HandleDownloadStarted(
+      net::URLRequest* request,
+      std::unique_ptr<ResourceHandler> handler,
+      bool is_content_initiated,
+      bool must_download);
+
   LoaderMap pending_loaders_;
 
   // Collection of temp files downloaded for child processes via
   // the download_to_file mechanism. We avoid deleting them until
   // the client no longer needs them.
   typedef std::map<int, scoped_refptr<storage::ShareableFileReference> >
       DeletableFilesMap;  // key is request id
   typedef std::map<int, DeletableFilesMap>
       RegisteredTempFiles;  // key is child process id
   RegisteredTempFiles registered_temp_files_;
@@ skipping 74 matching lines @@
 
   std::unique_ptr<ResourceScheduler> scheduler_;
 
   // Allows tests to use a mock CertStore. If set, the CertStore must
   // outlive this ResourceDispatcherHostImpl.
   CertStore* cert_store_for_testing_;
 
   // Used to invoke an interceptor for the HTTP header.
   HeaderInterceptorMap http_header_interceptor_map_;
 
+  // Points to the registered download handler intercept.
+  CreateDownloadHandlerIntercept create_download_handler_intercept_;
+
   DISALLOW_COPY_AND_ASSIGN(ResourceDispatcherHostImpl);
 };
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_LOADER_RESOURCE_DISPATCHER_HOST_IMPL_H_