[on hold] Remove dependency on RenderViewHost from LoadState notifications

content/browser/loader/resource_dispatcher_host_impl.h

Index: content/browser/loader/resource_dispatcher_host_impl.h
diff --git a/content/browser/loader/resource_dispatcher_host_impl.h b/content/browser/loader/resource_dispatcher_host_impl.h
index 648822822bc627cc3549f69ebe7d5a8a7d724206..c143468bd38bc4735c5f37031c9f38f06dd14908 100644
--- a/content/browser/loader/resource_dispatcher_host_impl.h
+++ b/content/browser/loader/resource_dispatcher_host_impl.h
@@ -30,6 +30,7 @@
 #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_request_info_impl.h"
 #include "content/browser/loader/resource_scheduler.h"
 #include "content/common/content_export.h"
 #include "content/common/resource_request_body.h"
@@ -342,15 +343,20 @@ class CONTENT_EXPORT ResourceDispatcherHostImpl
   friend class ResourceMessageDelegate;
 
   // Information about status of a ResourceLoader.
-  struct LoadInfo {
+  struct CONTENT_EXPORT LoadInfo {
+    LoadInfo();
+    ~LoadInfo();
+    LoadInfo(const LoadInfo& info);

[mmenke, 2016/06/01 15:19:32]

Why do we need a copy constructor?  Some reason the default one doesn't do the
trick?

Also, constructors should go before destructor, per Google style guide.

[Charlie Harrison, 2016/06/02 14:22:28]

"Complex class/struct needs an explicit out-of-line copy constructor"
unfortunately. This is just chromium style. Moved it before the destructor.

I think the complexity comes from the Closure.

[mmenke, 2016/06/02 15:20:17]

Can't you just use "= default;" in the cc file?

[Charlie Harrison, 2016/06/06 15:39:53]

Oh, yeah I can do that. Done. Sorry I think I misunderstood you before.

     GURL url;

[mmenke, 2016/06/01 15:19:32]

Blank line between methods and members.

[Charlie Harrison, 2016/06/02 14:22:28]

Done.

     net::LoadStateWithParam load_state;
     uint64_t upload_position;
     uint64_t upload_size;
+    // Associates the load with a web contents.

[clamy, 2016/06/01 13:59:34]

nit: empty line above this line?

[Charlie Harrison, 2016/06/02 14:22:28]

Done.

+    ResourceRequestInfo::WebContentsGetter web_contents_getter;
   };
 
-  // Map from ProcessID+RouteID pair to the "most interesting" LoadState.
-  typedef std::map<GlobalRoutingID, LoadInfo> LoadInfoMap;
+  // Map from ProcessID+FrameRouteID pair to the "most interesting" LoadState.

[nasko, 2016/06/01 16:54:00]

I wonder if this comment is now correct. It used to be RVH id and LoadInfo,
which was 1->many mapping, since RVH id could reflect multiple frames. Now that
it is a RenderFrameHost and LoadInfo, isn't it 1->1 mapping? At which point it
is no longer "most interesting", but just a straight mapping. Or is it also
tracking all subresource requests too, which will make it 1->many again.

In general, I'd rather see "most interesting" clarified, as this is very
ambiguous and not very helpful.

[Charlie Harrison, 2016/06/02 14:22:28]

Yeah this is still a 1:many relationship because it tracks all subresource
loads. We should clarify "most interesting" though. Keeping logic as is for this
PS though.

+  typedef std::map<GlobalFrameRoutingId, LoadInfo> LoadInfoMap;
 
   // ResourceLoaderDelegate implementation:
   ResourceDispatcherHostLoginDelegate* CreateLoginDelegate(
@@ -452,7 +458,7 @@ class CONTENT_EXPORT ResourceDispatcherHostImpl
   // are done as a single callback to avoid spamming the UI thread.
   static void UpdateLoadInfoOnUIThread(std::unique_ptr<LoadInfoMap> info_map);
 
-  // Gets the most interesting LoadInfo for each GlobalRoutingID.
+  // Gets the most interesting LoadInfo for each GlobalFrameRoutingID.
   std::unique_ptr<LoadInfoMap> GetLoadInfoForAllRoutes();
 
   // Checks all pending requests and updates the load info if necessary.