Deal with canceled requests when flushing deferred messages.

content/child/resource_dispatcher.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/child/resource_dispatcher.h"
 
 #include <utility>
 
@@ skipping 151 matching lines @@
     return true;
   }
 
   PendingRequestInfo* request_info = GetPendingRequestInfo(request_id);
   if (!request_info) {
     // Release resources in the message if it is a data message.
     ReleaseResourcesInDataMessage(message);
     return true;
   }
 
-  if (request_info->is_deferred) {
+  // Make sure any deferred messages are dispatched before we dispatch more.
+  if (request_info->is_deferred ||
+      !request_info->deferred_message_queue.empty()) {
     request_info->deferred_message_queue.push_back(new IPC::Message(message));
+    // Dispatches the messages only if the request is no longer deferred.

[kinuko, 2016/10/19 13:35:53]

nit: to make it clearer that FlushDeferredMessages don't flush things when
is_deferred is true can we have if (!is_deferred) here too or make the comment
even clearer, e.g. "it dispatches only when request_info->is_deferred is false"

[jb, 2016/10/20 13:06:50]

Makes sense. I will add an extra if for clarity.

+    FlushDeferredMessages(request_id);
     return true;
   }
-  // Make sure any deferred messages are dispatched before we dispatch more.
-  if (!request_info->deferred_message_queue.empty()) {
-    FlushDeferredMessages(request_id);
-    request_info = GetPendingRequestInfo(request_id);
-    DCHECK(request_info);
-    if (request_info->is_deferred) {
-      request_info->deferred_message_queue.push_back(new IPC::Message(message));
-      return true;
-    }
-  }
 
   DispatchMessage(message);
   return true;
 }
 
 ResourceDispatcher::PendingRequestInfo*
 ResourceDispatcher::GetPendingRequestInfo(int request_id) {
   PendingRequestMap::iterator it = pending_requests_.find(request_id);
   if (it == pending_requests_.end()) {
     // This might happen for kill()ed requests on the webkit end.
@@ skipping 377 matching lines @@
     IPC_MESSAGE_HANDLER(ResourceMsg_SetDataBuffer, OnSetDataBuffer)
     IPC_MESSAGE_HANDLER(ResourceMsg_InlinedDataChunkReceived,
                         OnReceivedInlinedDataChunk)
     IPC_MESSAGE_HANDLER(ResourceMsg_DataReceived, OnReceivedData)
     IPC_MESSAGE_HANDLER(ResourceMsg_DataDownloaded, OnDownloadedData)
     IPC_MESSAGE_HANDLER(ResourceMsg_RequestComplete, OnRequestComplete)
   IPC_END_MESSAGE_MAP()
 }
 
 void ResourceDispatcher::FlushDeferredMessages(int request_id) {
-  PendingRequestMap::iterator it = pending_requests_.find(request_id);
-  if (it == pending_requests_.end())  // The request could have become invalid.
-    return;
-  PendingRequestInfo* request_info = it->second.get();
-  if (request_info->is_deferred)
+  PendingRequestInfo* request_info = GetPendingRequestInfo(request_id);
+  if (!request_info || request_info->is_deferred)
     return;
   // Because message handlers could result in request_info being destroyed,
   // we need to work with a stack reference to the deferred queue.
   MessageQueue q;
   q.swap(request_info->deferred_message_queue);
   while (!q.empty()) {
     IPC::Message* m = q.front();
     q.pop_front();
     DispatchMessage(*m);
     delete m;
+    // We need to find the request again in the list as it may have completed
+    // by now and the request_info instance above may be invalid.
+    request_info = GetPendingRequestInfo(request_id);
+    if (!request_info) {
+      // The recipient is gone, the messages won't be handled and
+      // resources they might hold won't be released. Explicitly release
+      // them from here so that they won't leak.
+      ReleaseResourcesInMessageQueue(&q);

[kinuko, 2016/10/19 13:35:53]

I think if GetPendingRequestInfo returns null ReleaseResourcesInMessageQueue
should be also already called by RemovePendingRequest?

[jb, 2016/10/20 13:06:50]

In this context the queue in the request_info is swapped out for the empty
queue, so it won't be able to clean it.

+      return;
+    }
     // If this request is deferred in the context of the above message, then
     // we should honor the same and stop dispatching further messages.
-    // We need to find the request again in the list as it may have completed
-    // by now and the request_info instance above may be invalid.
-    PendingRequestMap::iterator index = pending_requests_.find(request_id);
-    if (index != pending_requests_.end()) {
-      PendingRequestInfo* pending_request = index->second.get();
-      if (pending_request->is_deferred) {
-        pending_request->deferred_message_queue.swap(q);
-        return;
-      }
+    if (request_info->is_deferred) {
+      request_info->deferred_message_queue.swap(q);
+      return;
     }
   }
 }
 
 void ResourceDispatcher::StartSync(
     std::unique_ptr<ResourceRequest> request,
     int routing_id,
     SyncLoadResponse* response,
     blink::WebURLRequest::LoadingIPCType ipc_type,
     mojom::URLLoaderFactory* url_loader_factory) {
@@ skipping 200 matching lines @@
     delete message;
   }
 }
 
 void ResourceDispatcher::SetResourceSchedulingFilter(
     scoped_refptr<ResourceSchedulingFilter> resource_scheduling_filter) {
   resource_scheduling_filter_ = resource_scheduling_filter;
 }
 
 }  // namespace content