Add a ResourceScheduler to ResourceDispatcherHost.

content/browser/loader/resource_dispatcher_host_impl.cc

Index: content/browser/loader/resource_dispatcher_host_impl.cc
diff --git a/content/browser/loader/resource_dispatcher_host_impl.cc b/content/browser/loader/resource_dispatcher_host_impl.cc
index b0045d4b84d13418624d827742a7423f70797805..446d988ccffc0455cd27c26b69bca517a07a46f4 100644
--- a/content/browser/loader/resource_dispatcher_host_impl.cc
+++ b/content/browser/loader/resource_dispatcher_host_impl.cc
@@ -379,6 +379,8 @@ ResourceDispatcherHostImpl::ResourceDispatcherHostImpl()
 ResourceDispatcherHostImpl::~ResourceDispatcherHostImpl() {
   DCHECK(g_resource_dispatcher_host);
   g_resource_dispatcher_host = NULL;
+  STLDeleteContainerPairSecondPointers(pending_loaders_.begin(),
+                                       pending_loaders_.end());
 }
 
 // static
@@ -416,7 +418,7 @@ void ResourceDispatcherHostImpl::CancelRequestsForContext(
   // the requests to cancel first, and then we start cancelling. We assert at
   // the end that there are no more to cancel since the context is about to go
   // away.
-  typedef std::vector<linked_ptr<ResourceLoader> > LoaderList;
+  typedef ScopedVector<ResourceLoader> LoaderList;
   LoaderList loaders_to_cancel;
 
   for (LoaderMap::iterator i = pending_loaders_.begin();
@@ -443,15 +445,16 @@ void ResourceDispatcherHostImpl::CancelRequestsForContext(
       blocked_loaders_map_.erase(i++);
       for (BlockedLoadersList::const_iterator it = loaders->begin();
            it != loaders->end(); ++it) {
-        linked_ptr<ResourceLoader> loader = *it;
+        scoped_ptr<ResourceLoader> loader(*it);
         info = loader->GetRequestInfo();
         // We make the assumption that all requests on the list have the same
         // ResourceContext.
         DCHECK_EQ(context, info->GetContext());
         IncrementOutstandingRequestsMemoryCost(-1 * info->memory_cost(),
                                                info->GetChildID());
-        loaders_to_cancel.push_back(loader);
+        loaders_to_cancel.push_back(loader.release());
       }
+      loaders->weak_clear();
       delete loaders;
     } else {
       ++i;
@@ -778,6 +781,8 @@ void ResourceDispatcherHostImpl::OnShutdown() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
 
   is_shutdown_ = true;
+  STLDeleteContainerPairSecondPointers(pending_loaders_.begin(),

[darin (slow to review), 2012/12/14 22:16:28]

makes me wish we had a ScopedMap type to go along with ScopedVector.

+                                       pending_loaders_.end());
   pending_loaders_.clear();
 
   // Make sure we shutdown the timer now, otherwise by the time our destructor
@@ -874,14 +879,14 @@ void ResourceDispatcherHostImpl::BeginRequest(
 
   // If the request that's coming in is being transferred from another process,
   // we want to reuse and resume the old loader rather than start a new one.
-  linked_ptr<ResourceLoader> deferred_loader;
+  scoped_ptr<ResourceLoader> deferred_loader;
   {
     LoaderMap::iterator it = pending_loaders_.find(
         GlobalRequestID(request_data.transferred_request_child_id,
                         request_data.transferred_request_request_id));
     if (it != pending_loaders_.end()) {
       if (it->second->is_transferring()) {
-        deferred_loader = it->second;
+        deferred_loader.reset(it->second);
         pending_loaders_.erase(it);
       } else {
         RecordAction(UserMetricsAction("BadMessageTerminate_RDH"));
@@ -922,7 +927,7 @@ void ResourceDispatcherHostImpl::BeginRequest(
   // Construct the request.
   scoped_ptr<net::URLRequest> new_request;
   net::URLRequest* request;
-  if (deferred_loader.get()) {
+  if (deferred_loader) {
     request = deferred_loader->request();
 
     // Give the ResourceLoader (or any of the ResourceHandlers held by it) a
@@ -1056,9 +1061,11 @@ void ResourceDispatcherHostImpl::BeginRequest(
                                       throttles.Pass()));
   }
 
-  if (deferred_loader.get()) {
-    pending_loaders_[extra_info->GetGlobalRequestID()] = deferred_loader;
-    deferred_loader->CompleteTransfer(handler.Pass());
+  if (deferred_loader) {
+    pending_loaders_[extra_info->GetGlobalRequestID()] =

[darin (slow to review), 2012/12/14 22:16:28]

nit: seems a little bloaty codesighs-wise to repeat the map lookup.

+        deferred_loader.release();
+    pending_loaders_[extra_info->GetGlobalRequestID()]->CompleteTransfer(
+        handler.Pass());
   } else {
     BeginRequestInternal(new_request.Pass(), handler.Pass());
   }
@@ -1380,6 +1387,7 @@ void ResourceDispatcherHostImpl::RemovePendingLoader(
   IncrementOutstandingRequestsMemoryCost(-1 * info->memory_cost(),
                                          info->GetChildID());
 
+  delete iter->second;
   pending_loaders_.erase(iter);
 
   // If we have no more pending requests, then stop the load state monitor
@@ -1485,26 +1493,26 @@ void ResourceDispatcherHostImpl::BeginRequestInternal(
     return;
   }
 
-  linked_ptr<ResourceLoader> loader(
+  scoped_ptr<ResourceLoader> loader(
       new ResourceLoader(request.Pass(), handler.Pass(), this));
 
   ProcessRouteIDs pair_id(info->GetChildID(), info->GetRouteID());
   BlockedLoadersMap::const_iterator iter = blocked_loaders_map_.find(pair_id);
   if (iter != blocked_loaders_map_.end()) {
     // The request should be blocked.
-    iter->second->push_back(loader);
+    iter->second->push_back(loader.release());
     return;
   }
 
-  StartLoading(info, loader);
+  StartLoading(info, loader.Pass());
 }
 
 void ResourceDispatcherHostImpl::StartLoading(
     ResourceRequestInfoImpl* info,
-    const linked_ptr<ResourceLoader>& loader) {
-  pending_loaders_[info->GetGlobalRequestID()] = loader;
-
-  loader->StartRequest();
+    scoped_ptr<ResourceLoader> loader) {
+  pending_loaders_[info->GetGlobalRequestID()] = loader.release();

[darin (slow to review), 2012/12/14 22:16:28]

same nit

+  pending_loaders_[info->GetGlobalRequestID()]->ScheduleRequest(

[darin (slow to review), 2012/12/14 22:16:28]

This feels a bit awkward to me.  I don't think the ResourceLoader should have to
know about the ResourceScheduler.  It seems like the ResourceDispatcherHostImpl
should just pass the ResourceLoader to the ResourceScheduler.

I think I might also just pass the URLRequest pointer at this time instead of
making the ResourceScheduler get the URLRequest from the ResourceLoader.

It feels like the scheduler is just something that can delay starting a request,
and it would be nice to build on existing infrastructure for delaying a request.
 Adding more infrastructure here adds complexity.

+      &resource_scheduler_);
 }
 
 void ResourceDispatcherHostImpl::OnUserGesture(WebContentsImpl* contents) {
@@ -1670,16 +1678,17 @@ void ResourceDispatcherHostImpl::ProcessBlockedRequestsForRoute(
 
   for (BlockedLoadersList::iterator loaders_iter = loaders->begin();
        loaders_iter != loaders->end(); ++loaders_iter) {
-    linked_ptr<ResourceLoader> loader = *loaders_iter;
+    scoped_ptr<ResourceLoader> loader(*loaders_iter);
     ResourceRequestInfoImpl* info = loader->GetRequestInfo();
     if (cancel_requests) {
       IncrementOutstandingRequestsMemoryCost(-1 * info->memory_cost(),
                                              info->GetChildID());
     } else {
-      StartLoading(info, loader);
+      StartLoading(info, loader.Pass());
     }
   }
 
+  loaders->weak_clear();
   delete loaders;
 }
 
@@ -1718,11 +1727,11 @@ ResourceLoader* ResourceDispatcherHostImpl::GetLoader(
   if (i == pending_loaders_.end())
     return NULL;
 
-  return i->second.get();
+  return i->second;
 }
 
-ResourceLoader* ResourceDispatcherHostImpl::GetLoader(int child_id,
-                                                      int request_id) const {
+ResourceLoader* ResourceDispatcherHostImpl::GetLoader(
+    int child_id, int request_id) const {
   return GetLoader(GlobalRequestID(child_id, request_id));
 }