Skip to content

Commit

Permalink
Remove WebURLLoaderImpl::SinkPeer
Browse files Browse the repository at this point in the history
As we've seen no regression, let's remove SinkPeer.

Bug: 1112310
Change-Id: I88ddcf8d3e5d8ffb839144a0ea6eb3d122e02e9d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2462995
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Reviewed-by: Dominic Farolino <dom@chromium.org>
Commit-Queue: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#817817}
  • Loading branch information
yutakahirano authored and Commit Bot committed Oct 16, 2020
1 parent c205c7f commit f02b5e0
Show file tree
Hide file tree
Showing 20 changed files with 19 additions and 147 deletions.
20 changes: 6 additions & 14 deletions chrome/browser/prerender/prerender_nostate_prefetch_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -558,24 +558,20 @@ class NoStatePrefetchBrowserTest

class NoStatePrefetchBrowserTestHttpCache
: public NoStatePrefetchBrowserTest,
public testing::WithParamInterface<std::tuple<bool, bool, bool>> {
public testing::WithParamInterface<std::tuple<bool, bool>> {
protected:
void SetUp() override {
bool split_cache_by_network_isolation_key;
bool append_frame_origin_to_network_isolation_key;
bool use_prefetch_loader;
std::tie(split_cache_by_network_isolation_key,
append_frame_origin_to_network_isolation_key,
use_prefetch_loader) = GetParam();
append_frame_origin_to_network_isolation_key) = GetParam();

std::vector<base::Feature> disabled_and_enabled_features[2];

disabled_and_enabled_features[split_cache_by_network_isolation_key]
.push_back(net::features::kSplitCacheByNetworkIsolationKey);
disabled_and_enabled_features[append_frame_origin_to_network_isolation_key]
.push_back(net::features::kAppendFrameOriginToNetworkIsolationKey);
disabled_and_enabled_features[use_prefetch_loader].push_back(
features::kNoStatePrefetchUsingPrefetchLoader);

feature_list_.InitWithFeatures(disabled_and_enabled_features[true],
disabled_and_enabled_features[false]);
Expand All @@ -596,8 +592,8 @@ IN_PROC_BROWSER_TEST_P(
NoStatePrefetchBrowserTestHttpCache_DefaultAndAppendFrameOrigin,
PrefetchTwoCrossOriginFrames) {
bool append_frame_origin_to_network_isolation_key;
std::tie(std::ignore, append_frame_origin_to_network_isolation_key,
std::ignore) = GetParam();
std::tie(std::ignore, append_frame_origin_to_network_isolation_key) =
GetParam();

GURL image_src =
embedded_test_server()->GetURL("/prerender/cacheable_image.png");
Expand Down Expand Up @@ -627,9 +623,7 @@ IN_PROC_BROWSER_TEST_P(
INSTANTIATE_TEST_SUITE_P(
All,
NoStatePrefetchBrowserTestHttpCache_DefaultAndAppendFrameOrigin,
::testing::Combine(::testing::Values(true),
::testing::Bool(),
::testing::Bool()));
::testing::Combine(::testing::Values(true), ::testing::Bool()));

// Checks that a page is correctly prefetched in the case of a
// <link rel=prerender> tag and the JavaScript on the page is not executed.
Expand Down Expand Up @@ -708,9 +702,7 @@ IN_PROC_BROWSER_TEST_P(
INSTANTIATE_TEST_SUITE_P(
All,
NoStatePrefetchBrowserTestHttpCache_DefaultAndDoubleKeyedHttpCache,
::testing::Combine(::testing::Bool(),
::testing::Values(false),
::testing::Bool()));
::testing::Combine(::testing::Bool(), ::testing::Values(false)));

// Checks that the expected resource types are fetched via NoState Prefetch.
IN_PROC_BROWSER_TEST_F(NoStatePrefetchBrowserTest, PrefetchAllResourceTypes) {
Expand Down
6 changes: 0 additions & 6 deletions content/public/common/content_features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -409,12 +409,6 @@ const base::Feature kNetworkServiceInProcess {
const base::Feature kNeverSlowMode{"NeverSlowMode",
base::FEATURE_DISABLED_BY_DEFAULT};

// A kill switch for the change for more code sharing between usual prefetch
// and no-state prefetch.
// TODO(yhirano): Remove this if M86 stable looks good.
const base::Feature kNoStatePrefetchUsingPrefetchLoader{
"NoStatePrefetchUsingPrefetchLoader", base::FEATURE_ENABLED_BY_DEFAULT};

// Kill switch for Web Notification content images.
const base::Feature kNotificationContentImage{"NotificationContentImage",
base::FEATURE_ENABLED_BY_DEFAULT};
Expand Down
1 change: 0 additions & 1 deletion content/public/common/content_features.h
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,6 @@ CONTENT_EXPORT extern const base::Feature kMouseSubframeNoImplicitCapture;
CONTENT_EXPORT extern const base::Feature kNetworkQualityEstimatorWebHoldback;
CONTENT_EXPORT extern const base::Feature kNetworkServiceInProcess;
CONTENT_EXPORT extern const base::Feature kNeverSlowMode;
CONTENT_EXPORT extern const base::Feature kNoStatePrefetchUsingPrefetchLoader;
CONTENT_EXPORT extern const base::Feature kNotificationContentImage;
CONTENT_EXPORT extern const base::Feature kNotificationTriggers;
CONTENT_EXPORT extern const base::Feature kOriginIsolationHeader;
Expand Down
2 changes: 0 additions & 2 deletions content/public/test/render_view_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,6 @@ class FakeWebURLLoader : public blink::WebURLLoader {
std::unique_ptr<network::ResourceRequest> request,
scoped_refptr<WebURLRequest::ExtraData> request_extra_data,
int requestor_id,
bool download_to_network_cache_only,
bool pass_response_pipe_to_client,
bool no_mime_sniffing,
base::TimeDelta timeout_interval,
Expand All @@ -149,7 +148,6 @@ class FakeWebURLLoader : public blink::WebURLLoader {
std::unique_ptr<network::ResourceRequest> request,
scoped_refptr<WebURLRequest::ExtraData> request_extra_data,
int requestor_id,
bool download_to_network_cache_only,
bool no_mime_sniffing,
std::unique_ptr<blink::ResourceLoadInfoNotifierWrapper>,
blink::WebURLLoaderClient* client) override {
Expand Down
4 changes: 1 addition & 3 deletions content/renderer/loader/child_url_loader_factory_bundle.cc
Original file line number Diff line number Diff line change
Expand Up @@ -303,9 +303,7 @@ void ChildURLLoaderFactoryBundle::CreateLoaderAndStart(
std::move(client), traffic_annotation);
return;
}
if (base::FeatureList::IsEnabled(
features::kNoStatePrefetchUsingPrefetchLoader) &&
(request.load_flags & net::LOAD_PREFETCH) && prefetch_loader_factory_) {
if ((request.load_flags & net::LOAD_PREFETCH) && prefetch_loader_factory_) {
// This is no-state prefetch (see
// WebURLRequest::GetLoadFlagsForWebUrlRequest).
prefetch_loader_factory_->CreateLoaderAndStart(
Expand Down
86 changes: 3 additions & 83 deletions content/renderer/loader/web_url_loader_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,6 @@ class WebURLLoaderImpl::Context : public base::RefCounted<Context> {
void Start(std::unique_ptr<network::ResourceRequest> request,
scoped_refptr<blink::WebURLRequest::ExtraData> request_extra_data,
int requestor_id,
bool download_to_network_cache_only,
bool pass_response_pipe_to_client,
bool no_mime_sniffing,
base::TimeDelta timeout_interval,
Expand Down Expand Up @@ -478,74 +477,6 @@ class WebURLLoaderImpl::RequestPeerImpl : public RequestPeer {
DISALLOW_COPY_AND_ASSIGN(RequestPeerImpl);
};

// A sink peer that doesn't forward the data.
class WebURLLoaderImpl::SinkPeer : public RequestPeer {
public:
explicit SinkPeer(Context* context)
: context_(context),
body_watcher_(FROM_HERE,
mojo::SimpleWatcher::ArmingPolicy::AUTOMATIC,
context->task_runner()) {}

// RequestPeer implementation:
void OnUploadProgress(uint64_t position, uint64_t size) override {}
bool OnReceivedRedirect(const net::RedirectInfo& redirect_info,
network::mojom::URLResponseHeadPtr head,
std::vector<std::string>*) override {
return true;
}
void OnReceivedResponse(network::mojom::URLResponseHeadPtr head) override {}
void OnStartLoadingResponseBody(
mojo::ScopedDataPipeConsumerHandle body) override {
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(&SinkPeer::OnBodyAvailable,
base::Unretained(this)));
}
void OnTransferSizeUpdated(int transfer_size_diff) override {}
void OnReceivedCachedMetadata(mojo_base::BigBuffer) override {}
void OnCompletedRequest(
const network::URLLoaderCompletionStatus& status) override {
body_handle_.reset();
body_watcher_.Cancel();
context_->resource_dispatcher()->Cancel(context_->request_id(),
context_->task_runner());
}
scoped_refptr<base::TaskRunner> GetTaskRunner() override {
return context_->task_runner();
}

private:
void OnBodyAvailable(MojoResult, const mojo::HandleSignalsState&) {
while (true) {
const void* buffer = nullptr;
uint32_t available = 0;
MojoResult rv = body_handle_->BeginReadData(&buffer, &available,
MOJO_READ_DATA_FLAG_NONE);
if (rv == MOJO_RESULT_SHOULD_WAIT) {
return;
}
if (rv != MOJO_RESULT_OK) {
break;
}
rv = body_handle_->EndReadData(available);
if (rv != MOJO_RESULT_OK) {
break;
}
}
body_handle_.reset();
body_watcher_.Cancel();
}

scoped_refptr<Context> context_;
mojo::ScopedDataPipeConsumerHandle body_handle_;
mojo::SimpleWatcher body_watcher_;
DISALLOW_COPY_AND_ASSIGN(SinkPeer);
};

// WebURLLoaderImpl::Context --------------------------------------------------

// static
Expand Down Expand Up @@ -609,7 +540,6 @@ void WebURLLoaderImpl::Context::Start(
std::unique_ptr<network::ResourceRequest> request,
scoped_refptr<blink::WebURLRequest::ExtraData> passed_extra_data,
int requestor_id,
bool download_to_network_cache_only,
bool pass_response_pipe_to_client,
bool no_mime_sniffing,
base::TimeDelta timeout_interval,
Expand Down Expand Up @@ -655,14 +585,7 @@ void WebURLLoaderImpl::Context::Start(
}
extra_data->CopyToResourceRequest(request.get());

std::unique_ptr<RequestPeer> peer;
if (download_to_network_cache_only &&
!base::FeatureList::IsEnabled(
features::kNoStatePrefetchUsingPrefetchLoader)) {
peer = std::make_unique<SinkPeer>(this);
} else {
peer = std::make_unique<WebURLLoaderImpl::RequestPeerImpl>(this);
}
auto peer = std::make_unique<WebURLLoaderImpl::RequestPeerImpl>(this);

if (resource_type == blink::mojom::ResourceType::kPrefetch) {
request->corb_detachable = true;
Expand Down Expand Up @@ -1079,7 +1002,6 @@ void WebURLLoaderImpl::LoadSynchronously(
std::unique_ptr<network::ResourceRequest> request,
scoped_refptr<blink::WebURLRequest::ExtraData> request_extra_data,
int requestor_id,
bool download_to_network_cache_only,
bool pass_response_pipe_to_client,
bool no_mime_sniffing,
base::TimeDelta timeout_interval,
Expand All @@ -1100,8 +1022,7 @@ void WebURLLoaderImpl::LoadSynchronously(

const bool report_raw_headers = request->report_raw_headers;
context_->Start(std::move(request), std::move(request_extra_data),
requestor_id, download_to_network_cache_only,
pass_response_pipe_to_client, no_mime_sniffing,
requestor_id, pass_response_pipe_to_client, no_mime_sniffing,
timeout_interval, &sync_load_response,
std::move(resource_load_info_notifier_wrapper));

Expand Down Expand Up @@ -1148,7 +1069,6 @@ void WebURLLoaderImpl::LoadAsynchronously(
std::unique_ptr<network::ResourceRequest> request,
scoped_refptr<blink::WebURLRequest::ExtraData> request_extra_data,
int requestor_id,
bool download_to_network_cache_only,
bool no_mime_sniffing,
std::unique_ptr<blink::ResourceLoadInfoNotifierWrapper>
resource_load_info_notifier_wrapper,
Expand All @@ -1159,7 +1079,7 @@ void WebURLLoaderImpl::LoadAsynchronously(

context_->set_client(client);
context_->Start(std::move(request), std::move(request_extra_data),
requestor_id, download_to_network_cache_only,
requestor_id,
/*pass_response_pipe_to_client=*/false, no_mime_sniffing,
base::TimeDelta(), nullptr,
std::move(resource_load_info_notifier_wrapper));
Expand Down
3 changes: 0 additions & 3 deletions content/renderer/loader/web_url_loader_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ class CONTENT_EXPORT WebURLLoaderImpl : public blink::WebURLLoader {
std::unique_ptr<network::ResourceRequest> request,
scoped_refptr<blink::WebURLRequest::ExtraData> request_extra_data,
int requestor_id,
bool download_to_network_cache_only,
bool pass_response_pipe_to_client,
bool no_mime_sniffing,
base::TimeDelta timeout_interval,
Expand All @@ -91,7 +90,6 @@ class CONTENT_EXPORT WebURLLoaderImpl : public blink::WebURLLoader {
std::unique_ptr<network::ResourceRequest> request,
scoped_refptr<blink::WebURLRequest::ExtraData> request_extra_data,
int requestor_id,
bool download_to_network_cache_only,
bool no_mime_sniffing,
std::unique_ptr<blink::ResourceLoadInfoNotifierWrapper>
resource_load_info_notifier_wrapper,
Expand All @@ -104,7 +102,6 @@ class CONTENT_EXPORT WebURLLoaderImpl : public blink::WebURLLoader {
private:
class Context;
class RequestPeerImpl;
class SinkPeer;

void Cancel();

Expand Down
3 changes: 1 addition & 2 deletions content/renderer/loader/web_url_loader_impl_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ class WebURLLoaderImplTest : public testing::Test {
request->priority = net::IDLE;
client()->loader()->LoadAsynchronously(
std::move(request), /*extra_data=*/nullptr, /*requestor_id=*/0,
/*download_to_network_cache_only=*/false, /*no_mime_sniffing=*/false,
/*no_mime_sniffing=*/false,
std::make_unique<blink::ResourceLoadInfoNotifierWrapper>(
/*resource_load_info_notifier=*/nullptr),
client());
Expand Down Expand Up @@ -668,7 +668,6 @@ TEST_F(WebURLLoaderImplTest, SyncLengths) {

client()->loader()->LoadSynchronously(
std::move(request), /*extra_data=*/nullptr, /*requestor_id=*/0,
/*download_to_network_cache_only=*/false,
/*pass_response_pipe_to_client=*/false, /*no_mime_sniffing=*/false,
base::TimeDelta(), nullptr, response, error, data, encoded_data_length,
encoded_body_length, downloaded_blob,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ class InternetDisconnectedWebURLLoader final : public WebURLLoader {
std::unique_ptr<network::ResourceRequest> request,
scoped_refptr<WebURLRequest::ExtraData> request_extra_data,
int requestor_id,
bool download_to_network_cache_only,
bool pass_response_pipe_to_client,
bool no_mime_sniffing,
base::TimeDelta timeout_interval,
Expand All @@ -56,7 +55,6 @@ class InternetDisconnectedWebURLLoader final : public WebURLLoader {
std::unique_ptr<network::ResourceRequest> request,
scoped_refptr<WebURLRequest::ExtraData> request_extra_data,
int requestor_id,
bool download_to_network_cache_only,
bool no_mime_sniffing,
std::unique_ptr<blink::ResourceLoadInfoNotifierWrapper>
resource_load_info_notifier_wrapper,
Expand Down
2 changes: 0 additions & 2 deletions third_party/blink/public/platform/web_url_loader.h
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ class WebURLLoader {
std::unique_ptr<network::ResourceRequest> request,
scoped_refptr<WebURLRequest::ExtraData> request_extra_data,
int requestor_id,
bool download_to_network_cache_only,
bool pass_response_pipe_to_client,
bool no_mime_sniffing,
base::TimeDelta timeout_interval,
Expand All @@ -90,7 +89,6 @@ class WebURLLoader {
std::unique_ptr<network::ResourceRequest> request,
scoped_refptr<WebURLRequest::ExtraData> request_extra_data,
int requestor_id,
bool download_to_network_cache_only,
bool no_mime_sniffing,
std::unique_ptr<ResourceLoadInfoNotifierWrapper>,
WebURLLoaderClient*) = 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ class NoopLoaderFactory final : public ResourceFetcher::LoaderFactory {
std::unique_ptr<network::ResourceRequest> request,
scoped_refptr<WebURLRequest::ExtraData> request_extra_data,
int requestor_id,
bool download_to_network_cache_only,
bool pass_response_pipe_to_client,
bool no_mime_sniffing,
base::TimeDelta timeout_interval,
Expand All @@ -102,7 +101,6 @@ class NoopLoaderFactory final : public ResourceFetcher::LoaderFactory {
std::unique_ptr<network::ResourceRequest> request,
scoped_refptr<WebURLRequest::ExtraData> request_extra_data,
int requestor_id,
bool download_to_network_cache_only,
bool no_mime_sniffing,
std::unique_ptr<blink::ResourceLoadInfoNotifierWrapper>
resource_load_info_notifier_wrapper,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ class PrefetchedSignedExchangeManager::PrefetchedSignedExchangeLoader
std::unique_ptr<network::ResourceRequest> request,
scoped_refptr<WebURLRequest::ExtraData> request_extra_data,
int requestor_id,
bool download_to_network_cache_only,
bool pass_response_pipe_to_client,
bool no_mime_sniffing,
base::TimeDelta timeout_interval,
Expand All @@ -88,16 +87,15 @@ class PrefetchedSignedExchangeManager::PrefetchedSignedExchangeLoader
std::unique_ptr<network::ResourceRequest> request,
scoped_refptr<WebURLRequest::ExtraData> request_extra_data,
int requestor_id,
bool download_to_network_cache_only,
bool no_mime_sniffing,
std::unique_ptr<blink::ResourceLoadInfoNotifierWrapper>
resource_load_info_notifier_wrapper,
WebURLLoaderClient* client) override {
if (url_loader_) {
url_loader_->LoadAsynchronously(
std::move(request), std::move(request_extra_data), requestor_id,
download_to_network_cache_only, no_mime_sniffing,
std::move(resource_load_info_notifier_wrapper), client);
no_mime_sniffing, std::move(resource_load_info_notifier_wrapper),
client);
return;
}
// It is safe to use Unretained(client), because |client| is a
Expand All @@ -106,8 +104,7 @@ class PrefetchedSignedExchangeManager::PrefetchedSignedExchangeLoader
pending_method_calls_.push(WTF::Bind(
&PrefetchedSignedExchangeLoader::LoadAsynchronously, GetWeakPtr(),
std::move(request), std::move(request_extra_data), requestor_id,
download_to_network_cache_only, no_mime_sniffing,
std::move(resource_load_info_notifier_wrapper),
no_mime_sniffing, std::move(resource_load_info_notifier_wrapper),
WTF::Unretained(client)));
}
void SetDefersLoading(bool value) override {
Expand Down
2 changes: 0 additions & 2 deletions third_party/blink/renderer/core/svg/graphics/svg_image.cc
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,6 @@ class FailingLoader final : public WebURLLoader {
std::unique_ptr<network::ResourceRequest> request,
scoped_refptr<WebURLRequest::ExtraData> request_extra_data,
int requestor_id,
bool download_to_network_cache_only,
bool pass_response_pipe_to_client,
bool no_mime_sniffing,
base::TimeDelta timeout_interval,
Expand All @@ -109,7 +108,6 @@ class FailingLoader final : public WebURLLoader {
std::unique_ptr<network::ResourceRequest> request,
scoped_refptr<WebURLRequest::ExtraData> request_extra_data,
int requestor_id,
bool download_to_network_cache_only,
bool no_mime_sniffing,
std::unique_ptr<blink::ResourceLoadInfoNotifierWrapper>
resource_load_info_notifier_wrapper,
Expand Down
Loading

0 comments on commit f02b5e0

Please sign in to comment.