Deal with canceled requests when flushing deferred messages.

content/child/resource_dispatcher.cc

Index: content/child/resource_dispatcher.cc
diff --git a/content/child/resource_dispatcher.cc b/content/child/resource_dispatcher.cc
index 4398e238603db7c6cca5b11030d9fb750cddbf1e..35ee99ad7dea92efd38fcf3e55cfb889c9141a1a 100644
--- a/content/child/resource_dispatcher.cc
+++ b/content/child/resource_dispatcher.cc
@@ -169,19 +169,13 @@ bool ResourceDispatcher::OnMessageReceived(const IPC::Message& message) {
     return true;
   }
 
-  if (request_info->is_deferred) {
-    request_info->deferred_message_queue.push_back(new IPC::Message(message));
-    return true;
-  }
   // Make sure any deferred messages are dispatched before we dispatch more.
-  if (!request_info->deferred_message_queue.empty()) {
+  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);
-    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;
-    }
+    return true;
   }
 
   DispatchMessage(message);
@@ -580,11 +574,8 @@ void ResourceDispatcher::DispatchMessage(const IPC::Message& message) {
 }
 
 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.
@@ -595,17 +586,21 @@ void ResourceDispatcher::FlushDeferredMessages(int request_id) {
     q.pop_front();
     DispatchMessage(*m);
     delete m;
-    // 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;
-      }
+    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.
+    if (request_info->is_deferred) {
+      request_info->deferred_message_queue.swap(q);
+      return;
     }
   }
 }