Skip to content

Commit

Permalink
Avoid IPC message creation between URLLoaderClientImpl and ResourceDi…
Browse files Browse the repository at this point in the history
…spatcher

Currently we create an IPC message for each URLLoaderClient call for code
maintenanceability. This CL stops that for performance.

BUG=710969

Review-Url: https://codereview.chromium.org/2814013004
Cr-Commit-Position: refs/heads/master@{#464646}
(cherry picked from commit e45a888)

Review-Url: https://codereview.chromium.org/2823803002 .
Cr-Commit-Position: refs/branch-heads/3071@{#11}
Cr-Branched-From: a106f0a-refs/heads/master@{#464641}
  • Loading branch information
yutakahirano committed Apr 17, 2017
1 parent d96d57b commit 11af3a6
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 17 deletions.
62 changes: 48 additions & 14 deletions content/child/url_loader_client_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ void URLLoaderClientImpl::FlushDeferredMessages() {
has_completion_message = true;
break;
}
Dispatch(messages[index]);
resource_dispatcher_->DispatchMessage(messages[index]);
if (!weak_this)
return;
if (is_deferred_) {
Expand Down Expand Up @@ -103,7 +103,8 @@ void URLLoaderClientImpl::FlushDeferredMessages() {
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());
}
}

Expand All @@ -112,28 +113,47 @@ void URLLoaderClientImpl::OnReceiveResponse(
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 uint8_t* data_ptr = reinterpret_cast<const uint8_t*>(data.data());
std::vector<char> data_to_pass(data_ptr, data_ptr + data.size());
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) {
Expand All @@ -158,28 +178,42 @@ void URLLoaderClientImpl::OnStartLoadingResponseBody(
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) {
DCHECK(NeedsStoringMessage());
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();
}
}

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();
}

Expand Down
3 changes: 2 additions & 1 deletion content/child/url_loader_client_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,8 @@ class CONTENT_EXPORT URLLoaderClientImpl final : public mojom::URLLoaderClient {
void OnComplete(const ResourceRequestCompletionStatus& status) override;

private:
void Dispatch(const IPC::Message& message);
bool NeedsStoringMessage() const;
void StoreAndDispatch(const IPC::Message& message);

mojo::Binding<mojom::URLLoaderClient> binding_;
scoped_refptr<URLResponseBodyConsumer> body_consumer_;
Expand Down
3 changes: 1 addition & 2 deletions content/child/url_response_body_consumer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,7 @@ void URLResponseBodyConsumer::NotifyCompletionIfAppropriate() {
// Cancel this instance in order not to notify twice.
Cancel();

resource_dispatcher_->DispatchMessage(
ResourceMsg_RequestComplete(request_id_, completion_status_));
resource_dispatcher_->OnRequestComplete(request_id_, completion_status_);
// |this| may be deleted.
}

Expand Down

0 comments on commit 11af3a6

Please sign in to comment.