Skip to content

Commit

Permalink
[Merge 115] Add a high priority queue for sync cookie calls in the ne…
Browse files Browse the repository at this point in the history
…twork service

This change adds a minimal scheduler for the network service thread
which has a default task queue and high priority task queue. The
RestrictedCookieManager interface will be bound using the high priority
task queue, which should make the sync cookie accesses/writes from the
renderer faster if there are other tasks that need to be run in the
network service.

The hope is that re-ordering these tasks with other network tasks will
not be a problem since they are generally called as sync calls from the
renderer. If there turns out to be problems with this later we can
revisit how tasks are prioritized.

(cherry picked from commit a922565)

Bug: 1448685
Change-Id: I42f7f4e895a0b7d8708c49db707aab6286ef18bf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4561626
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Reviewed-by: Scott Haseley <shaseley@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1149519}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4573725
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Auto-Submit: Clark DuVall <cduvall@chromium.org>
Cr-Commit-Position: refs/branch-heads/5790@{#133}
Cr-Branched-From: 1d71a33-refs/heads/main@{#1148114}
  • Loading branch information
clarkduvall authored and Chromium LUCI CQ committed May 30, 2023
1 parent 803348b commit b8bf3ae
Show file tree
Hide file tree
Showing 10 changed files with 178 additions and 7 deletions.
25 changes: 21 additions & 4 deletions content/browser/network_service_instance_impl.cc
Expand Up @@ -61,6 +61,7 @@
#include "services/network/network_service.h"
#include "services/network/public/cpp/features.h"
#include "services/network/public/cpp/network_switches.h"
#include "services/network/public/cpp/thread_delegate.h"
#include "services/network/public/mojom/net_log.mojom.h"
#include "services/network/public/mojom/network_change_manager.mojom.h"
#include "services/network/public/mojom/network_context.mojom.h"
Expand Down Expand Up @@ -137,6 +138,13 @@ std::unique_ptr<network::NetworkService>& GetLocalNetworkService() {
return service.GetOrCreateValue();
}

// If this is enabled, RestrictedCookieManagers in the network service will
// receive messages on a high priority task queue to improve performance of sync
// cookie calls from the renderer.
BASE_FEATURE(kNetworkServiceCookiesHighPriorityTaskRunner,
"NetworkServiceCookiesHighPriorityTaskRunner",
base::FEATURE_DISABLED_BY_DEFAULT);

// If this feature is enabled, the Network Service will run on its own thread
// when running in-process; otherwise it will run on the IO thread.
//
Expand Down Expand Up @@ -362,6 +370,11 @@ void CreateInProcessNetworkService(
scoped_refptr<base::SingleThreadTaskRunner> task_runner;
if (base::FeatureList::IsEnabled(kNetworkServiceDedicatedThread)) {
base::Thread::Options options(base::MessagePumpType::IO, 0);
if (base::FeatureList::IsEnabled(
kNetworkServiceCookiesHighPriorityTaskRunner)) {
options.delegate =
std::make_unique<network::ThreadDelegate>(options.message_pump_type);
}
GetNetworkServiceDedicatedThread().StartWithOptions(std::move(options));
task_runner = GetNetworkServiceDedicatedThread().task_runner();
} else {
Expand Down Expand Up @@ -579,10 +592,14 @@ network::mojom::NetworkService* GetNetworkService() {
} else {
if (service_was_bound)
LOG(ERROR) << "Network service crashed, restarting service.";
ServiceProcessHost::Launch(std::move(receiver),
ServiceProcessHost::Options()
.WithDisplayName(u"Network Service")
.Pass());
ServiceProcessHost::Options options;
options.WithDisplayName(u"Network Service");
if (base::FeatureList::IsEnabled(
kNetworkServiceCookiesHighPriorityTaskRunner)) {
options.WithExtraCommandLineSwitches(
{network::switches::kNetworkServiceScheduler});
}
ServiceProcessHost::Launch(std::move(receiver), std::move(options));
}
} else {
DCHECK(IsInProcessNetworkService())
Expand Down
8 changes: 7 additions & 1 deletion content/child/child_process.cc
Expand Up @@ -24,6 +24,8 @@
#include "content/public/common/content_switches.h"
#include "mojo/public/cpp/system/dynamic_library_support.h"
#include "sandbox/policy/sandbox_type.h"
#include "services/network/public/cpp/network_switches.h"
#include "services/network/public/cpp/thread_delegate.h"
#include "services/tracing/public/cpp/trace_startup.h"
#include "third_party/abseil-cpp/absl/base/attributes.h"
#include "third_party/blink/public/common/features.h"
Expand Down Expand Up @@ -63,9 +65,9 @@ ChildProcess::ChildProcess(base::ThreadType io_thread_type,
thread_pool_init_params)
: resetter_(&child_process, this, nullptr),
io_thread_(std::make_unique<ChildIOThread>()) {
#if BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_CHROMEOS)
const base::CommandLine& command_line =
*base::CommandLine::ForCurrentProcess();
#if BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_CHROMEOS)
const bool is_embedded_in_browser_process =
!command_line.HasSwitch(switches::kProcessType);
if (IsMojoCoreSharedLibraryEnabled() && !is_embedded_in_browser_process) {
Expand Down Expand Up @@ -137,6 +139,10 @@ ChildProcess::ChildProcess(base::ThreadType io_thread_type,
// of process.
thread_options.thread_type = base::ThreadType::kCompositing;
#endif
if (command_line.HasSwitch(network::switches::kNetworkServiceScheduler)) {
thread_options.delegate = std::make_unique<network::ThreadDelegate>(
thread_options.message_pump_type);
}
CHECK(io_thread_->StartWithOptions(std::move(thread_options)));
}

Expand Down
5 changes: 4 additions & 1 deletion services/network/network_context.cc
Expand Up @@ -110,6 +110,7 @@
#include "services/network/public/cpp/network_switches.h"
#include "services/network/public/cpp/parsed_headers.h"
#include "services/network/public/cpp/simple_host_resolver.h"
#include "services/network/public/cpp/thread_delegate.h"
#include "services/network/public/mojom/clear_data_filter.mojom.h"
#include "services/network/public/mojom/network_context.mojom-forward.h"
#include "services/network/public/mojom/network_context.mojom.h"
Expand Down Expand Up @@ -818,7 +819,9 @@ void NetworkContext::OnComputedFirstPartySetMetadata(

auto callback = base::BindOnce(&NetworkContext::OnRCMDisconnect,
base::Unretained(this), ptr.get());
ptr->InstallReceiver(std::move(receiver), std::move(callback));
ptr->InstallReceiver(std::move(receiver),
ThreadDelegate::GetHighPriorityTaskRunner(),
std::move(callback));
restricted_cookie_managers_.insert(std::move(ptr));
}

Expand Down
2 changes: 2 additions & 0 deletions services/network/public/cpp/BUILD.gn
Expand Up @@ -131,6 +131,8 @@ component("cpp") {
"spki_hash_set.h",
"supports_loading_mode/supports_loading_mode_parser.cc",
"supports_loading_mode/supports_loading_mode_parser.h",
"thread_delegate.cc",
"thread_delegate.h",
"timing_allow_origin_parser.cc",
"timing_allow_origin_parser.h",
"trust_token_http_headers.cc",
Expand Down
4 changes: 4 additions & 0 deletions services/network/public/cpp/network_switches.cc
Expand Up @@ -114,4 +114,8 @@ const char kUseFirstPartySet[] = "use-first-party-set";
// https://github.com/web-platform-tests/rfcs/blob/master/rfcs/address_space_overrides.md
const char kIpAddressSpaceOverrides[] = "ip-address-space-overrides";

// Enables running high priority tasks in the network services using
// ThreadDelegate::GetHighPriorityTaskRunner().
const char kNetworkServiceScheduler[] = "network-service-scheduler";

} // namespace network::switches
1 change: 1 addition & 0 deletions services/network/public/cpp/network_switches.h
Expand Up @@ -27,6 +27,7 @@ COMPONENT_EXPORT(NETWORK_CPP)
extern const char kAdditionalTrustTokenKeyCommitments[];
COMPONENT_EXPORT(NETWORK_CPP) extern const char kUseFirstPartySet[];
COMPONENT_EXPORT(NETWORK_CPP) extern const char kIpAddressSpaceOverrides[];
COMPONENT_EXPORT(NETWORK_CPP) extern const char kNetworkServiceScheduler[];

} // namespace switches

Expand Down
84 changes: 84 additions & 0 deletions services/network/public/cpp/thread_delegate.cc
@@ -0,0 +1,84 @@
// Copyright 2023 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "services/network/public/cpp/thread_delegate.h"

#include "base/task/sequence_manager/sequence_manager.h"
#include "third_party/abseil-cpp/absl/base/attributes.h"

namespace network {
namespace {

using ProtoPriority = perfetto::protos::pbzero::SequenceManagerTask::Priority;

ABSL_CONST_INIT thread_local ThreadDelegate* thread_local_delegate = nullptr;

ProtoPriority TaskPriorityToProto(
base::sequence_manager::TaskQueue::QueuePriority priority) {
DCHECK_LT(static_cast<size_t>(priority),
static_cast<size_t>(TaskPriority::kNumPriorities));
switch (static_cast<TaskPriority>(priority)) {
case TaskPriority::kHighPriority:
return ProtoPriority::HIGH_PRIORITY;
case TaskPriority::kNormalPriority:
return ProtoPriority::NORMAL_PRIORITY;
case TaskPriority::kNumPriorities:
return ProtoPriority::UNKNOWN;
}
}

base::sequence_manager::SequenceManager::PrioritySettings
GetPrioritySettings() {
base::sequence_manager::SequenceManager::PrioritySettings settings(
TaskPriority::kNumPriorities, TaskPriority::kNormalPriority);
settings.SetProtoPriorityConverter(&TaskPriorityToProto);
return settings;
}

} // namespace

// static
scoped_refptr<base::SequencedTaskRunner>
ThreadDelegate::GetHighPriorityTaskRunner() {
if (thread_local_delegate) {
return thread_local_delegate->high_priority_task_queue_->task_runner();
}
return base::SequencedTaskRunner::GetCurrentDefault();
}

ThreadDelegate::ThreadDelegate(base::MessagePumpType message_pump_type)
: sequence_manager_(base::sequence_manager::CreateUnboundSequenceManager(
base::sequence_manager::SequenceManager::Settings::Builder()
.SetMessagePumpType(message_pump_type)
.SetPrioritySettings(GetPrioritySettings())
.Build())),
default_task_queue_(sequence_manager_->CreateTaskQueue(
base::sequence_manager::TaskQueue::Spec(
base::sequence_manager::QueueName::DEFAULT_TQ))),
high_priority_task_queue_(sequence_manager_->CreateTaskQueue(
base::sequence_manager::TaskQueue::Spec(
base::sequence_manager::QueueName::OTHER_TQ))),
message_pump_type_(message_pump_type) {
default_task_queue_->SetQueuePriority(TaskPriority::kNormalPriority);
high_priority_task_queue_->SetQueuePriority(TaskPriority::kHighPriority);
sequence_manager_->SetDefaultTaskRunner(default_task_queue_->task_runner());
}

ThreadDelegate::~ThreadDelegate() {
thread_local_delegate = nullptr;
}

scoped_refptr<base::SingleThreadTaskRunner>
ThreadDelegate::GetDefaultTaskRunner() {
return default_task_queue_->task_runner();
}

void ThreadDelegate::BindToCurrentThread(base::TimerSlack timer_slack) {
thread_local_delegate = this;
sequence_manager_->BindToMessagePump(
base::MessagePump::Create(message_pump_type_));
sequence_manager_->SetTimerSlack(timer_slack);
}

} // namespace network
52 changes: 52 additions & 0 deletions services/network/public/cpp/thread_delegate.h
@@ -0,0 +1,52 @@
// Copyright 2023 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef SERVICES_NETWORK_PUBLIC_CPP_THREAD_DELEGATE_H_
#define SERVICES_NETWORK_PUBLIC_CPP_THREAD_DELEGATE_H_

#include "base/component_export.h"
#include "base/message_loop/message_pump.h"
#include "base/task/sequence_manager/task_queue.h"
#include "base/threading/thread.h"

namespace base {
namespace sequence_manager {
class TaskQueue;
class SequenceManager;
} // namespace sequence_manager
} // namespace base

namespace network {

enum class TaskPriority : base::sequence_manager::TaskQueue::QueuePriority {
kHighPriority = 0,
kNormalPriority = 1,

kNumPriorities = 2,
};

// A thread delegate which allows running high priority tasks.
class COMPONENT_EXPORT(NETWORK_CPP) ThreadDelegate
: public base::Thread::Delegate {
public:
explicit ThreadDelegate(base::MessagePumpType message_pump_type);
~ThreadDelegate() override;

// Gets the high priority task runner for this thread, or the default task
// runner if a high priority task runner doesn't exist.
static scoped_refptr<base::SequencedTaskRunner> GetHighPriorityTaskRunner();

scoped_refptr<base::SingleThreadTaskRunner> GetDefaultTaskRunner() override;
void BindToCurrentThread(base::TimerSlack timer_slack) override;

private:
std::unique_ptr<base::sequence_manager::SequenceManager> sequence_manager_;
scoped_refptr<base::sequence_manager::TaskQueue> default_task_queue_;
scoped_refptr<base::sequence_manager::TaskQueue> high_priority_task_queue_;
base::MessagePumpType message_pump_type_;
};

} // namespace network

#endif // SERVICES_NETWORK_PUBLIC_CPP_THREAD_DELEGATE_H_
3 changes: 2 additions & 1 deletion services/network/restricted_cookie_manager.cc
Expand Up @@ -851,9 +851,10 @@ void RestrictedCookieManager::CookiesEnabledFor(

void RestrictedCookieManager::InstallReceiver(
mojo::PendingReceiver<mojom::RestrictedCookieManager> pending_receiver,
scoped_refptr<base::SequencedTaskRunner> task_runner,
base::OnceClosure on_disconnect_callback) {
DCHECK(!receiver_.is_bound());
receiver_.Bind(std::move(pending_receiver));
receiver_.Bind(std::move(pending_receiver), std::move(task_runner));
receiver_.set_disconnect_handler(std::move(on_disconnect_callback));
}

Expand Down
1 change: 1 addition & 0 deletions services/network/restricted_cookie_manager.h
Expand Up @@ -160,6 +160,7 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) RestrictedCookieManager
// |pending_receiver|.
void InstallReceiver(
mojo::PendingReceiver<mojom::RestrictedCookieManager> pending_receiver,
scoped_refptr<base::SequencedTaskRunner> task_runner,
base::OnceClosure on_disconnect_callback);

// Computes the First-Party Set metadata corresponding to the given `origin`,
Expand Down

0 comments on commit b8bf3ae

Please sign in to comment.