Skip to content

Commit

Permalink
Revert 276494 "Revert 275655 "Redirect HTML resource bytes direc..."
Browse files Browse the repository at this point in the history 
Relanding with two fixes: Check that Blink hasn't shut down the background (parser) thread before posting tasks to it, and only attach the ThreadedDataProvider if we have a shared memory handle available (fixes a crash when loading FTP listings)

> Revert 275655 "Redirect HTML resource bytes directly to parser t..."
>
> > Redirect HTML resource bytes directly to parser thread (Chrome side)
> >
> > Blink side: https://codereview.chromium.org/100563004/
> >
> > Note: This can't land until the Blink-side has landed.
> >
> > R=jamesr@chromium.org,jam@chromium.org
> > BUG=277886
> >
> > Review URL: https://codereview.chromium.org/109283006
>
> TBR=oysteine@chromium.org
>
> Review URL: https://codereview.chromium.org/326403005

TBR=darin@chromium.org

Review URL: https://codereview.chromium.org/334813004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@277551 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
oysteine@chromium.org committed Jun 16, 2014
1 parent 286b0de commit 079fe6c
Show file tree
Hide file tree
Showing 9 changed files with 485 additions and 26 deletions.
81 changes: 64 additions & 17 deletions content/child/resource_dispatcher.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "content/child/request_info.h"
#include "content/child/site_isolation_policy.h"
#include "content/child/sync_load_response.h"
#include "content/child/threaded_data_provider.h"
#include "content/common/inter_process_time_ticks_converter.h"
#include "content/common/resource_messages.h"
#include "content/public/child/request_peer.h"
Expand Down Expand Up @@ -80,6 +81,8 @@ class IPCResourceLoaderBridge : public ResourceLoaderBridge {
virtual void SetDefersLoading(bool value) OVERRIDE;
virtual void DidChangePriority(net::RequestPriority new_priority,
int intra_priority_value) OVERRIDE;
virtual bool AttachThreadedDataReceiver(
blink::WebThreadedDataReceiver* threaded_data_receiver) OVERRIDE;
virtual void SyncLoad(SyncLoadResponse* response) OVERRIDE;

private:
Expand Down Expand Up @@ -218,6 +221,17 @@ void IPCResourceLoaderBridge::DidChangePriority(
request_id_, new_priority, intra_priority_value);
}

bool IPCResourceLoaderBridge::AttachThreadedDataReceiver(
blink::WebThreadedDataReceiver* threaded_data_receiver) {
if (request_id_ < 0) {
NOTREACHED() << "Trying to attach threaded receiver on unstarted request";
return false;
}

return dispatcher_->AttachThreadedDataReceiver(request_id_,
threaded_data_receiver);
}

void IPCResourceLoaderBridge::SyncLoad(SyncLoadResponse* response) {
if (request_id_ != -1) {
NOTREACHED() << "Starting a request twice";
Expand Down Expand Up @@ -403,6 +417,7 @@ void ResourceDispatcher::OnReceivedData(int request_id,
TRACE_EVENT0("loader", "ResourceDispatcher::OnReceivedData");
DCHECK_GT(data_length, 0);
PendingRequestInfo* request_info = GetPendingRequestInfo(request_id);
bool send_ack = true;
if (request_info && data_length > 0) {
CHECK(base::SharedMemory::IsHandleValid(request_info->buffer->handle()));
CHECK_GE(request_info->buffer_size, data_offset + data_length);
Expand All @@ -414,40 +429,51 @@ void ResourceDispatcher::OnReceivedData(int request_id,

base::TimeTicks time_start = base::TimeTicks::Now();

const char* data_ptr = static_cast<char*>(request_info->buffer->memory());
CHECK(data_ptr);
CHECK(data_ptr + data_offset);
const char* data_start = static_cast<char*>(request_info->buffer->memory());
CHECK(data_start);
CHECK(data_start + data_offset);
const char* data_ptr = data_start + data_offset;

// Check whether this response data is compliant with our cross-site
// document blocking policy. We only do this for the first packet.
std::string alternative_data;
if (request_info->site_isolation_metadata.get()) {
request_info->blocked_response =
SiteIsolationPolicy::ShouldBlockResponse(
request_info->site_isolation_metadata, data_ptr + data_offset,
data_length, &alternative_data);
request_info->site_isolation_metadata, data_ptr, data_length,
&alternative_data);
request_info->site_isolation_metadata.reset();
}

// When the response is not blocked.
if (!request_info->blocked_response) {
request_info->peer->OnReceivedData(
data_ptr + data_offset, data_length, encoded_data_length);
} else if (alternative_data.size() > 0) {
// When the response is blocked, and when we have any alternative data to
// When the response is blocked we may have any alternative data to
// send to the renderer. When |alternative_data| is zero-sized, we do not
// call peer's callback.
request_info->peer->OnReceivedData(alternative_data.data(),
alternative_data.size(),
alternative_data.size());
if (request_info->blocked_response && !alternative_data.empty()) {
data_ptr = alternative_data.data();
data_length = alternative_data.size();
encoded_data_length = alternative_data.size();
}
}

if (!request_info->blocked_response || !alternative_data.empty()) {
if (request_info->threaded_data_provider) {
request_info->threaded_data_provider->OnReceivedDataOnForegroundThread(
data_ptr, data_length, encoded_data_length);
// A threaded data provider will take care of its own ACKing, as the
// data may be processed later on another thread.
send_ack = false;
} else {
request_info->peer->OnReceivedData(
data_ptr, data_length, encoded_data_length);
}
}

UMA_HISTOGRAM_TIMES("ResourceDispatcher.OnReceivedDataTime",
base::TimeTicks::Now() - time_start);
}

// Acknowledge the reception of this data.
message_sender_->Send(new ResourceHostMsg_DataReceived_ACK(request_id));
if (send_ack)
message_sender_->Send(new ResourceHostMsg_DataReceived_ACK(request_id));
}

void ResourceDispatcher::OnDownloadedData(int request_id,
Expand Down Expand Up @@ -620,8 +646,25 @@ void ResourceDispatcher::DidChangePriority(int request_id,
request_id, new_priority, intra_priority_value));
}

bool ResourceDispatcher::AttachThreadedDataReceiver(
int request_id, blink::WebThreadedDataReceiver* threaded_data_receiver) {
PendingRequestInfo* request_info = GetPendingRequestInfo(request_id);
DCHECK(request_info);

if (request_info->buffer != NULL) {
DCHECK(!request_info->threaded_data_provider);
request_info->threaded_data_provider = new ThreadedDataProvider(
request_id, threaded_data_receiver, request_info->buffer,
request_info->buffer_size);
return true;
}

return false;
}

ResourceDispatcher::PendingRequestInfo::PendingRequestInfo()
: peer(NULL),
threaded_data_provider(NULL),
resource_type(ResourceType::SUB_RESOURCE),
is_deferred(false),
download_to_file(false),
Expand All @@ -637,6 +680,7 @@ ResourceDispatcher::PendingRequestInfo::PendingRequestInfo(
const GURL& request_url,
bool download_to_file)
: peer(peer),
threaded_data_provider(NULL),
resource_type(resource_type),
origin_pid(origin_pid),
is_deferred(false),
Expand All @@ -647,7 +691,10 @@ ResourceDispatcher::PendingRequestInfo::PendingRequestInfo(
request_start(base::TimeTicks::Now()),
blocked_response(false) {}

ResourceDispatcher::PendingRequestInfo::~PendingRequestInfo() {}
ResourceDispatcher::PendingRequestInfo::~PendingRequestInfo() {
if (threaded_data_provider)
threaded_data_provider->Stop();
}

void ResourceDispatcher::DispatchMessage(const IPC::Message& message) {
IPC_BEGIN_MESSAGE_MAP(ResourceDispatcher, message)
Expand Down
11 changes: 11 additions & 0 deletions content/child/resource_dispatcher.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,18 @@

struct ResourceMsg_RequestCompleteData;

namespace blink {
class WebThreadedDataReceiver;
}

namespace webkit_glue {
class ResourceLoaderBridge;
}

namespace content {
class RequestPeer;
class ResourceDispatcherDelegate;
class ThreadedDataProvider;
struct ResourceResponseInfo;
struct RequestInfo;
struct ResourceResponseHead;
Expand Down Expand Up @@ -77,6 +82,11 @@ class CONTENT_EXPORT ResourceDispatcher : public IPC::Listener {
net::RequestPriority new_priority,
int intra_priority_value);

// The provided data receiver will receive incoming resource data rather
// than the resource bridge.
bool AttachThreadedDataReceiver(
int request_id, blink::WebThreadedDataReceiver* threaded_data_receiver);

IPC::Sender* message_sender() const { return message_sender_; }

// This does not take ownership of the delegate. It is expected that the
Expand Down Expand Up @@ -107,6 +117,7 @@ class CONTENT_EXPORT ResourceDispatcher : public IPC::Listener {
~PendingRequestInfo();

RequestPeer* peer;
ThreadedDataProvider* threaded_data_provider;
ResourceType::Type resource_type;
// The PID of the original process which issued this request. This gets
// non-zero only for a request proxied by another renderer, particularly
Expand Down
Loading

0 comments on commit 079fe6c

Please sign in to comment.