Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(20)

Unified Diff: third_party/WebKit/Source/platform/scheduler/test/lazy_scheduler_message_loop_delegate_for_tests.cc

Issue 2818533003: Make nesting/running states a RunLoop rather than a MessageLoop concept. (Closed)
Patch Set: Fix nesting observer issues with LazySchedulerMessageLoopDelegateForTests Created 3 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: third_party/WebKit/Source/platform/scheduler/test/lazy_scheduler_message_loop_delegate_for_tests.cc
diff --git a/third_party/WebKit/Source/platform/scheduler/test/lazy_scheduler_message_loop_delegate_for_tests.cc b/third_party/WebKit/Source/platform/scheduler/test/lazy_scheduler_message_loop_delegate_for_tests.cc
index bd41c59fef331ffed136343a1e3a45d4f71680ac..2dd45a83ad98f301b09236437bd9a904c2e6aac8 100644
--- a/third_party/WebKit/Source/platform/scheduler/test/lazy_scheduler_message_loop_delegate_for_tests.cc
+++ b/third_party/WebKit/Source/platform/scheduler/test/lazy_scheduler_message_loop_delegate_for_tests.cc
@@ -7,6 +7,7 @@
#include <utility>
#include "base/memory/ptr_util.h"
+#include "base/message_loop/message_loop.h"
#include "base/time/default_tick_clock.h"
namespace blink {
@@ -44,7 +45,7 @@ base::MessageLoop* LazySchedulerMessageLoopDelegateForTests::EnsureMessageLoop()
if (pending_task_runner_)
message_loop_->SetTaskRunner(std::move(pending_task_runner_));
if (pending_observer_)
- message_loop_->AddNestingObserver(pending_observer_);
+ base::RunLoop::AddNestingObserverOnCurrentThread(pending_observer_);
return message_loop_;
}
@@ -90,26 +91,51 @@ bool LazySchedulerMessageLoopDelegateForTests::RunsTasksOnCurrentThread()
}
bool LazySchedulerMessageLoopDelegateForTests::IsNested() const {
- return EnsureMessageLoop()->IsNested();
+ DCHECK(RunsTasksOnCurrentThread());
+ EnsureMessageLoop();
+ return base::RunLoop::IsNestedOnCurrentThread();
}
void LazySchedulerMessageLoopDelegateForTests::AddNestingObserver(
- base::MessageLoop::NestingObserver* observer) {
+ base::RunLoop::NestingObserver* observer) {
+ // While |observer| _could_ be associated with the current thread regardless
+ // of the presence of a MessageLoop, the association is delayed until
+ // EnsureMessageLoop() is invoked. This works around a state issue where
+ // otherwise many tests fail because of the following sequence:
+ // 1) blink::scheduler::CreateRendererSchedulerForTests()
+ // -> TaskQueueManager::TaskQueueManager()
+ // -> LazySchedulerMessageLoopDelegateForTests::AddNestingObserver()
+ // 2) Any test framework with a base::MessageLoop member (and not caring
+ // about the blink scheduler) does:
+ // ThreadTaskRunnerHandle::Get()->PostTask(
+ // FROM_HERE, an_init_task_with_a_nested_loop);
+ // RunLoop.RunUntilIdle();
+ // 3) |a_task_with_a_nested_loop| triggers
+ // TaskQueueManager::OnBeginNestedLoop() which:
+ // a) flags any_thread().is_nested = true;
+ // b) posts a task to self, which triggers:
+ // LazySchedulerMessageLoopDelegateForTests::PostDelayedTask()
+ // 4) This self-task in turn triggers TaskQueueManager::DoWork()
+ // which expects to be the only one to trigger nested loops (doesn't
+ // support TaskQueueManager::OnBeginNestedLoop() being invoked before
+ // it kicks in), resulting in it hitting:
+ // DCHECK_EQ(any_thread().is_nested, delegate_->IsNested()); (1 vs 0).
+ // TODO(skyostil): fix this convulotion as part of http://crbug.com/495659.
Sami 2017/05/04 15:15:24 typo: convolution :)
if (!HasMessageLoop()) {
pending_observer_ = observer;
return;
}
- message_loop_->AddNestingObserver(observer);
+ base::RunLoop::AddNestingObserverOnCurrentThread(observer);
}
void LazySchedulerMessageLoopDelegateForTests::RemoveNestingObserver(
- base::MessageLoop::NestingObserver* observer) {
+ base::RunLoop::NestingObserver* observer) {
if (!message_loop_ || message_loop_ != base::MessageLoop::current()) {
DCHECK_EQ(pending_observer_, observer);
pending_observer_ = nullptr;
return;
}
- message_loop_->RemoveNestingObserver(observer);
+ base::RunLoop::RemoveNestingObserverOnCurrentThread(observer);
}
base::TimeTicks LazySchedulerMessageLoopDelegateForTests::NowTicks() {

Powered by Google App Engine
This is Rietveld 408576698