Avoid IPC message creation between URLLoaderClientImpl and ResourceDispatcher

content/child/url_loader_client_impl.cc

 // Copyright 2016 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.
 
 #include "content/child/url_loader_client_impl.h"
 
 #include "base/callback.h"
 #include "base/single_thread_task_runner.h"
 #include "content/child/resource_dispatcher.h"
 #include "content/child/url_response_body_consumer.h"
@@ skipping 42 matching lines @@
   //  - transfer size change (dispatched later)
   //  - completion (dispatched by |body_consumer_| or dispatched later)
   for (size_t index = 0; index < messages.size(); ++index) {
     if (messages[index].type() == ResourceMsg_RequestComplete::ID) {
       // The completion message arrives at the end of the message queue.
       DCHECK(!has_completion_message);
       DCHECK_EQ(index, messages.size() - 1);
       has_completion_message = true;
       break;
     }
-    Dispatch(messages[index]);
+    resource_dispatcher_->DispatchMessage(messages[index]);
     if (!weak_this)
       return;
     if (is_deferred_) {
       deferred_messages_.insert(deferred_messages_.begin(),
                                 messages.begin() + index + 1, messages.end());
       return;
     }
   }
 
   // Dispatch the transfer size update.
@@ skipping 22 matching lines @@
     // Dispatch the response body.
     body_consumer_->UnsetDefersLoading();
     return;
   }
 
   // Dispatch the completion message.
   if (has_completion_message) {
     DCHECK_GT(messages.size(), 0u);
     DCHECK_EQ(messages.back().type(),
               static_cast<uint32_t>(ResourceMsg_RequestComplete::ID));
-    Dispatch(messages.back());
+
+    resource_dispatcher_->DispatchMessage(messages.back());
   }
 }
 
 void URLLoaderClientImpl::OnReceiveResponse(
     const ResourceResponseHead& response_head,
     mojom::DownloadedTempFilePtr downloaded_file) {
   has_received_response_ = true;
   downloaded_file_ = std::move(downloaded_file);
-  Dispatch(ResourceMsg_ReceivedResponse(request_id_, response_head));
+  if (NeedsStoringMessage())
+    StoreAndDispatch(ResourceMsg_ReceivedResponse(request_id_, response_head));
+  else
+    resource_dispatcher_->OnReceivedResponse(request_id_, response_head);
 }
 
 void URLLoaderClientImpl::OnReceiveRedirect(
     const net::RedirectInfo& redirect_info,
     const ResourceResponseHead& response_head) {
   DCHECK(!has_received_response_);
   DCHECK(!body_consumer_);
-  Dispatch(
-      ResourceMsg_ReceivedRedirect(request_id_, redirect_info, response_head));
+  if (NeedsStoringMessage()) {
+    StoreAndDispatch(ResourceMsg_ReceivedRedirect(request_id_, redirect_info,
+                                                  response_head));
+  } else {
+    resource_dispatcher_->OnReceivedRedirect(request_id_, redirect_info,
+                                             response_head);
+  }
 }
 
 void URLLoaderClientImpl::OnDataDownloaded(int64_t data_len,
                                            int64_t encoded_data_len) {
-  Dispatch(ResourceMsg_DataDownloaded(request_id_, data_len, encoded_data_len));
+  if (NeedsStoringMessage()) {
+    StoreAndDispatch(
+        ResourceMsg_DataDownloaded(request_id_, data_len, encoded_data_len));
+  } else {
+    resource_dispatcher_->OnDownloadedData(request_id_, data_len,
+                                           encoded_data_len);
+  }
 }
 
 void URLLoaderClientImpl::OnReceiveCachedMetadata(
     const std::vector<uint8_t>& data) {
-  const char* data_ptr = reinterpret_cast<const char*>(data.data());
-  Dispatch(ResourceMsg_ReceivedCachedMetadata(
-      request_id_, std::vector<char>(data_ptr, data_ptr + data.size())));
+  const std::vector<char>& data_to_pass =
+      reinterpret_cast<const std::vector<char>&>(data);

[yhirano, 2017/04/12 13:29:27]

tzik@, is this OK?

[tzik, 2017/04/13 01:55:49]

Oh... Can we avoid it by modifying the IPC?

[yhirano, 2017/04/13 14:16:26]

We can, but it needs more work.
OK, I removed this controversial change from this CL.

+  if (NeedsStoringMessage()) {
+    StoreAndDispatch(
+        ResourceMsg_ReceivedCachedMetadata(request_id_, data_to_pass));
+  } else {
+    resource_dispatcher_->OnReceivedCachedMetadata(request_id_, data_to_pass);
+  }
 }
 
 void URLLoaderClientImpl::OnTransferSizeUpdated(int32_t transfer_size_diff) {
   if (is_deferred_) {
     accumulated_transfer_size_diff_during_deferred_ += transfer_size_diff;
   } else {
     resource_dispatcher_->OnTransferSizeUpdated(request_id_,
                                                 transfer_size_diff);
   }
 }
 
 void URLLoaderClientImpl::OnStartLoadingResponseBody(
     mojo::ScopedDataPipeConsumerHandle body) {
   DCHECK(!body_consumer_);
   DCHECK(has_received_response_);
   body_consumer_ = new URLResponseBodyConsumer(
       request_id_, resource_dispatcher_, std::move(body), task_runner_);
   if (is_deferred_)
     body_consumer_->SetDefersLoading();
 }
 
 void URLLoaderClientImpl::OnComplete(
     const ResourceRequestCompletionStatus& status) {
   if (!body_consumer_) {
-    Dispatch(ResourceMsg_RequestComplete(request_id_, status));
+    if (NeedsStoringMessage()) {
+      StoreAndDispatch(ResourceMsg_RequestComplete(request_id_, status));
+    } else {
+      resource_dispatcher_->OnRequestComplete(request_id_, status);
+    }
     return;
   }
   body_consumer_->OnComplete(status);
 }
 
-void URLLoaderClientImpl::Dispatch(const IPC::Message& message) {
+bool URLLoaderClientImpl::NeedsStoringMessage() const {
+  return is_deferred_ || deferred_messages_.size() > 0;
+}
+
+void URLLoaderClientImpl::StoreAndDispatch(const IPC::Message& message) {
   if (is_deferred_) {
     deferred_messages_.push_back(message);
   } else if (deferred_messages_.size() > 0) {
     deferred_messages_.push_back(message);
     FlushDeferredMessages();
   } else {
-    resource_dispatcher_->DispatchMessage(message);
+    NOTREACHED();

[tzik, 2017/04/13 01:55:49]

Can we DCHECK(is_deferred_ || !deferred_message_.empty()); around line 196?

[yhirano, 2017/04/13 14:16:26]

Done.

   }
 }
 
 void URLLoaderClientImpl::OnUploadProgress(int64_t current_position,
                                            int64_t total_size,
                                            const base::Closure& ack_callback) {
-  Dispatch(
-      ResourceMsg_UploadProgress(request_id_, current_position, total_size));
+  if (NeedsStoringMessage()) {
+    StoreAndDispatch(
+        ResourceMsg_UploadProgress(request_id_, current_position, total_size));
+  } else {
+    resource_dispatcher_->OnUploadProgress(request_id_, current_position,
+                                           total_size);
+  }
   ack_callback.Run();
 }
 
 }  // namespace content