Skip to content

Commit

Permalink
Dispatch mojo data pipe to ResourceLoader
Browse files Browse the repository at this point in the history 
In order to deprecate WebDataConsumerHandle, this CL dispatches a mojo
data pipe to ResourceLoader. Further cleanup CLs will follow.

Bug: 894819
Change-Id: Id01b73053555ce0daf06a76f4521270663e40aeb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1460577
Commit-Queue: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#639865}
  • Loading branch information
@yutakahirano
yutakahirano authored and Commit Bot committed Mar 12, 2019
1 parent 6bc21ec commit 38bdba0
Show file tree
Hide file tree
Showing 9 changed files with 77 additions and 147 deletions.
44 changes: 1 addition & 43 deletions content/renderer/loader/web_url_loader_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -886,21 +886,6 @@ void WebURLLoaderImpl::Context::OnReceivedResponse(
WebURLResponse response;
PopulateURLResponse(url_, info, &response, report_raw_headers_, request_id_);

if (use_stream_on_response_) {
auto read_handle = std::make_unique<SharedMemoryDataConsumerHandle>(
base::BindOnce(&Context::CancelBodyStreaming, this),
&body_stream_writer_);

// Here |body_stream_writer_| has an indirect reference to |this| and that
// creates a reference cycle, but it is not a problem because the cycle
// will break if one of the following happens:
// 1) The body data transfer is done (with or without an error).
// 2) |read_handle| (and its reader) is detached.
client_->DidReceiveResponse(response, std::move(read_handle));
// TODO(yhirano): Support ftp listening and multipart
return;
}

client_->DidReceiveResponse(response);

// DidReceiveResponse() may have triggered a cancel, causing the |client_| to
Expand All @@ -911,40 +896,13 @@ void WebURLLoaderImpl::Context::OnReceivedResponse(

void WebURLLoaderImpl::Context::OnStartLoadingResponseBody(
mojo::ScopedDataPipeConsumerHandle body) {
if (!pass_response_pipe_to_client_) {
DCHECK(base::FeatureList::IsEnabled(
blink::features::kResourceLoadViaDataPipe));
body_handle_ = std::move(body);
body_watcher_.Watch(
body_handle_.get(),
MOJO_HANDLE_SIGNAL_READABLE | MOJO_HANDLE_SIGNAL_PEER_CLOSED,
MOJO_TRIGGER_CONDITION_SIGNALS_SATISFIED,
base::BindRepeating(&WebURLLoaderImpl::Context::OnBodyAvailable, this));
if (defers_loading_ == NOT_DEFERRING)
OnBodyAvailable(MOJO_RESULT_OK, {});
return;
}

if (client_)
client_->DidStartLoadingResponseBody(std::move(body));
}

void WebURLLoaderImpl::Context::OnReceivedData(
std::unique_ptr<ReceivedData> data) {
const char* payload = data->payload();
int data_length = data->length();
if (!client_)
return;

TRACE_EVENT_WITH_FLOW0(
"loading", "WebURLLoaderImpl::Context::OnReceivedData",
this, TRACE_EVENT_FLAG_FLOW_IN | TRACE_EVENT_FLAG_FLOW_OUT);

if (use_stream_on_response_) {
body_stream_writer_->AddData(std::move(data));
} else {
client_->DidReceiveData(payload, data_length);
}
NOTREACHED();
}

void WebURLLoaderImpl::Context::OnTransferSizeUpdated(int transfer_size_diff) {
Expand Down
114 changes: 33 additions & 81 deletions content/renderer/loader/web_url_loader_impl_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
#include "base/run_loop.h"
#include "base/single_thread_task_runner.h"
#include "base/stl_util.h"
#include "base/test/scoped_feature_list.h"
#include "base/test/scoped_task_environment.h"
#include "base/time/default_tick_clock.h"
#include "base/time/time.h"
Expand All @@ -41,7 +40,6 @@
#include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h"
#include "services/network/public/mojom/request_context_frame_type.mojom.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/blink/public/common/features.h"
#include "third_party/blink/public/platform/scheduler/test/renderer_scheduler_test_support.h"
#include "third_party/blink/public/platform/web_data.h"
#include "third_party/blink/public/platform/web_string.h"
Expand Down Expand Up @@ -223,17 +221,15 @@ class TestWebURLLoaderClient : public blink::WebURLLoaderClient {
loader_.reset();
}

void DidReceiveData(const char* data, int dataLength) override {
EXPECT_TRUE(loader_);
// The response should have started, but must not have finished, or failed.
EXPECT_TRUE(did_receive_response_);
EXPECT_FALSE(did_finish_);
EXPECT_FALSE(error_);

received_data_.append(data, dataLength);
void DidStartLoadingResponseBody(
mojo::ScopedDataPipeConsumerHandle response_body) override {
DCHECK(!response_body_);
DCHECK(response_body);
response_body_ = std::move(response_body);
}

if (delete_on_receive_data_)
loader_.reset();
void DidReceiveData(const char* data, int dataLength) override {
NOTREACHED();
}

void DidFinishLoading(
Expand Down Expand Up @@ -277,7 +273,7 @@ class TestWebURLLoaderClient : public blink::WebURLLoaderClient {

bool did_receive_redirect() const { return did_receive_redirect_; }
bool did_receive_response() const { return did_receive_response_; }
const std::string& received_data() const { return received_data_; }
bool did_receive_response_body() const { return !!response_body_; }
bool did_finish() const { return did_finish_; }
const base::Optional<blink::WebURLError>& error() const { return error_; }
const blink::WebURLResponse& response() const { return response_; }
Expand All @@ -294,19 +290,17 @@ class TestWebURLLoaderClient : public blink::WebURLLoaderClient {

bool did_receive_redirect_;
bool did_receive_response_;
std::string received_data_;
mojo::ScopedDataPipeConsumerHandle response_body_;
bool did_finish_;
base::Optional<blink::WebURLError> error_;
blink::WebURLResponse response_;

DISALLOW_COPY_AND_ASSIGN(TestWebURLLoaderClient);
};

class WebURLLoaderImplTest : public testing::TestWithParam<bool> {
class WebURLLoaderImplTest : public testing::Test {
public:
WebURLLoaderImplTest() {
scoped_feature_list_.InitWithFeatureState(
blink::features::kResourceLoadViaDataPipe, IsResourceLoadViaDataPipe());
client_.reset(new TestWebURLLoaderClient(&dispatcher_));
}

Expand Down Expand Up @@ -357,37 +351,18 @@ class WebURLLoaderImplTest : public testing::TestWithParam<bool> {
}

void DoStartLoadingResponseBody() {
if (!IsResourceLoadViaDataPipe())
return;
mojo::ScopedDataPipeConsumerHandle handle_to_pass;
MojoResult rv =
mojo::CreateDataPipe(nullptr, &body_handle_, &handle_to_pass);
ASSERT_EQ(MOJO_RESULT_OK, rv);
peer()->OnStartLoadingResponseBody(std::move(handle_to_pass));
}

// Assumes it is called only once for a request.
void DoReceiveData() {
EXPECT_EQ("", client()->received_data());
uint32_t size = strlen(kTestData);
if (IsResourceLoadViaDataPipe()) {
body_handle_->WriteData(kTestData, &size, MOJO_WRITE_DATA_FLAG_NONE);
EXPECT_EQ(strlen(kTestData), size);
base::RunLoop().RunUntilIdle();
} else {
peer()->OnReceivedData(
std::make_unique<FixedReceivedData>(kTestData, size));
}
EXPECT_EQ(kTestData, client()->received_data());
}

void DoCompleteRequest() {
EXPECT_FALSE(client()->did_finish());
if (body_handle_.is_valid()) {
DCHECK(IsResourceLoadViaDataPipe());
body_handle_.reset();
base::RunLoop().RunUntilIdle();
}
DCHECK(body_handle_);
body_handle_.reset();
base::RunLoop().RunUntilIdle();
network::URLLoaderCompletionStatus status(net::OK);
status.encoded_data_length = base::size(kTestData);
status.encoded_body_length = base::size(kTestData);
Expand All @@ -400,11 +375,9 @@ class WebURLLoaderImplTest : public testing::TestWithParam<bool> {

void DoFailRequest() {
EXPECT_FALSE(client()->did_finish());
if (body_handle_.is_valid()) {
DCHECK(IsResourceLoadViaDataPipe());
body_handle_.reset();
base::RunLoop().RunUntilIdle();
}
DCHECK(body_handle_);
body_handle_.reset();
base::RunLoop().RunUntilIdle();
network::URLLoaderCompletionStatus status(net::ERR_FAILED);
status.encoded_data_length = base::size(kTestData);
status.encoded_body_length = base::size(kTestData);
Expand All @@ -419,91 +392,71 @@ class WebURLLoaderImplTest : public testing::TestWithParam<bool> {
TestResourceDispatcher* dispatcher() { return &dispatcher_; }
RequestPeer* peer() { return dispatcher()->peer(); }

static bool IsResourceLoadViaDataPipe() { return GetParam(); }

private:
base::test::ScopedTaskEnvironment task_environment_;
base::test::ScopedFeatureList scoped_feature_list_;
TestResourceDispatcher dispatcher_;
mojo::ScopedDataPipeProducerHandle body_handle_;
std::unique_ptr<TestWebURLLoaderClient> client_;
};

INSTANTIATE_TEST_SUITE_P(WebURLLoaderImplTestP,
WebURLLoaderImplTest,
testing::Bool());

TEST_P(WebURLLoaderImplTest, Success) {
TEST_F(WebURLLoaderImplTest, Success) {
DoStartAsyncRequest();
DoReceiveResponse();
DoStartLoadingResponseBody();
DoReceiveData();
DoCompleteRequest();
EXPECT_FALSE(dispatcher()->canceled());
EXPECT_EQ(kTestData, client()->received_data());
EXPECT_TRUE(client()->did_receive_response_body());
}

TEST_P(WebURLLoaderImplTest, Redirect) {
TEST_F(WebURLLoaderImplTest, Redirect) {
DoStartAsyncRequest();
DoReceiveRedirect();
DoReceiveResponse();
DoStartLoadingResponseBody();
DoReceiveData();
DoCompleteRequest();
EXPECT_FALSE(dispatcher()->canceled());
EXPECT_EQ(kTestData, client()->received_data());
EXPECT_TRUE(client()->did_receive_response_body());
}

TEST_P(WebURLLoaderImplTest, Failure) {
TEST_F(WebURLLoaderImplTest, Failure) {
DoStartAsyncRequest();
DoReceiveResponse();
DoStartLoadingResponseBody();
DoReceiveData();
DoFailRequest();
EXPECT_FALSE(dispatcher()->canceled());
}

// The client may delete the WebURLLoader during any callback from the loader.
// These tests make sure that doesn't result in a crash.
TEST_P(WebURLLoaderImplTest, DeleteOnReceiveRedirect) {
TEST_F(WebURLLoaderImplTest, DeleteOnReceiveRedirect) {
client()->set_delete_on_receive_redirect();
DoStartAsyncRequest();
DoReceiveRedirect();
}

TEST_P(WebURLLoaderImplTest, DeleteOnReceiveResponse) {
TEST_F(WebURLLoaderImplTest, DeleteOnReceiveResponse) {
client()->set_delete_on_receive_response();
DoStartAsyncRequest();
DoReceiveResponse();
}

TEST_P(WebURLLoaderImplTest, DeleteOnReceiveData) {
client()->set_delete_on_receive_data();
DoStartAsyncRequest();
DoReceiveResponse();
DoStartLoadingResponseBody();
DoReceiveData();
}

TEST_P(WebURLLoaderImplTest, DeleteOnFinish) {
TEST_F(WebURLLoaderImplTest, DeleteOnFinish) {
client()->set_delete_on_finish();
DoStartAsyncRequest();
DoReceiveResponse();
DoStartLoadingResponseBody();
DoReceiveData();
DoCompleteRequest();
}

TEST_P(WebURLLoaderImplTest, DeleteOnFail) {
TEST_F(WebURLLoaderImplTest, DeleteOnFail) {
client()->set_delete_on_fail();
DoStartAsyncRequest();
DoReceiveResponse();
DoStartLoadingResponseBody();
DoReceiveData();
DoFailRequest();
}

TEST_P(WebURLLoaderImplTest, DefersLoadingBeforeStart) {
TEST_F(WebURLLoaderImplTest, DefersLoadingBeforeStart) {
client()->loader()->SetDefersLoading(true);
EXPECT_FALSE(dispatcher()->defers_loading());
DoStartAsyncRequest();
Expand All @@ -512,7 +465,7 @@ TEST_P(WebURLLoaderImplTest, DefersLoadingBeforeStart) {

// Checks that the navigation response override parameters provided on
// navigation commit are properly applied.
TEST_P(WebURLLoaderImplTest, BrowserSideNavigationCommit) {
TEST_F(WebURLLoaderImplTest, BrowserSideNavigationCommit) {
// Initialize the request and the stream override.
const GURL kNavigationURL = GURL(kTestURL);
const std::string kMimeType = "text/html";
Expand Down Expand Up @@ -543,13 +496,12 @@ TEST_P(WebURLLoaderImplTest, BrowserSideNavigationCommit) {
EXPECT_EQ(kMimeType, client()->response().MimeType().Latin1());

DoStartLoadingResponseBody();
DoReceiveData();
DoCompleteRequest();
EXPECT_FALSE(dispatcher()->canceled());
EXPECT_EQ(kTestData, client()->received_data());
EXPECT_TRUE(client()->did_receive_response_body());
}

TEST_P(WebURLLoaderImplTest, ResponseIPAddress) {
TEST_F(WebURLLoaderImplTest, ResponseIPAddress) {
GURL url("http://example.test/");

struct TestCase {
Expand Down Expand Up @@ -577,7 +529,7 @@ TEST_P(WebURLLoaderImplTest, ResponseIPAddress) {
};
}

TEST_P(WebURLLoaderImplTest, ResponseCert) {
TEST_F(WebURLLoaderImplTest, ResponseCert) {
GURL url("https://test.example/");

net::CertificateList certs;
Expand Down Expand Up @@ -619,7 +571,7 @@ TEST_P(WebURLLoaderImplTest, ResponseCert) {
security_details.certificate[1]);
}

TEST_P(WebURLLoaderImplTest, ResponseCertWithNoSANs) {
TEST_F(WebURLLoaderImplTest, ResponseCertWithNoSANs) {
GURL url("https://test.example/");

net::CertificateList certs;
Expand Down Expand Up @@ -653,7 +605,7 @@ TEST_P(WebURLLoaderImplTest, ResponseCertWithNoSANs) {

// Verifies that the lengths used by the PerformanceResourceTiming API are
// correctly assigned for sync XHR.
TEST_P(WebURLLoaderImplTest, SyncLengths) {
TEST_F(WebURLLoaderImplTest, SyncLengths) {
static const char kBodyData[] = "Today is Thursday";
const int kEncodedBodyLength = 30;
const int kEncodedDataLength = 130;
Expand Down
2 changes: 1 addition & 1 deletion third_party/blink/renderer/core/html/imports/html_import_loader.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ void HTMLImportLoader::ResponseReceived(Resource* resource,
SetState(StartWritingAndParsing(response));
}

void HTMLImportLoader::DataReceived(Resource*,
void HTMLImportLoader::DataReceived(Resource* resource,
const char* data,
size_t length) {
document_->Parser()->AppendBytes(data, length);
Expand Down
6 changes: 6 additions & 0 deletions third_party/blink/renderer/core/loader/preload_helper.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -513,12 +513,18 @@ Resource* PreloadHelper::StartPreload(ResourceType type,
break;
case ResourceType::kAudio:
case ResourceType::kVideo:
params.MutableResourceRequest().SetUseStreamOnResponse(true);
params.MutableOptions().data_buffering_policy = kDoNotBufferData;
resource = RawResource::FetchMedia(params, resource_fetcher, nullptr);
break;
case ResourceType::kTextTrack:
params.MutableResourceRequest().SetUseStreamOnResponse(true);
params.MutableOptions().data_buffering_policy = kDoNotBufferData;
resource = RawResource::FetchTextTrack(params, resource_fetcher, nullptr);
break;
case ResourceType::kImportResource:
params.MutableResourceRequest().SetUseStreamOnResponse(true);
params.MutableOptions().data_buffering_policy = kDoNotBufferData;
resource = RawResource::FetchImport(params, resource_fetcher, nullptr);
break;
case ResourceType::kRaw:
Expand Down
1 change: 1 addition & 0 deletions third_party/blink/renderer/core/loader/resource/image_resource.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,7 @@ void ImageResource::DecodeError(bool all_data_received) {
if (!all_data_received && Loader()) {
// Observers are notified via ImageResource::finish().
// TODO(hiroshige): Do not call didFinishLoading() directly.
Loader()->AbortResponseBodyLoading();
Loader()->DidFinishLoading(
CurrentTimeTicks(), size, size, size, false,
std::vector<network::cors::PreflightTimingInfo>());
Expand Down
Loading

0 comments on commit 38bdba0

Please sign in to comment.