Commit 75f1ce9b authored by Gabriel Charette's avatar Gabriel Charette Committed by Commit Bot

Delete unused ScopedDoNotUseUIDefaultQueueFromIO

It was never used and never will be as the IO thread is being
phased out altogether.

This is a partial revert of https://chromium-review.googlesource.com/c/chromium/src/+/1660340

This CL keeps (diff with the raw revert on PS1):
 1) TaskQueueThrottler's Metadata inner class which isolates some logic
 2) TaskObserver::OnQueueNextWakeUpChanged dropping the unused TaskQueue* param
 3) TaskQueueImpl::SetObserver in favor of TaskQueueImpl::SetOnNextWakeUpChangedCallback
   (more adaptable to future instrumentation)
 4) BrowserTaskQueues::Handle as a ref-counted class
   (could go back to being copied around but avoiding duplicate state
    makes state management easier and even essential in some cases like
    validator_sets_ used to require)
   Note: The provider of Handle at the time has been renamed from
         BrowserIOTaskEnvironment to BrowserIOThreadDelegate if you're
         looking for it.
   Side-effect: this keeps all the . to -> usage changes for Handle.

And removes:
 A) TaskQueueObserver::OnPostTask
 B) BrowserTaskQueues::Validator
 C) ScopedDoNotUseUIDefaultQueueFromIO

This is associated to crbug.com/1026641 because supporting
BrowserTaskQueues::Validator adds an unnecessary burden (not that it'd
be impossible to re-add them if explicitly necessary post-migration).

TBR=nasko@chromium.org (BUILD.gn)

Bug: 863341, 1026641
Change-Id: I7bfdc8730cacbac33e2e9b57d1936b6755033753
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2008462
Commit-Queue: Gabriel Charette <gab@chromium.org>
Auto-Submit: Gabriel Charette <gab@chromium.org>
Reviewed-by: default avatarAlexander Timin <altimin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#734660}
parent 2cc4ff8f
...@@ -2212,7 +2212,6 @@ class MockTaskQueueObserver : public TaskQueue::Observer { ...@@ -2212,7 +2212,6 @@ class MockTaskQueueObserver : public TaskQueue::Observer {
public: public:
~MockTaskQueueObserver() override = default; ~MockTaskQueueObserver() override = default;
MOCK_METHOD2(OnPostTask, void(Location, TimeDelta));
MOCK_METHOD1(OnQueueNextWakeUpChanged, void(TimeTicks)); MOCK_METHOD1(OnQueueNextWakeUpChanged, void(TimeTicks));
}; };
...@@ -2226,14 +2225,12 @@ TEST_P(SequenceManagerTest, TaskQueueObserver_ImmediateTask) { ...@@ -2226,14 +2225,12 @@ TEST_P(SequenceManagerTest, TaskQueueObserver_ImmediateTask) {
// We should get a OnQueueNextWakeUpChanged notification when a task is posted // We should get a OnQueueNextWakeUpChanged notification when a task is posted
// on an empty queue. // on an empty queue.
EXPECT_CALL(observer, OnPostTask(_, TimeDelta()));
EXPECT_CALL(observer, OnQueueNextWakeUpChanged(_)); EXPECT_CALL(observer, OnQueueNextWakeUpChanged(_));
queue->task_runner()->PostTask(FROM_HERE, BindOnce(&NopTask)); queue->task_runner()->PostTask(FROM_HERE, BindOnce(&NopTask));
sequence_manager()->ReloadEmptyWorkQueues(); sequence_manager()->ReloadEmptyWorkQueues();
Mock::VerifyAndClearExpectations(&observer); Mock::VerifyAndClearExpectations(&observer);
// But not subsequently. // But not subsequently.
EXPECT_CALL(observer, OnPostTask(_, TimeDelta()));
EXPECT_CALL(observer, OnQueueNextWakeUpChanged(_)).Times(0); EXPECT_CALL(observer, OnQueueNextWakeUpChanged(_)).Times(0);
queue->task_runner()->PostTask(FROM_HERE, BindOnce(&NopTask)); queue->task_runner()->PostTask(FROM_HERE, BindOnce(&NopTask));
sequence_manager()->ReloadEmptyWorkQueues(); sequence_manager()->ReloadEmptyWorkQueues();
...@@ -2244,7 +2241,6 @@ TEST_P(SequenceManagerTest, TaskQueueObserver_ImmediateTask) { ...@@ -2244,7 +2241,6 @@ TEST_P(SequenceManagerTest, TaskQueueObserver_ImmediateTask) {
sequence_manager()->DidRunTask(); sequence_manager()->DidRunTask();
sequence_manager()->SelectNextTask(); sequence_manager()->SelectNextTask();
sequence_manager()->DidRunTask(); sequence_manager()->DidRunTask();
EXPECT_CALL(observer, OnPostTask(_, TimeDelta()));
EXPECT_CALL(observer, OnQueueNextWakeUpChanged(_)); EXPECT_CALL(observer, OnQueueNextWakeUpChanged(_));
queue->task_runner()->PostTask(FROM_HERE, BindOnce(&NopTask)); queue->task_runner()->PostTask(FROM_HERE, BindOnce(&NopTask));
sequence_manager()->ReloadEmptyWorkQueues(); sequence_manager()->ReloadEmptyWorkQueues();
...@@ -2267,7 +2263,6 @@ TEST_P(SequenceManagerTest, TaskQueueObserver_DelayedTask) { ...@@ -2267,7 +2263,6 @@ TEST_P(SequenceManagerTest, TaskQueueObserver_DelayedTask) {
// We should get OnQueueNextWakeUpChanged notification when a delayed task is // We should get OnQueueNextWakeUpChanged notification when a delayed task is
// is posted on an empty queue. // is posted on an empty queue.
EXPECT_CALL(observer, OnPostTask(_, delay10s));
EXPECT_CALL(observer, OnQueueNextWakeUpChanged(start_time + delay10s)); EXPECT_CALL(observer, OnQueueNextWakeUpChanged(start_time + delay10s));
queue->task_runner()->PostDelayedTask(FROM_HERE, BindOnce(&NopTask), queue->task_runner()->PostDelayedTask(FROM_HERE, BindOnce(&NopTask),
delay10s); delay10s);
...@@ -2275,14 +2270,12 @@ TEST_P(SequenceManagerTest, TaskQueueObserver_DelayedTask) { ...@@ -2275,14 +2270,12 @@ TEST_P(SequenceManagerTest, TaskQueueObserver_DelayedTask) {
// We should not get an OnQueueNextWakeUpChanged notification for a longer // We should not get an OnQueueNextWakeUpChanged notification for a longer
// delay. // delay.
EXPECT_CALL(observer, OnPostTask(_, delay100s));
EXPECT_CALL(observer, OnQueueNextWakeUpChanged(_)).Times(0); EXPECT_CALL(observer, OnQueueNextWakeUpChanged(_)).Times(0);
queue->task_runner()->PostDelayedTask(FROM_HERE, BindOnce(&NopTask), queue->task_runner()->PostDelayedTask(FROM_HERE, BindOnce(&NopTask),
delay100s); delay100s);
Mock::VerifyAndClearExpectations(&observer); Mock::VerifyAndClearExpectations(&observer);
// We should get an OnQueueNextWakeUpChanged notification for a shorter delay. // We should get an OnQueueNextWakeUpChanged notification for a shorter delay.
EXPECT_CALL(observer, OnPostTask(_, delay1s));
EXPECT_CALL(observer, OnQueueNextWakeUpChanged(start_time + delay1s)); EXPECT_CALL(observer, OnQueueNextWakeUpChanged(start_time + delay1s));
queue->task_runner()->PostDelayedTask(FROM_HERE, BindOnce(&NopTask), delay1s); queue->task_runner()->PostDelayedTask(FROM_HERE, BindOnce(&NopTask), delay1s);
Mock::VerifyAndClearExpectations(&observer); Mock::VerifyAndClearExpectations(&observer);
...@@ -2294,7 +2287,6 @@ TEST_P(SequenceManagerTest, TaskQueueObserver_DelayedTask) { ...@@ -2294,7 +2287,6 @@ TEST_P(SequenceManagerTest, TaskQueueObserver_DelayedTask) {
// When a queue has been enabled, we may get a notification if the // When a queue has been enabled, we may get a notification if the
// TimeDomain's next scheduled wake-up has changed. // TimeDomain's next scheduled wake-up has changed.
EXPECT_CALL(observer, OnPostTask(_, _)).Times(0);
EXPECT_CALL(observer, OnQueueNextWakeUpChanged(start_time + delay1s)); EXPECT_CALL(observer, OnQueueNextWakeUpChanged(start_time + delay1s));
voter->SetVoteToEnable(true); voter->SetVoteToEnable(true);
Mock::VerifyAndClearExpectations(&observer); Mock::VerifyAndClearExpectations(&observer);
...@@ -2315,10 +2307,8 @@ TEST_P(SequenceManagerTest, TaskQueueObserver_DelayedTaskMultipleQueues) { ...@@ -2315,10 +2307,8 @@ TEST_P(SequenceManagerTest, TaskQueueObserver_DelayedTaskMultipleQueues) {
TimeDelta delay1s(TimeDelta::FromSeconds(1)); TimeDelta delay1s(TimeDelta::FromSeconds(1));
TimeDelta delay10s(TimeDelta::FromSeconds(10)); TimeDelta delay10s(TimeDelta::FromSeconds(10));
EXPECT_CALL(observer0, OnPostTask(_, delay1s));
EXPECT_CALL(observer0, OnQueueNextWakeUpChanged(start_time + delay1s)) EXPECT_CALL(observer0, OnQueueNextWakeUpChanged(start_time + delay1s))
.Times(1); .Times(1);
EXPECT_CALL(observer1, OnPostTask(_, delay10s));
EXPECT_CALL(observer1, OnQueueNextWakeUpChanged(start_time + delay10s)) EXPECT_CALL(observer1, OnQueueNextWakeUpChanged(start_time + delay10s))
.Times(1); .Times(1);
queues[0]->task_runner()->PostDelayedTask(FROM_HERE, BindOnce(&NopTask), queues[0]->task_runner()->PostDelayedTask(FROM_HERE, BindOnce(&NopTask),
...@@ -2334,26 +2324,22 @@ TEST_P(SequenceManagerTest, TaskQueueObserver_DelayedTaskMultipleQueues) { ...@@ -2334,26 +2324,22 @@ TEST_P(SequenceManagerTest, TaskQueueObserver_DelayedTaskMultipleQueues) {
queues[1]->CreateQueueEnabledVoter(); queues[1]->CreateQueueEnabledVoter();
// Disabling a queue should not trigger a notification. // Disabling a queue should not trigger a notification.
EXPECT_CALL(observer0, OnPostTask(_, _)).Times(0);
EXPECT_CALL(observer0, OnQueueNextWakeUpChanged(_)).Times(0); EXPECT_CALL(observer0, OnQueueNextWakeUpChanged(_)).Times(0);
voter0->SetVoteToEnable(false); voter0->SetVoteToEnable(false);
Mock::VerifyAndClearExpectations(&observer0); Mock::VerifyAndClearExpectations(&observer0);
// But re-enabling it should should trigger an OnQueueNextWakeUpChanged // But re-enabling it should should trigger an OnQueueNextWakeUpChanged
// notification. // notification.
EXPECT_CALL(observer0, OnPostTask(_, _)).Times(0);
EXPECT_CALL(observer0, OnQueueNextWakeUpChanged(start_time + delay1s)); EXPECT_CALL(observer0, OnQueueNextWakeUpChanged(start_time + delay1s));
voter0->SetVoteToEnable(true); voter0->SetVoteToEnable(true);
Mock::VerifyAndClearExpectations(&observer0); Mock::VerifyAndClearExpectations(&observer0);
// Disabling a queue should not trigger a notification. // Disabling a queue should not trigger a notification.
EXPECT_CALL(observer1, OnPostTask(_, _)).Times(0);
EXPECT_CALL(observer1, OnQueueNextWakeUpChanged(_)).Times(0); EXPECT_CALL(observer1, OnQueueNextWakeUpChanged(_)).Times(0);
voter1->SetVoteToEnable(false); voter1->SetVoteToEnable(false);
Mock::VerifyAndClearExpectations(&observer0); Mock::VerifyAndClearExpectations(&observer0);
// But re-enabling it should should trigger a notification. // But re-enabling it should should trigger a notification.
EXPECT_CALL(observer1, OnPostTask(_, _)).Times(0);
EXPECT_CALL(observer1, OnQueueNextWakeUpChanged(start_time + delay10s)); EXPECT_CALL(observer1, OnQueueNextWakeUpChanged(start_time + delay10s));
voter1->SetVoteToEnable(true); voter1->SetVoteToEnable(true);
Mock::VerifyAndClearExpectations(&observer1); Mock::VerifyAndClearExpectations(&observer1);
...@@ -2383,7 +2369,6 @@ TEST_P(SequenceManagerTest, TaskQueueObserver_DelayedWorkWhichCanRunNow) { ...@@ -2383,7 +2369,6 @@ TEST_P(SequenceManagerTest, TaskQueueObserver_DelayedWorkWhichCanRunNow) {
// We should get a notification when a delayed task is posted on an empty // We should get a notification when a delayed task is posted on an empty
// queue. // queue.
EXPECT_CALL(observer, OnPostTask(_, _));
EXPECT_CALL(observer, OnQueueNextWakeUpChanged(_)); EXPECT_CALL(observer, OnQueueNextWakeUpChanged(_));
queue->task_runner()->PostDelayedTask(FROM_HERE, BindOnce(&NopTask), delay1s); queue->task_runner()->PostDelayedTask(FROM_HERE, BindOnce(&NopTask), delay1s);
Mock::VerifyAndClearExpectations(&observer); Mock::VerifyAndClearExpectations(&observer);
...@@ -2394,7 +2379,6 @@ TEST_P(SequenceManagerTest, TaskQueueObserver_DelayedWorkWhichCanRunNow) { ...@@ -2394,7 +2379,6 @@ TEST_P(SequenceManagerTest, TaskQueueObserver_DelayedWorkWhichCanRunNow) {
AdvanceMockTickClock(delay10s); AdvanceMockTickClock(delay10s);
EXPECT_CALL(observer, OnPostTask(_, _)).Times(0);
EXPECT_CALL(observer, OnQueueNextWakeUpChanged(_)); EXPECT_CALL(observer, OnQueueNextWakeUpChanged(_));
queue->SetTimeDomain(mock_time_domain.get()); queue->SetTimeDomain(mock_time_domain.get());
Mock::VerifyAndClearExpectations(&observer); Mock::VerifyAndClearExpectations(&observer);
...@@ -2425,7 +2409,6 @@ TEST_P(SequenceManagerTest, TaskQueueObserver_SweepCanceledDelayedTasks) { ...@@ -2425,7 +2409,6 @@ TEST_P(SequenceManagerTest, TaskQueueObserver_SweepCanceledDelayedTasks) {
TimeDelta delay1(TimeDelta::FromSeconds(5)); TimeDelta delay1(TimeDelta::FromSeconds(5));
TimeDelta delay2(TimeDelta::FromSeconds(10)); TimeDelta delay2(TimeDelta::FromSeconds(10));
EXPECT_CALL(observer, OnPostTask(_, _)).Times(AnyNumber());
EXPECT_CALL(observer, OnQueueNextWakeUpChanged(start_time + delay1)).Times(1); EXPECT_CALL(observer, OnQueueNextWakeUpChanged(start_time + delay1)).Times(1);
CancelableTask task1(mock_tick_clock()); CancelableTask task1(mock_tick_clock());
...@@ -3410,7 +3393,6 @@ TEST_P(SequenceManagerTest, ObserverNotFiredAfterTaskQueueDestructed) { ...@@ -3410,7 +3393,6 @@ TEST_P(SequenceManagerTest, ObserverNotFiredAfterTaskQueueDestructed) {
main_tq->SetObserver(&observer); main_tq->SetObserver(&observer);
// We don't expect the observer to fire if the TaskQueue gets destructed. // We don't expect the observer to fire if the TaskQueue gets destructed.
EXPECT_CALL(observer, OnPostTask(_, _)).Times(0);
EXPECT_CALL(observer, OnQueueNextWakeUpChanged(_)).Times(0); EXPECT_CALL(observer, OnQueueNextWakeUpChanged(_)).Times(0);
auto task_runner = main_tq->task_runner(); auto task_runner = main_tq->task_runner();
main_tq = nullptr; main_tq = nullptr;
...@@ -3431,8 +3413,6 @@ TEST_P(SequenceManagerTest, ...@@ -3431,8 +3413,6 @@ TEST_P(SequenceManagerTest,
main_tq->CreateQueueEnabledVoter(); main_tq->CreateQueueEnabledVoter();
voter->SetVoteToEnable(false); voter->SetVoteToEnable(false);
EXPECT_CALL(observer, OnPostTask(_, _));
// We don't expect the OnQueueNextWakeUpChanged to fire if the TaskQueue gets // We don't expect the OnQueueNextWakeUpChanged to fire if the TaskQueue gets
// disabled. // disabled.
EXPECT_CALL(observer, OnQueueNextWakeUpChanged(_)).Times(0); EXPECT_CALL(observer, OnQueueNextWakeUpChanged(_)).Times(0);
...@@ -3458,8 +3438,6 @@ TEST_P(SequenceManagerTest, ...@@ -3458,8 +3438,6 @@ TEST_P(SequenceManagerTest,
main_tq->CreateQueueEnabledVoter(); main_tq->CreateQueueEnabledVoter();
voter->SetVoteToEnable(false); voter->SetVoteToEnable(false);
EXPECT_CALL(observer, OnPostTask(_, _));
// We don't expect the observer to fire if the TaskQueue gets blocked. // We don't expect the observer to fire if the TaskQueue gets blocked.
EXPECT_CALL(observer, OnQueueNextWakeUpChanged(_)).Times(0); EXPECT_CALL(observer, OnQueueNextWakeUpChanged(_)).Times(0);
......
...@@ -50,10 +50,6 @@ class BASE_EXPORT TaskQueue : public RefCountedThreadSafe<TaskQueue> { ...@@ -50,10 +50,6 @@ class BASE_EXPORT TaskQueue : public RefCountedThreadSafe<TaskQueue> {
public: public:
virtual ~Observer() = default; virtual ~Observer() = default;
// Notify observer that a task has been posted on the TaskQueue. Can be
// called on any thread.
virtual void OnPostTask(Location from_here, TimeDelta delay) = 0;
// Notify observer that the time at which this queue wants to run // Notify observer that the time at which this queue wants to run
// the next task has changed. |next_wakeup| can be in the past // the next task has changed. |next_wakeup| can be in the past
// (e.g. TimeTicks() can be used to notify about immediate work). // (e.g. TimeTicks() can be used to notify about immediate work).
......
...@@ -279,8 +279,6 @@ void TaskQueueImpl::PostImmediateTaskImpl(PostedTask task, ...@@ -279,8 +279,6 @@ void TaskQueueImpl::PostImmediateTaskImpl(PostedTask task,
// See https://crbug.com/901800 // See https://crbug.com/901800
base::internal::CheckedAutoLock lock(any_thread_lock_); base::internal::CheckedAutoLock lock(any_thread_lock_);
LazyNow lazy_now = any_thread_.time_domain->CreateLazyNow(); LazyNow lazy_now = any_thread_.time_domain->CreateLazyNow();
if (any_thread_.task_queue_observer)
any_thread_.task_queue_observer->OnPostTask(task.location, TimeDelta());
bool add_queue_time_to_tasks = sequence_manager_->GetAddQueueTimeToTasks(); bool add_queue_time_to_tasks = sequence_manager_->GetAddQueueTimeToTasks();
if (add_queue_time_to_tasks || delayed_fence_allowed_) if (add_queue_time_to_tasks || delayed_fence_allowed_)
task.queue_time = lazy_now.Now(); task.queue_time = lazy_now.Now();
...@@ -364,10 +362,6 @@ void TaskQueueImpl::PostDelayedTaskImpl(PostedTask task, ...@@ -364,10 +362,6 @@ void TaskQueueImpl::PostDelayedTaskImpl(PostedTask task,
TimeTicks time_domain_now = main_thread_only().time_domain->Now(); TimeTicks time_domain_now = main_thread_only().time_domain->Now();
TimeTicks time_domain_delayed_run_time = time_domain_now + task.delay; TimeTicks time_domain_delayed_run_time = time_domain_now + task.delay;
if (main_thread_only().task_queue_observer) {
main_thread_only().task_queue_observer->OnPostTask(task.location,
task.delay);
}
if (sequence_manager_->GetAddQueueTimeToTasks()) if (sequence_manager_->GetAddQueueTimeToTasks())
task.queue_time = time_domain_now; task.queue_time = time_domain_now;
...@@ -386,8 +380,6 @@ void TaskQueueImpl::PostDelayedTaskImpl(PostedTask task, ...@@ -386,8 +380,6 @@ void TaskQueueImpl::PostDelayedTaskImpl(PostedTask task,
{ {
base::internal::CheckedAutoLock lock(any_thread_lock_); base::internal::CheckedAutoLock lock(any_thread_lock_);
time_domain_now = any_thread_.time_domain->Now(); time_domain_now = any_thread_.time_domain->Now();
if (any_thread_.task_queue_observer)
any_thread_.task_queue_observer->OnPostTask(task.location, task.delay);
} }
TimeTicks time_domain_delayed_run_time = time_domain_now + task.delay; TimeTicks time_domain_delayed_run_time = time_domain_now + task.delay;
if (sequence_manager_->GetAddQueueTimeToTasks()) if (sequence_manager_->GetAddQueueTimeToTasks())
......
...@@ -1593,8 +1593,6 @@ jumbo_source_set("browser") { ...@@ -1593,8 +1593,6 @@ jumbo_source_set("browser") {
"scheduler/responsiveness/native_event_observer_mac.mm", "scheduler/responsiveness/native_event_observer_mac.mm",
"scheduler/responsiveness/watcher.cc", "scheduler/responsiveness/watcher.cc",
"scheduler/responsiveness/watcher.h", "scheduler/responsiveness/watcher.h",
"scheduler/scoped_do_not_use_ui_default_queue_from_io.cc",
"scheduler/scoped_do_not_use_ui_default_queue_from_io.h",
"scoped_active_url.cc", "scoped_active_url.cc",
"scoped_active_url.h", "scoped_active_url.h",
"screen_enumeration/screen_enumeration_impl.cc", "screen_enumeration/screen_enumeration_impl.cc",
......
...@@ -317,60 +317,6 @@ std::unique_ptr<BrowserProcessSubThread> BrowserTaskExecutor::CreateIOThread() { ...@@ -317,60 +317,6 @@ std::unique_ptr<BrowserProcessSubThread> BrowserTaskExecutor::CreateIOThread() {
return io_thread; return io_thread;
} }
#if DCHECK_IS_ON()
// static
void BrowserTaskExecutor::AddValidator(
const base::TaskTraits& traits,
BrowserTaskQueues::Validator* validator) {
if (!g_browser_task_executor)
return;
auto id_and_queue = g_browser_task_executor->GetThreadIdAndQueueType(traits);
switch (id_and_queue.thread_id) {
case BrowserThread::ID::IO:
g_browser_task_executor->browser_io_thread_handle_->AddValidator(
id_and_queue.queue_type, validator);
break;
case BrowserThread::ID::UI:
g_browser_task_executor->browser_ui_thread_handle_->AddValidator(
id_and_queue.queue_type, validator);
break;
case BrowserThread::ID::ID_COUNT:
NOTREACHED();
break;
}
}
// static
void BrowserTaskExecutor::RemoveValidator(
const base::TaskTraits& traits,
BrowserTaskQueues::Validator* validator) {
if (!g_browser_task_executor)
return;
auto id_and_queue = g_browser_task_executor->GetThreadIdAndQueueType(traits);
switch (id_and_queue.thread_id) {
case BrowserThread::ID::IO:
g_browser_task_executor->browser_io_thread_handle_->RemoveValidator(
id_and_queue.queue_type, validator);
break;
case BrowserThread::ID::UI:
g_browser_task_executor->browser_ui_thread_handle_->RemoveValidator(
id_and_queue.queue_type, validator);
break;
case BrowserThread::ID::ID_COUNT:
NOTREACHED();
break;
}
}
#endif
BrowserThread::ID BrowserTaskExecutor::GetCurrentThreadID() const { BrowserThread::ID BrowserTaskExecutor::GetCurrentThreadID() const {
NOTREACHED() NOTREACHED()
<< "Should have been routed to UIThreadExecutor or IOThreadExecutor"; << "Should have been routed to UIThreadExecutor or IOThreadExecutor";
......
...@@ -160,19 +160,6 @@ class CONTENT_EXPORT BrowserTaskExecutor : public BaseBrowserTaskExecutor { ...@@ -160,19 +160,6 @@ class CONTENT_EXPORT BrowserTaskExecutor : public BaseBrowserTaskExecutor {
static void RunAllPendingTasksOnThreadForTesting( static void RunAllPendingTasksOnThreadForTesting(
BrowserThread::ID identifier); BrowserThread::ID identifier);
#if DCHECK_IS_ON()
// Adds a Validator for |traits|. It is assumed the lifetime of |validator| is
// is longer than that of the BrowserTaskExecutor unless RemoveValidator
// is called. Does nothing if the BrowserTaskExecutor is not registered.
static void AddValidator(const base::TaskTraits& traits,
BrowserTaskQueues::Validator* validator);
// Removes a Validator previously added by AddValidator. Does nothing if the
// BrowserTaskExecutor is not registered.
static void RemoveValidator(const base::TaskTraits& traits,
BrowserTaskQueues::Validator* validator);
#endif // DCHECK_IS_ON()
private: private:
friend class BrowserIOThreadDelegate; friend class BrowserIOThreadDelegate;
friend class BrowserTaskExecutorTest; friend class BrowserTaskExecutorTest;
......
...@@ -150,52 +150,6 @@ void BrowserTaskQueues::Handle::ScheduleRunAllPendingTasksForTesting( ...@@ -150,52 +150,6 @@ void BrowserTaskQueues::Handle::ScheduleRunAllPendingTasksForTesting(
base::ScopedClosureRunner(std::move(on_pending_task_ran)))); base::ScopedClosureRunner(std::move(on_pending_task_ran))));
} }
#if DCHECK_IS_ON()
void BrowserTaskQueues::Handle::AddValidator(QueueType queue_type,
Validator* validator) {
validator_sets_[static_cast<size_t>(queue_type)].AddValidator(validator);
}
void BrowserTaskQueues::Handle::RemoveValidator(QueueType queue_type,
Validator* validator) {
validator_sets_[static_cast<size_t>(queue_type)].RemoveValidator(validator);
}
BrowserTaskQueues::ValidatorSet::ValidatorSet() = default;
BrowserTaskQueues::ValidatorSet::~ValidatorSet() {
// Note the queue has already been shut down by the time we're deleted so we
// don't need to unregister.
DCHECK(validators_.empty());
}
void BrowserTaskQueues::ValidatorSet::AddValidator(Validator* validator) {
base::AutoLock lock(lock_);
DCHECK_EQ(validators_.count(validator), 0u)
<< "Validator added more than once";
validators_.insert(validator);
}
void BrowserTaskQueues::ValidatorSet::RemoveValidator(Validator* validator) {
base::AutoLock lock(lock_);
size_t num_erased = validators_.erase(validator);
DCHECK_EQ(num_erased, 1u) << "Validator not in set";
}
void BrowserTaskQueues::ValidatorSet::OnPostTask(base::Location from_here,
base::TimeDelta delay) {
base::AutoLock lock(lock_);
for (Validator* validator : validators_) {
validator->ValidatePostTask(from_here);
}
}
void BrowserTaskQueues::ValidatorSet::OnQueueNextWakeUpChanged(
base::TimeTicks next_wake_up) {}
#endif // DCHECK_IS_ON()
BrowserTaskQueues::QueueData::QueueData() = default; BrowserTaskQueues::QueueData::QueueData() = default;
BrowserTaskQueues::QueueData::~QueueData() = default; BrowserTaskQueues::QueueData::~QueueData() = default;
...@@ -240,17 +194,6 @@ BrowserTaskQueues::BrowserTaskQueues( ...@@ -240,17 +194,6 @@ BrowserTaskQueues::BrowserTaskQueues(
QueuePriority::kBestEffortPriority); QueuePriority::kBestEffortPriority);
handle_ = base::AdoptRef(new Handle(this)); handle_ = base::AdoptRef(new Handle(this));
#if DCHECK_IS_ON()
for (size_t i = 0; i < queue_data_.size(); ++i) {
queue_data_[i].task_queue->SetObserver(&handle_->validator_sets_[i]);
}
// Treat the |default_task_queue_| the same as the USER_BLOCKING task queue
// from a validation point of view.
default_task_queue_->SetObserver(
&handle_->validator_sets_[static_cast<int>(QueueType::kUserBlocking)]);
#endif
} }
BrowserTaskQueues::~BrowserTaskQueues() { BrowserTaskQueues::~BrowserTaskQueues() {
......
...@@ -6,11 +6,9 @@ ...@@ -6,11 +6,9 @@
#define CONTENT_BROWSER_SCHEDULER_BROWSER_TASK_QUEUES_H_ #define CONTENT_BROWSER_SCHEDULER_BROWSER_TASK_QUEUES_H_
#include <array> #include <array>
#include <set>
#include "base/memory/scoped_refptr.h" #include "base/memory/scoped_refptr.h"
#include "base/task/sequence_manager/task_queue.h" #include "base/task/sequence_manager/task_queue.h"
#include "base/thread_annotations.h"
#include "content/common/content_export.h" #include "content/common/content_export.h"
#include "content/public/browser/browser_thread.h" #include "content/public/browser/browser_thread.h"
...@@ -71,36 +69,6 @@ class CONTENT_EXPORT BrowserTaskQueues { ...@@ -71,36 +69,6 @@ class CONTENT_EXPORT BrowserTaskQueues {
static constexpr size_t kNumQueueTypes = static constexpr size_t kNumQueueTypes =
static_cast<size_t>(QueueType::kMaxValue) + 1; static_cast<size_t>(QueueType::kMaxValue) + 1;
// Per queue interface for validating PostTasks. Validation is only enabled if
// DCHECKs are on.
class Validator {
public:
virtual ~Validator() = default;
// This should check fail if there's a problem.
virtual void ValidatePostTask(const base::Location& from_here) = 0;
};
#if DCHECK_IS_ON()
class CONTENT_EXPORT ValidatorSet
: public base::sequence_manager::TaskQueue::Observer {
public:
ValidatorSet();
~ValidatorSet() override;
void AddValidator(Validator* validator);
void RemoveValidator(Validator* validator);
// base::sequence_manager::TaskQueue::Observer:
void OnPostTask(base::Location from_here, base::TimeDelta delay) override;
void OnQueueNextWakeUpChanged(base::TimeTicks next_wake_up) override;
private:
base::Lock lock_;
std::set<Validator*> validators_ GUARDED_BY(lock_);
};
#endif
// Handle to a BrowserTaskQueues instance that can be used from any thread // Handle to a BrowserTaskQueues instance that can be used from any thread
// as all operations are thread safe. // as all operations are thread safe.
// //
...@@ -151,12 +119,6 @@ class CONTENT_EXPORT BrowserTaskQueues { ...@@ -151,12 +119,6 @@ class CONTENT_EXPORT BrowserTaskQueues {
void ScheduleRunAllPendingTasksForTesting( void ScheduleRunAllPendingTasksForTesting(
base::OnceClosure on_pending_task_ran); base::OnceClosure on_pending_task_ran);
// Adds a Validator for |queue_type|.
void AddValidator(QueueType queue_type, Validator* validator);
// Removes a Validator previously added by AddValidator.
void RemoveValidator(QueueType queue_type, Validator* validator);
private: private:
friend base::RefCountedThreadSafe<Handle>; friend base::RefCountedThreadSafe<Handle>;
...@@ -174,9 +136,6 @@ class CONTENT_EXPORT BrowserTaskQueues { ...@@ -174,9 +136,6 @@ class CONTENT_EXPORT BrowserTaskQueues {
scoped_refptr<base::SingleThreadTaskRunner> default_task_runner_; scoped_refptr<base::SingleThreadTaskRunner> default_task_runner_;
std::array<scoped_refptr<base::SingleThreadTaskRunner>, kNumQueueTypes> std::array<scoped_refptr<base::SingleThreadTaskRunner>, kNumQueueTypes>
browser_task_runners_; browser_task_runners_;
#if DCHECK_IS_ON()
std::array<ValidatorSet, kNumQueueTypes> validator_sets_;
#endif
}; };
// |sequence_manager| and |time_domain| must outlive this instance. // |sequence_manager| and |time_domain| must outlive this instance.
......
// Copyright 2019 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "content/browser/scheduler/scoped_do_not_use_ui_default_queue_from_io.h"
#include "base/bind.h"
#include "base/trace_event/trace_event.h"
#include "content/browser/scheduler/browser_task_executor.h"
namespace content {
ScopedDoNotUseUIDefaultQueueFromIO::ScopedDoNotUseUIDefaultQueueFromIO(
const base::Location& scoped_location)
: scoped_location_(scoped_location) {
TRACE_EVENT_BEGIN0("toplevel", "ScopedDoNotUseUIDefaultQueueFromIO");
#if DCHECK_IS_ON()
// Only has an effect in the browser process.
BrowserTaskExecutor::AddValidator({BrowserThread::UI}, this);
#endif
}
void ScopedDoNotUseUIDefaultQueueFromIO::ValidatePostTask(
const base::Location& post_task_location) {
DCHECK(!BrowserThread::CurrentlyOn(BrowserThread::IO))
<< "It's prohibited by ScopedDoNotUseUIDefaultQueueFromIO at "
<< scoped_location_.ToString()
<< " to post to the UI thread's default queue at this time due to the "
"risk of accidental task reordering. Please specify a non-default "
"BrowserTaskType. See PostTask "
<< post_task_location.ToString();
}
ScopedDoNotUseUIDefaultQueueFromIO::~ScopedDoNotUseUIDefaultQueueFromIO() {
TRACE_EVENT_END0("toplevel", "ScopedDoNotUseUIDefaultQueueFromIO");
#if DCHECK_IS_ON()
BrowserTaskExecutor::RemoveValidator({BrowserThread::UI}, this);
#endif
}
} // namespace content
// Copyright 2019 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef CONTENT_BROWSER_SCHEDULER_SCOPED_DO_NOT_USE_UI_DEFAULT_QUEUE_FROM_IO_H_
#define CONTENT_BROWSER_SCHEDULER_SCOPED_DO_NOT_USE_UI_DEFAULT_QUEUE_FROM_IO_H_
#include "base/location.h"
#include "content/browser/scheduler/browser_task_queues.h"
#include "content/common/content_export.h"
namespace content {
// When ScopedDoNotUseUIDefaultQueueFromIO exists, it's prohibited to post to
// the UI thread's default task runner from the IO thread.
class CONTENT_EXPORT ScopedDoNotUseUIDefaultQueueFromIO
: public BrowserTaskQueues::Validator {
public:
explicit ScopedDoNotUseUIDefaultQueueFromIO(const base::Location& location);
~ScopedDoNotUseUIDefaultQueueFromIO() override;
private:
void ValidatePostTask(const base::Location& post_task_location) override;
const base::Location scoped_location_;
};
} // namespace content
#endif // CONTENT_BROWSER_SCHEDULER_SCOPED_DO_NOT_USE_UI_DEFAULT_QUEUE_FROM_IO_H_
// Copyright 2019 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "content/browser/scheduler/scoped_do_not_use_ui_default_queue_from_io.h"
#include "base/bind_helpers.h"
#include "base/task/post_task.h"
#include "base/test/bind_test_util.h"
#include "base/test/gtest_util.h"
#include "build/build_config.h"
#include "content/public/browser/browser_task_traits.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/test/browser_task_environment.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace content {
#if defined(THREAD_SANITIZER)
// The combination of EXPECT_DCHECK_DEATH and TSan doesn't work, although we get
// the expected DCHECK with TSan builds.
#define MAYBE_BadPostFromIO DISABLED_BadPostFromIO
#elif defined(OS_MACOSX)
// This test fails to DCHECK on mac release builds for reasons unknown.
#define MAYBE_BadPostFromIO DISABLED_BadPostFromIO
#else
#define MAYBE_BadPostFromIO BadPostFromIO
#endif
TEST(ScopedDoNotUseUIDefaultQueueFromIO, MAYBE_BadPostFromIO) {
EXPECT_DCHECK_DEATH({
BrowserTaskEnvironment task_environment;
base::RunLoop run_loop;
base::PostTask(
FROM_HERE, {BrowserThread::IO}, base::BindLambdaForTesting([&]() {
ScopedDoNotUseUIDefaultQueueFromIO do_not_post_to_ui_default(
FROM_HERE);
// Posting to the UI thread with no other traits is prohibited.
base::PostTask(FROM_HERE, {BrowserThread::UI}, base::DoNothing());
}));
run_loop.Run();
});
}
TEST(ScopedDoNotUseUIDefaultQueueFromIO, PostFromIO) {
BrowserTaskEnvironment task_environment;
base::RunLoop run_loop;
base::PostTask(
FROM_HERE, {BrowserThread::IO}, base::BindLambdaForTesting([&]() {
{
ScopedDoNotUseUIDefaultQueueFromIO do_not_post_to_ui_default(
FROM_HERE);
// Posting with non default BrowserTaskType is OK.
base::PostTask(FROM_HERE,
{BrowserThread::IO, BrowserTaskType::kNavigation},
base::DoNothing());
// Posting to the IO thread default queue is OK.
base::PostTask(FROM_HERE, {BrowserThread::IO}, base::DoNothing());
}
// After |do_not_post_to_ui_default| has gone out of scope it's fine to
// post to the UI thread's default queue again.
base::PostTask(FROM_HERE, {BrowserThread::UI}, run_loop.QuitClosure());
}));
run_loop.Run();
}
TEST(ScopedDoNotUseUIDefaultQueueFromIO, PostFromUI) {
BrowserTaskEnvironment task_environment(
BrowserTaskEnvironment::REAL_IO_THREAD);
base::RunLoop run_loop;
base::PostTask(
FROM_HERE, {BrowserThread::UI}, base::BindLambdaForTesting([&]() {
ScopedDoNotUseUIDefaultQueueFromIO do_not_post_to_ui_default(FROM_HERE);
// It's fine to post from the UI thread.
base::PostTask(FROM_HERE, {BrowserThread::UI}, run_loop.QuitClosure());
}));
run_loop.Run();
}
} // namespace content
...@@ -1751,7 +1751,6 @@ test("content_unittests") { ...@@ -1751,7 +1751,6 @@ test("content_unittests") {
"../browser/scheduler/responsiveness/jank_monitor_unittest.cc", "../browser/scheduler/responsiveness/jank_monitor_unittest.cc",
"../browser/scheduler/responsiveness/metric_source_unittest.cc", "../browser/scheduler/responsiveness/metric_source_unittest.cc",
"../browser/scheduler/responsiveness/watcher_unittest.cc", "../browser/scheduler/responsiveness/watcher_unittest.cc",
"../browser/scheduler/scoped_do_not_use_ui_default_queue_from_io_unittest.cc",
"../browser/screen_orientation/screen_orientation_provider_unittest.cc", "../browser/screen_orientation/screen_orientation_provider_unittest.cc",
"../browser/screenlock_monitor/screenlock_monitor_unittest.cc", "../browser/screenlock_monitor/screenlock_monitor_unittest.cc",
"../browser/service_worker/embedded_worker_instance_unittest.cc", "../browser/service_worker/embedded_worker_instance_unittest.cc",
......
...@@ -602,9 +602,6 @@ bool TaskQueueThrottler::Metadata::DecrementRefCount() { ...@@ -602,9 +602,6 @@ bool TaskQueueThrottler::Metadata::DecrementRefCount() {
return false; return false;
} }
void TaskQueueThrottler::Metadata::OnPostTask(base::Location from_here,
base::TimeDelta delay) {}
void TaskQueueThrottler::Metadata::OnQueueNextWakeUpChanged( void TaskQueueThrottler::Metadata::OnQueueNextWakeUpChanged(
base::TimeTicks wake_up) { base::TimeTicks wake_up) {
throttler_->OnQueueNextWakeUpChanged(queue_, wake_up); throttler_->OnQueueNextWakeUpChanged(queue_, wake_up);
......
...@@ -143,7 +143,6 @@ class PLATFORM_EXPORT TaskQueueThrottler : public BudgetPoolController { ...@@ -143,7 +143,6 @@ class PLATFORM_EXPORT TaskQueueThrottler : public BudgetPoolController {
bool DecrementRefCount(); bool DecrementRefCount();
// TaskQueue::Observer implementation: // TaskQueue::Observer implementation:
void OnPostTask(base::Location from_here, base::TimeDelta delay) override;
void OnQueueNextWakeUpChanged(base::TimeTicks wake_up) override; void OnQueueNextWakeUpChanged(base::TimeTicks wake_up) override;
size_t throttling_ref_count() const { return throttling_ref_count_; } size_t throttling_ref_count() const { return throttling_ref_count_; }
...@@ -157,6 +156,8 @@ class PLATFORM_EXPORT TaskQueueThrottler : public BudgetPoolController { ...@@ -157,6 +156,8 @@ class PLATFORM_EXPORT TaskQueueThrottler : public BudgetPoolController {
TaskQueueThrottler* const throttler_; TaskQueueThrottler* const throttler_;
size_t throttling_ref_count_ = 0; size_t throttling_ref_count_ = 0;
HashSet<BudgetPool*> budget_pools_; HashSet<BudgetPool*> budget_pools_;
DISALLOW_COPY_AND_ASSIGN(Metadata);
}; };
using TaskQueueMap = using TaskQueueMap =
......
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