Skip to content

GitLab

T
tangled
Commit b61fcc11 authored by Francois Doray's avatar Francois Doray Committed by Commit Bot
Browse files
Options

[base] Remove MessagePump::DoWork/DoDelayedWork. 

All MessagePumps now use DoSomeWork.

In a future, CL:
- DoSomeWork will be renamed to DoWork
- Complexity of ThreadControllerWithMessagePump and SequenceManager
  can be reduced. For example, WorkDeduplicator can be simplified.
These changes will be done separately to minimize the size of this CL.

Bug: 885371
Change-Id: I3c557e19bf8da6dc21c655202d3572a71a23c0a1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2070716
Commit-Queue: François Doray <fdoray@chromium.org>
Reviewed-by: default avatarGabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#753371}
parent 2c73fbed
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 163 additions and 389 deletions
base/message_loop/message_pump.h
View file @ b61fcc11
This diff is collapsed.
base/message_loop/message_pump_unittest.cc
View file @ b61fcc11
......@@ -32,12 +32,6 @@ namespace base {
namespace {
bool PumpTypeUsesDoSomeWork(MessagePumpType type) {
// TODO(https://crbug.com/885371): All MessagePumps use DoSomeWork(). As a
// result, tests should be simplified.
return true;
}
class MockMessagePumpDelegate : public MessagePump::Delegate {
public:
MockMessagePumpDelegate() = default;
......@@ -46,8 +40,6 @@ class MockMessagePumpDelegate : public MessagePump::Delegate {
void BeforeDoInternalWork() override {}
void BeforeWait() override {}
MOCK_METHOD0(DoSomeWork, MessagePump::Delegate::NextWorkInfo());
MOCK_METHOD0(DoWork, bool());
MOCK_METHOD1(DoDelayedWork, bool(TimeTicks*));
MOCK_METHOD0(DoIdleWork, bool());
private:
......@@ -59,8 +51,6 @@ class MessagePumpTest : public ::testing::TestWithParam<MessagePumpType> {
MessagePumpTest() : message_pump_(MessagePump::Create(GetParam())) {}
protected:
const bool pump_uses_do_some_work_ = PumpTypeUsesDoSomeWork(GetParam());
std::unique_ptr<MessagePump> message_pump_;
};
......@@ -69,19 +59,11 @@ class MessagePumpTest : public ::testing::TestWithParam<MessagePumpType> {
TEST_P(MessagePumpTest, QuitStopsWork) {
testing::StrictMock<MockMessagePumpDelegate> delegate;
// Not expecting any calls to DoDelayedWork or DoIdleWork after quitting.
if (pump_uses_do_some_work_) {
EXPECT_CALL(delegate, DoSomeWork).WillOnce(Invoke([this] {
message_pump_->Quit();
return MessagePump::Delegate::NextWorkInfo{TimeTicks::Max()};
}));
} else {
EXPECT_CALL(delegate, DoWork).WillOnce(Invoke([this] {
message_pump_->Quit();
return false;
}));
}
EXPECT_CALL(delegate, DoDelayedWork(_)).Times(0);
// Not expecting any calls to DoIdleWork after quitting.
EXPECT_CALL(delegate, DoSomeWork).WillOnce(Invoke([this] {
message_pump_->Quit();
return MessagePump::Delegate::NextWorkInfo{TimeTicks::Max()};
}));
EXPECT_CALL(delegate, DoIdleWork()).Times(0);
message_pump_->ScheduleWork();
......@@ -93,51 +75,28 @@ TEST_P(MessagePumpTest, QuitStopsWorkWithNestedRunLoop) {
testing::StrictMock<MockMessagePumpDelegate> delegate;
testing::StrictMock<MockMessagePumpDelegate> nested_delegate;
// We first schedule a call to DoWork, which runs a nested run loop. After the
// nested loop exits, we schedule another DoWork which quits the outer
// We first schedule a call to DoSomeWork, which runs a nested run loop. After
// the nested loop exits, we schedule another DoSomeWork which quits the outer
// (original) run loop. The test verifies that there are no extra calls to
// DoWork after the outer loop quits.
if (pump_uses_do_some_work_) {
EXPECT_CALL(delegate, DoSomeWork).WillOnce(Invoke([&] {
message_pump_->ScheduleWork();
message_pump_->Run(&nested_delegate);
message_pump_->ScheduleWork();
return MessagePump::Delegate::NextWorkInfo{TimeTicks::Max()};
}));
EXPECT_CALL(nested_delegate, DoSomeWork).WillOnce(Invoke([&] {
// Quit the nested run loop.
message_pump_->Quit();
return MessagePump::Delegate::NextWorkInfo{TimeTicks::Max()};
}));
} else {
EXPECT_CALL(delegate, DoWork).WillOnce(Invoke([&] {
message_pump_->ScheduleWork();
message_pump_->Run(&nested_delegate);
message_pump_->ScheduleWork();
return false;
}));
EXPECT_CALL(nested_delegate, DoWork).WillOnce(Invoke([&] {
// Quit the nested run loop.
message_pump_->Quit();
return false;
}));
EXPECT_CALL(delegate, DoDelayedWork(_)).WillOnce(Return(false));
}
// DoSomeWork after the outer loop quits.
EXPECT_CALL(delegate, DoSomeWork).WillOnce(Invoke([&] {
message_pump_->ScheduleWork();
message_pump_->Run(&nested_delegate);
message_pump_->ScheduleWork();
return MessagePump::Delegate::NextWorkInfo{TimeTicks::Max()};
}));
EXPECT_CALL(nested_delegate, DoSomeWork).WillOnce(Invoke([&] {
// Quit the nested run loop.
message_pump_->Quit();
return MessagePump::Delegate::NextWorkInfo{TimeTicks::Max()};
}));
// The outer pump may or may not trigger idle work at this point.
EXPECT_CALL(delegate, DoIdleWork()).Times(AnyNumber());
if (pump_uses_do_some_work_) {
EXPECT_CALL(delegate, DoSomeWork).WillOnce(Invoke([this] {
message_pump_->Quit();
return MessagePump::Delegate::NextWorkInfo{TimeTicks::Max()};
}));
} else {
EXPECT_CALL(delegate, DoWork).WillOnce(Invoke([this] {
message_pump_->Quit();
return false;
}));
}
EXPECT_CALL(delegate, DoSomeWork).WillOnce(Invoke([this] {
message_pump_->Quit();
return MessagePump::Delegate::NextWorkInfo{TimeTicks::Max()};
}));
message_pump_->ScheduleWork();
message_pump_->Run(&delegate);
......@@ -182,25 +141,6 @@ class TimerSlackTestDelegate : public MessagePump::Delegate {
return MessagePump::Delegate::NextWorkInfo{TimeTicks::Max()};
}
bool DoWork() override {
switch (action_.load()) {
case NONE:
break;
case SCHEDULE_DELAYED_WORK:
// After being woken up by the other thread, we schedule work after a
// short delay. If the pump refreshes its timer correctly, it will wake
// up shortly, finishing the test.
action_.store(QUIT);
message_pump_->ScheduleDelayedWork(TimeTicks::Now() +
TimeDelta::FromMilliseconds(50));
break;
case QUIT:
message_pump_->Quit();
break;
}
return false;
}
bool DoDelayedWork(base::TimeTicks*) override { return false; }
bool DoIdleWork() override { return false; }
void WakeUpFromOtherThread() {
......@@ -253,17 +193,10 @@ TEST_P(MessagePumpTest, RunWithoutScheduleWorkInvokesDoWork) {
#if defined(OS_IOS)
EXPECT_CALL(delegate, DoIdleWork).Times(AnyNumber());
#endif
if (pump_uses_do_some_work_) {
EXPECT_CALL(delegate, DoSomeWork).WillOnce(Invoke([this] {
message_pump_->Quit();
return MessagePump::Delegate::NextWorkInfo{TimeTicks::Max()};
}));
} else {
EXPECT_CALL(delegate, DoWork).WillOnce(Invoke([this] {
message_pump_->Quit();
return false;
}));
}
EXPECT_CALL(delegate, DoSomeWork).WillOnce(Invoke([this] {
message_pump_->Quit();
return MessagePump::Delegate::NextWorkInfo{TimeTicks::Max()};
}));
message_pump_->Run(&delegate);
}
......@@ -272,35 +205,19 @@ TEST_P(MessagePumpTest, NestedRunWithoutScheduleWorkInvokesDoWork) {
#if defined(OS_IOS)
EXPECT_CALL(delegate, DoIdleWork).Times(AnyNumber());
#endif
if (pump_uses_do_some_work_) {
EXPECT_CALL(delegate, DoSomeWork).WillOnce(Invoke([this] {
testing::StrictMock<MockMessagePumpDelegate> nested_delegate;
EXPECT_CALL(delegate, DoSomeWork).WillOnce(Invoke([this] {
testing::StrictMock<MockMessagePumpDelegate> nested_delegate;
#if defined(OS_IOS)
EXPECT_CALL(nested_delegate, DoIdleWork).Times(AnyNumber());
EXPECT_CALL(nested_delegate, DoIdleWork).Times(AnyNumber());
#endif
EXPECT_CALL(nested_delegate, DoSomeWork).WillOnce(Invoke([this] {
message_pump_->Quit();
return MessagePump::Delegate::NextWorkInfo{TimeTicks::Max()};
}));
message_pump_->Run(&nested_delegate);
EXPECT_CALL(nested_delegate, DoSomeWork).WillOnce(Invoke([this] {
message_pump_->Quit();
return MessagePump::Delegate::NextWorkInfo{TimeTicks::Max()};
}));
} else {
EXPECT_CALL(delegate, DoWork).WillOnce(Invoke([this] {
testing::StrictMock<MockMessagePumpDelegate> nested_delegate;
#if defined(OS_IOS)
EXPECT_CALL(nested_delegate, DoIdleWork).Times(AnyNumber());
#endif
EXPECT_CALL(nested_delegate, DoWork).WillOnce(Invoke([this] {
message_pump_->Quit();
return false;
}));
message_pump_->Run(&nested_delegate);
message_pump_->Quit();
return false;
}));
}
message_pump_->Run(&nested_delegate);
message_pump_->Quit();
return MessagePump::Delegate::NextWorkInfo{TimeTicks::Max()};
}));
message_pump_->Run(&delegate);
}
......
base/message_loop/message_pump_win.h
View file @ b61fcc11
......@@ -82,10 +82,9 @@ class BASE_EXPORT MessagePumpWin : public MessagePump {
//
// MessagePumpForUI implements a "traditional" Windows message pump. It contains
// a nearly infinite loop that peeks out messages, and then dispatches them.
// Intermixed with those peeks are callouts to DoWork for pending tasks, and
// DoDelayedWork for pending timers. When there are no events to be serviced,
// this pump goes into a wait state. In most cases, this message pump handles
// all processing.
// Intermixed with those peeks are callouts to DoSomeWork. When there are no
// events to be serviced, this pump goes into a wait state. In most cases, this
// message pump handles all processing.
//
// However, when a task, or windows event, invokes on the stack a native dialog
// box or such, that window typically provides a bare bones (native?) message
......@@ -96,15 +95,14 @@ class BASE_EXPORT MessagePumpWin : public MessagePump {
//
// The basic structure of the extension (referred to as a sub-pump) is that a
// special message, kMsgHaveWork, is repeatedly injected into the Windows
// Message queue. Each time the kMsgHaveWork message is peeked, checks are
// made for an extended set of events, including the availability of Tasks to
// run.
// Message queue. Each time the kMsgHaveWork message is peeked, checks are made
// for an extended set of events, including the availability of Tasks to run.
//
// After running a task, the special message kMsgHaveWork is again posted to
// the Windows Message queue, ensuring a future time slice for processing a
// future event. To prevent flooding the Windows Message queue, care is taken
// to be sure that at most one kMsgHaveWork message is EVER pending in the
// Window's Message queue.
// After running a task, the special message kMsgHaveWork is again posted to the
// Windows Message queue, ensuring a future time slice for processing a future
// event. To prevent flooding the Windows Message queue, care is taken to be
// sure that at most one kMsgHaveWork message is EVER pending in the Window's
// Message queue.
//
// There are a few additional complexities in this system where, when there are
// no Tasks to run, this otherwise infinite stream of messages which drives the
......@@ -115,8 +113,8 @@ class BASE_EXPORT MessagePumpWin : public MessagePump {
// prevent a bare-bones message pump from ever peeking a WM_PAINT or WM_TIMER.
// Such paint and timer events always give priority to a posted message, such as
// kMsgHaveWork messages. As a result, care is taken to do some peeking in
// between the posting of each kMsgHaveWork message (i.e., after kMsgHaveWork
// is peeked, and before a replacement kMsgHaveWork is posted).
// between the posting of each kMsgHaveWork message (i.e., after kMsgHaveWork is
// peeked, and before a replacement kMsgHaveWork is posted).
//
// NOTE: Although it may seem odd that messages are used to start and stop this
// flow (as opposed to signaling objects, etc.), it should be understood that
......@@ -148,8 +146,10 @@ class BASE_EXPORT MessagePumpForUI : public MessagePumpWin {
void RemoveObserver(Observer* obseerver);
private:
bool MessageCallback(
UINT message, WPARAM wparam, LPARAM lparam, LRESULT* result);
bool MessageCallback(UINT message,
WPARAM wparam,
LPARAM lparam,
LRESULT* result);
void DoRunLoop() override;
void WaitForWork(Delegate::NextWorkInfo next_work_info);
void HandleWorkMessage();
......@@ -235,7 +235,8 @@ class BASE_EXPORT MessagePumpForIO : public MessagePumpWin {
// |context| completes. |error| is the Win32 error code of the IO operation
// (ERROR_SUCCESS if there was no error). |bytes_transfered| will be zero
// on error.
virtual void OnIOCompleted(IOContext* context, DWORD bytes_transfered,
virtual void OnIOCompleted(IOContext* context,
DWORD bytes_transfered,
DWORD error) = 0;
};
......
base/task/sequence_manager/test/mock_time_message_pump_unittest.cc
View file @ b61fcc11
......@@ -24,8 +24,6 @@ class MockMessagePumpDelegate : public MessagePump::Delegate {
public:
MOCK_METHOD0(BeforeDoInternalWork, void());
MOCK_METHOD0(BeforeWait, void());
MOCK_METHOD0(DoWork, bool());
MOCK_METHOD1(DoDelayedWork, bool(TimeTicks*));
MOCK_METHOD0(DoSomeWork, NextWorkInfo());
MOCK_METHOD0(DoIdleWork, bool());
};
......
base/task/sequence_manager/thread_controller_with_message_pump_impl.cc
View file @ b61fcc11
......@@ -286,90 +286,6 @@ ThreadControllerWithMessagePumpImpl::DoSomeWork() {
continuation_lazy_now.Now()};
}
bool ThreadControllerWithMessagePumpImpl::DoWork() {
// Nested runloops are covered by the parent loop hang watch scope.
// TODO(crbug/1034046): Provide more granular scoping that reuses the parent
// scope deadline.
if (main_thread_only().runloop_count == 1) {
hang_watch_scope_.emplace(base::HangWatchScope::kDefaultHangWatchTime);
}
work_deduplicator_.OnWorkStarted();
bool ran_task = false;
LazyNow continuation_lazy_now(time_source_);
TimeDelta delay_till_next_task =
DoWorkImpl(&continuation_lazy_now, &ran_task);
// Schedule a continuation.
// TODO(altimin, gab): Make this more efficient by merging DoWork
// and DoDelayedWork and allowing returning base::TimeTicks() when we have
// immediate work.
if (delay_till_next_task.is_zero()) {
// Need to run new work immediately, but due to the contract of DoWork we
// only need to return true to ensure that happens.
ran_task = true;
}
// DoDelayedWork always follows DoWork, (although the inverse is not true) so
// we don't need to schedule a delayed wakeup here.
WorkDeduplicator::NextTask next_task =
ran_task ? WorkDeduplicator::NextTask::kIsImmediate
: WorkDeduplicator::NextTask::kIsDelayed;
return work_deduplicator_.DidCheckForMoreWork(next_task) ==
ShouldScheduleWork::kScheduleImmediate;
}
bool ThreadControllerWithMessagePumpImpl::DoDelayedWork(
TimeTicks* next_run_time) {
// Nested runloops are covered by the parent loop hang watch scope.
// TODO(crbug/1034046): Provide more granular scoping that reuses the parent
// scope deadline.
if (main_thread_only().runloop_count == 1) {
hang_watch_scope_.emplace(base::HangWatchScope::kDefaultHangWatchTime);
}
work_deduplicator_.OnDelayedWorkStarted();
LazyNow continuation_lazy_now(time_source_);
bool ran_task = false;
WorkDeduplicator::NextTask next_task = WorkDeduplicator::NextTask::kIsDelayed;
TimeDelta delay_till_next_task =
DoWorkImpl(&continuation_lazy_now, &ran_task);
// Schedule a continuation.
// TODO(altimin, gab): Make this more efficient by merging DoWork
// and DoDelayedWork and allowing returning base::TimeTicks() when we have
// immediate work.
if (delay_till_next_task.is_zero()) {
*next_run_time = TimeTicks();
next_task = WorkDeduplicator::NextTask::kIsImmediate;
} else if (delay_till_next_task != TimeDelta::Max()) {
// Cancels any previously scheduled delayed wake-ups.
*next_run_time =
CapAtOneDay(delay_till_next_task + continuation_lazy_now.Now(),
&continuation_lazy_now);
// Don't request a run time past |main_thread_only().quit_runloop_after|.
if (*next_run_time > main_thread_only().quit_runloop_after) {
*next_run_time = main_thread_only().quit_runloop_after;
// If we've passed |quit_runloop_after| there's no more work to do.
if (continuation_lazy_now.Now() >= main_thread_only().quit_runloop_after)
*next_run_time = TimeTicks();
}
// The MessagePump will call ScheduleDelayedWork on our behalf, so we need
// to update |main_thread_only().next_delayed_do_work|.
main_thread_only().next_delayed_do_work = *next_run_time;
} else {
// There's no more work to do.
*next_run_time = TimeTicks();
}
// Figure out if we need to post an immediate continuation.
if (work_deduplicator_.OnDelayedWorkEnded(next_task) ==
ShouldScheduleWork::kScheduleImmediate) {
pump_->ScheduleWork();
}
return ran_task;
}
TimeDelta ThreadControllerWithMessagePumpImpl::DoWorkImpl(
LazyNow* continuation_lazy_now,
bool* ran_task) {
......
base/task/sequence_manager/thread_controller_with_message_pump_impl.h
View file @ b61fcc11
......@@ -87,8 +87,6 @@ class BASE_EXPORT ThreadControllerWithMessagePumpImpl
void BeforeDoInternalWork() override;
void BeforeWait() override;
MessagePump::Delegate::NextWorkInfo DoSomeWork() override;
bool DoWork() override;
bool DoDelayedWork(TimeTicks* next_run_time) override;
bool DoIdleWork() override;
// RunLoop::Delegate implementation.
......
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