Commit d7997ead authored by groby's avatar groby Committed by Commit bot

[MD Settings] Remove jank for webui.

WebUI URLRequest source data is (on desktop) backed by a memory-mapped
file. Copying that data in CompleteRead can trigger jankiness due to
potential file IO caused by the read.

This CL moves the memcpy on the file thread. It is also a re-land of
https://codereview.chromium.org/2158123003/ with minor changes to fix a
crash.

R=dbeam@chromium.org, mmenke@chromium.org, dproy@chromium.org
BUG=455423

Review-Url: https://codereview.chromium.org/2268653002
Cr-Commit-Position: refs/heads/master@{#414269}
parent e8f9faf5
...@@ -23,6 +23,7 @@ ...@@ -23,6 +23,7 @@
#include "base/strings/string_number_conversions.h" #include "base/strings/string_number_conversions.h"
#include "base/strings/string_util.h" #include "base/strings/string_util.h"
#include "base/strings/stringprintf.h" #include "base/strings/stringprintf.h"
#include "base/threading/worker_pool.h"
#include "base/trace_event/trace_event.h" #include "base/trace_event/trace_event.h"
#include "content/browser/blob_storage/chrome_blob_storage_context.h" #include "content/browser/blob_storage/chrome_blob_storage_context.h"
#include "content/browser/histogram_internals_request_job.h" #include "content/browser/histogram_internals_request_job.h"
...@@ -106,6 +107,20 @@ std::string GetOriginHeaderValue(const net::URLRequest* request) { ...@@ -106,6 +107,20 @@ std::string GetOriginHeaderValue(const net::URLRequest* request) {
return result; return result;
} }
// Copy data from source buffer into IO buffer destination.
// TODO(groby): Very similar to URLRequestSimpleJob, unify at some point.
void CopyData(const scoped_refptr<net::IOBuffer>& buf,
int buf_size,
const scoped_refptr<base::RefCountedMemory>& data,
int64_t data_offset) {
// TODO(pkasting): Remove ScopedTracker below once crbug.com/455423 is
// fixed.
tracked_objects::ScopedTracker tracking_profile(
FROM_HERE_WITH_EXPLICIT_FUNCTION(
"455423 URLRequestChromeJob::CompleteRead memcpy"));
memcpy(buf->data(), data->front() + data_offset, buf_size);
}
} // namespace } // namespace
// URLRequestChromeJob is a net::URLRequestJob that manages running // URLRequestChromeJob is a net::URLRequestJob that manages running
...@@ -211,10 +226,9 @@ class URLRequestChromeJob : public net::URLRequestJob { ...@@ -211,10 +226,9 @@ class URLRequestChromeJob : public net::URLRequestJob {
static void DelayStartForDevTools( static void DelayStartForDevTools(
const base::WeakPtr<URLRequestChromeJob>& job); const base::WeakPtr<URLRequestChromeJob>& job);
// Do the actual copy from data_ (the data we're serving) into |buf|. // Post a task to copy |data_| to |buf_| on a worker thread, to avoid browser
// Separate from ReadRawData so we can handle async I/O. Returns the number of // jank. (|data_| might be mem-mapped, so a memcpy can trigger file ops).
// bytes read. int PostReadTask(scoped_refptr<net::IOBuffer> buf, int buf_size);
int CompleteRead(net::IOBuffer* buf, int buf_size);
// The actual data we're serving. NULL until it's been fetched. // The actual data we're serving. NULL until it's been fetched.
scoped_refptr<base::RefCountedMemory> data_; scoped_refptr<base::RefCountedMemory> data_;
...@@ -383,17 +397,22 @@ void URLRequestChromeJob::MimeTypeAvailable(const std::string& mime_type) { ...@@ -383,17 +397,22 @@ void URLRequestChromeJob::MimeTypeAvailable(const std::string& mime_type) {
void URLRequestChromeJob::DataAvailable(base::RefCountedMemory* bytes) { void URLRequestChromeJob::DataAvailable(base::RefCountedMemory* bytes) {
TRACE_EVENT_ASYNC_END0("browser", "DataManager:Request", this); TRACE_EVENT_ASYNC_END0("browser", "DataManager:Request", this);
if (bytes) { DCHECK(!data_);
data_ = bytes;
if (pending_buf_.get()) { // A passed-in nullptr signals an error.
CHECK(pending_buf_->data()); if (!bytes) {
int result = CompleteRead(pending_buf_.get(), pending_buf_size_);
pending_buf_ = nullptr;
ReadRawDataComplete(result);
}
} else {
// The request failed.
ReadRawDataComplete(net::ERR_FAILED); ReadRawDataComplete(net::ERR_FAILED);
return;
}
// All further requests will be satisfied from the passed-in data.
data_ = bytes;
if (pending_buf_) {
int result = PostReadTask(pending_buf_, pending_buf_size_);
pending_buf_ = nullptr;
if (result != net::ERR_IO_PENDING)
ReadRawDataComplete(result);
} }
} }
...@@ -402,32 +421,39 @@ base::WeakPtr<URLRequestChromeJob> URLRequestChromeJob::AsWeakPtr() { ...@@ -402,32 +421,39 @@ base::WeakPtr<URLRequestChromeJob> URLRequestChromeJob::AsWeakPtr() {
} }
int URLRequestChromeJob::ReadRawData(net::IOBuffer* buf, int buf_size) { int URLRequestChromeJob::ReadRawData(net::IOBuffer* buf, int buf_size) {
DCHECK(!pending_buf_.get());
// If data isn't available yet, mark this as asynchronous.
if (!data_.get()) { if (!data_.get()) {
DCHECK(!pending_buf_.get());
CHECK(buf->data());
pending_buf_ = buf; pending_buf_ = buf;
pending_buf_size_ = buf_size; pending_buf_size_ = buf_size;
return net::ERR_IO_PENDING; return net::ERR_IO_PENDING;
} }
// Otherwise, the data is available. return PostReadTask(buf, buf_size);
return CompleteRead(buf, buf_size);
} }
int URLRequestChromeJob::CompleteRead(net::IOBuffer* buf, int buf_size) { int URLRequestChromeJob::PostReadTask(scoped_refptr<net::IOBuffer> buf,
int buf_size) {
DCHECK(buf);
DCHECK(data_);
CHECK(buf->data());
int remaining = data_->size() - data_offset_; int remaining = data_->size() - data_offset_;
if (buf_size > remaining) if (buf_size > remaining)
buf_size = remaining; buf_size = remaining;
if (buf_size > 0) {
// TODO(pkasting): Remove ScopedTracker below once crbug.com/455423 is if (buf_size == 0)
// fixed. return 0;
tracked_objects::ScopedTracker tracking_profile(
FROM_HERE_WITH_EXPLICIT_FUNCTION( base::WorkerPool::GetTaskRunner(false)->PostTaskAndReply(
"455423 URLRequestChromeJob::CompleteRead memcpy")); FROM_HERE, base::Bind(&CopyData, base::RetainedRef(buf), buf_size, data_,
memcpy(buf->data(), data_->front() + data_offset_, buf_size); data_offset_),
data_offset_ += buf_size; base::Bind(&URLRequestChromeJob::ReadRawDataComplete, AsWeakPtr(),
} buf_size));
return buf_size; data_offset_ += buf_size;
return net::ERR_IO_PENDING;
} }
void URLRequestChromeJob::DelayStartForDevTools( void URLRequestChromeJob::DelayStartForDevTools(
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment