Add a ResourceScheduler to ResourceDispatcherHost.

content/browser/loader/resource_dispatcher_host_impl.cc

 // 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.
 
 // See http://dev.chromium.org/developers/design-documents/multi-process-resource-loading
 
 #include "content/browser/loader/resource_dispatcher_host_impl.h"
 
 #include <set>
 #include <vector>
@@ skipping 361 matching lines @@
       BrowserThread::IO, FROM_HERE,
       base::Bind(&appcache::AppCacheInterceptor::EnsureRegistered));
 
   update_load_states_timer_.reset(
       new base::RepeatingTimer<ResourceDispatcherHostImpl>());
 }
 
 ResourceDispatcherHostImpl::~ResourceDispatcherHostImpl() {
   DCHECK(g_resource_dispatcher_host);
   g_resource_dispatcher_host = NULL;
+  STLDeleteContainerPairSecondPointers(pending_loaders_.begin(),
+                                       pending_loaders_.end());
 }
 
 // static
 ResourceDispatcherHostImpl* ResourceDispatcherHostImpl::Get() {
   return g_resource_dispatcher_host;
 }
 
 void ResourceDispatcherHostImpl::SetDelegate(
     ResourceDispatcherHostDelegate* delegate) {
   delegate_ = delegate;
@@ skipping 17 matching lines @@
     ResourceContext* context) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   DCHECK(context);
 
   CHECK(ContainsKey(active_resource_contexts_, context));
 
   // Note that request cancellation has side effects. Therefore, we gather all
   // 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();
        i != pending_loaders_.end();) {
     if (i->second->GetRequestInfo()->GetContext() == context) {
       loaders_to_cancel.push_back(i->second);
       pending_loaders_.erase(i++);
     } else {
       ++i;
     }
   }
 
   for (BlockedLoadersMap::iterator i = blocked_loaders_map_.begin();
        i != blocked_loaders_map_.end();) {
     BlockedLoadersList* loaders = i->second;
     if (loaders->empty()) {
       // This can happen if BlockRequestsForRoute() has been called for a route,
       // but we haven't blocked any matching requests yet.
       ++i;
       continue;
     }
     ResourceRequestInfoImpl* info = loaders->front()->GetRequestInfo();
     if (info->GetContext() == context) {
       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;
     }
   }
 
 #ifndef NDEBUG
   for (LoaderList::iterator i = loaders_to_cancel.begin();
        i != loaders_to_cancel.end(); ++i) {
     // There is no strict requirement that this be the case, but currently
@@ skipping 306 matching lines @@
     return false;
   }
 
   return info->GetAssociatedRenderView(render_process_id, render_view_id);
 }
 
 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
   // runs if the timer is still running the Task is deleted twice (once by
   // the MessageLoop and the second time by RepeatingTimer).
   update_load_states_timer_.reset();
 
   // Clear blocked requests if any left.
   // Note that we have to do this in 2 passes as we cannot call
   // CancelBlockedRequestsForRoute while iterating over
@@ skipping 76 matching lines @@
   int child_id = filter_->child_id();
 
   // If we crash here, figure out what URL the renderer was requesting.
   // http://crbug.com/91398
   char url_buf[128];
   base::strlcpy(url_buf, request_data.url.spec().c_str(), arraysize(url_buf));
   base::debug::Alias(url_buf);
 
   // 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"));
         filter_->BadMessageReceived();
         return;
       }
     }
   }
 
   ResourceContext* resource_context = filter_->resource_context();
@@ skipping 20 matching lines @@
     AbortRequestBeforeItStarts(filter_, sync_result, route_id, request_id);
     return;
   }
 
   int load_flags =
       BuildLoadFlagsForRequest(request_data, child_id, sync_result != NULL);
 
   // 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
     // chance to reset some state before we complete the transfer.
     deferred_loader->WillCompleteTransfer();
   } else {
     net::URLRequestContext* context =
         filter_->GetURLRequestContext(request_data.resource_type);
     new_request.reset(context->CreateRequest(request_data.url, NULL));
     request = new_request.get();
@@ skipping 113 matching lines @@
         throttles.begin(),
         new TransferNavigationResourceThrottle(request));
   }
 
   if (!throttles.empty()) {
     handler.reset(
         new ThrottlingResourceHandler(handler.Pass(), child_id, request_id,
                                       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());
   }
 }
 
 void ResourceDispatcherHostImpl::OnReleaseDownloadedFile(int request_id) {
   UnregisterDownloadedTempFile(filter_->child_id(), request_id);
 }
 
 void ResourceDispatcherHostImpl::OnDataReceivedACK(int request_id) {
@@ skipping 301 matching lines @@
 
 void ResourceDispatcherHostImpl::RemovePendingLoader(
     const LoaderMap::iterator& iter) {
   ResourceRequestInfoImpl* info = iter->second->GetRequestInfo();
 
   // Remove the memory credit that we added when pushing the request onto
   // the pending list.
   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
   if (pending_loaders_.empty() && update_load_states_timer_.get())
     update_load_states_timer_->Stop();
 }
 
 void ResourceDispatcherHostImpl::CancelRequest(int child_id,
                                                int request_id,
                                                bool from_renderer) {
@@ skipping 85 matching lines @@
     }
 
     IncrementOutstandingRequestsMemoryCost(-1 * info->memory_cost(),
                                            info->GetChildID());
 
     // A ResourceHandler must not outlive its associated URLRequest.
     handler.reset();
     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) {
   last_user_gesture_time_ = TimeTicks::Now();
 }
 
 net::URLRequest* ResourceDispatcherHostImpl::GetURLRequest(
     const GlobalRequestID& id) {
   ResourceLoader* loader = GetLoader(id);
   if (!loader)
@@ skipping 145 matching lines @@
     return;
   }
 
   BlockedLoadersList* loaders = iter->second;
 
   // Removing the vector from the map unblocks any subsequent requests.
   blocked_loaders_map_.erase(iter);
 
   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;
 }
 
 ResourceDispatcherHostImpl::HttpAuthResourceType
 ResourceDispatcherHostImpl::HttpAuthResourceTypeOf(net::URLRequest* request) {
   // Use the same critera as for cookies to determine the sub-resource type
   // that is requesting to be authenticated.
   if (!request->first_party_for_cookies().is_valid())
     return HTTP_AUTH_RESOURCE_TOP;
 
@@ skipping 18 matching lines @@
 }
 
 ResourceLoader* ResourceDispatcherHostImpl::GetLoader(
     const GlobalRequestID& id) const {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
 
   LoaderMap::const_iterator i = pending_loaders_.find(id);
   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));
 }
 
 }  // namespace content