Introduce ResourceRequesterInfo to abstract the requester of resource request

content/browser/loader/resource_message_filter.h

Index: content/browser/loader/resource_message_filter.h
diff --git a/content/browser/loader/resource_message_filter.h b/content/browser/loader/resource_message_filter.h
index bcc07138851c51c5c92be8f5e46738ff5e55150c..0b66c992c116dc8174540fc40b1086815488d8b2 100644
--- a/content/browser/loader/resource_message_filter.h
+++ b/content/browser/loader/resource_message_filter.h
@@ -31,6 +31,7 @@ namespace content {
 class ChromeAppCacheService;
 class ChromeBlobStorageContext;
 class ResourceContext;
+class ResourceRequesterInfo;
 class ServiceWorkerContextWrapper;
 
 // This class filters out incoming IPC messages for network requests and
@@ -47,13 +48,11 @@ class CONTENT_EXPORT ResourceMessageFilter
                               ResourceContext**,
                               net::URLRequestContext**)> GetContextsCallback;
 
-  // |appcache_service|, |blob_storage_context|, |file_system_context| may be
-  // NULL in unittests or for requests from the (NPAPI) plugin process.
-  // The |origin_pid| argument to |get_contexts_callback| is not used
-  // (and may be invalid) for requests that are NOT from the NPAPI plugin
-  // process.
+  // |appcache_service|, |blob_storage_context|, |file_system_context|,
+  // |service_worker_context| may be nullptr in unittests.
+  // InitializeOnIOThread() needs to be manually called for unittests where
+  // OnFilterAdded() is not called.

[mmenke, 2016/11/21 19:51:55]

nit:  "is not called" -> "would not otherwise be called"

[horo, 2016/11/22 01:12:07]

Done.

   ResourceMessageFilter(int child_id,
-                        int process_type,
                         ChromeAppCacheService* appcache_service,
                         ChromeBlobStorageContext* blob_storage_context,
                         storage::FileSystemContext* file_system_context,
@@ -61,36 +60,11 @@ class CONTENT_EXPORT ResourceMessageFilter
                         const GetContextsCallback& get_contexts_callback);
 
   // BrowserMessageFilter implementation.
+  void OnFilterAdded(IPC::Channel* channel) override;
   void OnChannelClosing() override;
   bool OnMessageReceived(const IPC::Message& message) override;
   void OnDestruct() const override;
 
-  void GetContexts(ResourceType resource_type,
-                   ResourceContext** resource_context,
-                   net::URLRequestContext** request_context);
-
-  // Returns the net::URLRequestContext for the given request.
-  net::URLRequestContext* GetURLRequestContext(ResourceType request_type);
-
-  ChromeAppCacheService* appcache_service() const {
-    return appcache_service_.get();
-  }
-
-  ChromeBlobStorageContext* blob_storage_context() const {
-    return blob_storage_context_.get();
-  }
-
-  storage::FileSystemContext* file_system_context() const {
-    return file_system_context_.get();
-  }
-
-  ServiceWorkerContextWrapper* service_worker_context() const {
-    return service_worker_context_.get();
-  }
-
-  int child_id() const { return child_id_; }
-  int process_type() const { return process_type_; }
-
   base::WeakPtr<ResourceMessageFilter> GetWeakPtr();
 
   void CreateLoaderAndStart(
@@ -103,27 +77,27 @@ class CONTENT_EXPORT ResourceMessageFilter
                 int32_t request_id,
                 const ResourceRequest& request,
                 const SyncLoadCallback& callback) override;
+  int child_id() const;
 
  protected:
+  friend class URLLoaderFactoryImplTest;

[mmenke, 2016/11/21 19:51:55]

This is in the wrong section, but see below comment about making a ForTesting
version of methods it needs and making them public.  Makes the API much clearer.

[horo, 2016/11/22 01:12:07]

Done.

+
   // Protected destructor so that we can be overriden in tests.
   ~ResourceMessageFilter() override;
 
+  // Initializes the weak pointer of this filter in |requester_info_|.
+  void InitializeOnIOThread();
+
  private:
   friend struct BrowserThread::DeleteOnThread<BrowserThread::IO>;
   friend class base::DeleteHelper<ResourceMessageFilter>;
+  friend class AsyncResourceHandlerTest;
+  friend class ResourceDispatcherHostTest;

[mmenke, 2016/11/21 19:51:55]

Instead of friending these, can we rename requester_info() to
requester_info_for_testing() / for_test(), and make it public?  Prefer that to
giving tests access to the guts of this class.  Alternatively, could modify the
tests to go through the ResourceMessageFilter's OnMessageReceived method, if
that works (Not sure if switching "just works", or if more work would be needed
to hook things up)

[horo, 2016/11/22 01:12:07]

Done.

 
-  // The ID of the child process.
-  int child_id_;
+  ResourceRequesterInfo* requester_info() { return requester_info_.get(); }
 
-  int process_type_;
   bool is_channel_closed_;
-
-  scoped_refptr<ChromeAppCacheService> appcache_service_;
-  scoped_refptr<ChromeBlobStorageContext> blob_storage_context_;
-  scoped_refptr<storage::FileSystemContext> file_system_context_;
-  scoped_refptr<ServiceWorkerContextWrapper> service_worker_context_;
-
-  GetContextsCallback get_contexts_callback_;
+  scoped_refptr<ResourceRequesterInfo> requester_info_;

[mmenke, 2016/11/21 19:51:55]

include ref_counted

[horo, 2016/11/22 01:12:07]

Done.

 
   // This must come last to make sure weak pointers are invalidated first.
   base::WeakPtrFactory<ResourceMessageFilter> weak_ptr_factory_;