diff options
-rw-r--r-- | base/message_loop_unittest.cc | 54 | ||||
-rw-r--r-- | base/message_pump.h | 12 | ||||
-rw-r--r-- | base/message_pump_mac.h | 19 | ||||
-rw-r--r-- | base/message_pump_mac.mm | 111 |
4 files changed, 101 insertions, 95 deletions
diff --git a/base/message_loop_unittest.cc b/base/message_loop_unittest.cc index 9576a58..2869bb8 100644 --- a/base/message_loop_unittest.cc +++ b/base/message_loop_unittest.cc @@ -768,6 +768,18 @@ class RecursiveTask : public OrderedTasks { bool is_reentrant_; }; +class RecursiveSlowTask : public RecursiveTask { + public: + RecursiveSlowTask(int depth, TaskList* order, int cookie, bool is_reentrant) + : RecursiveTask(depth, order, cookie, is_reentrant) { + } + + virtual void Run() { + RecursiveTask::Run(); + PlatformThread::Sleep(10); // milliseconds + } +}; + class QuitTask : public OrderedTasks { public: QuitTask(TaskList* order, int cookie) @@ -893,6 +905,42 @@ void RunTest_RecursiveDenial1(MessageLoop::Type message_loop_type) { EXPECT_EQ(order[13], TaskItem(RECURSIVE, 2, false)); } +void RunTest_RecursiveDenial3(MessageLoop::Type message_loop_type) { + MessageLoop loop(message_loop_type); + + EXPECT_TRUE(MessageLoop::current()->NestableTasksAllowed()); + TaskList order; + MessageLoop::current()->PostTask(FROM_HERE, + new RecursiveSlowTask(2, &order, 1, false)); + MessageLoop::current()->PostTask(FROM_HERE, + new RecursiveSlowTask(2, &order, 2, false)); + MessageLoop::current()->PostDelayedTask(FROM_HERE, + new OrderedTasks(&order, 3), 5); + MessageLoop::current()->PostDelayedTask(FROM_HERE, + new QuitTask(&order, 4), 5); + + MessageLoop::current()->Run(); + + // FIFO order. + ASSERT_EQ(16U, order.size()); + EXPECT_EQ(order[ 0], TaskItem(RECURSIVE, 1, true)); + EXPECT_EQ(order[ 1], TaskItem(RECURSIVE, 1, false)); + EXPECT_EQ(order[ 2], TaskItem(RECURSIVE, 2, true)); + EXPECT_EQ(order[ 3], TaskItem(RECURSIVE, 2, false)); + EXPECT_EQ(order[ 4], TaskItem(RECURSIVE, 1, true)); + EXPECT_EQ(order[ 5], TaskItem(RECURSIVE, 1, false)); + EXPECT_EQ(order[ 6], TaskItem(ORDERERD, 3, true)); + EXPECT_EQ(order[ 7], TaskItem(ORDERERD, 3, false)); + EXPECT_EQ(order[ 8], TaskItem(RECURSIVE, 2, true)); + EXPECT_EQ(order[ 9], TaskItem(RECURSIVE, 2, false)); + EXPECT_EQ(order[10], TaskItem(QUITMESSAGELOOP, 4, true)); + EXPECT_EQ(order[11], TaskItem(QUITMESSAGELOOP, 4, false)); + EXPECT_EQ(order[12], TaskItem(RECURSIVE, 1, true)); + EXPECT_EQ(order[13], TaskItem(RECURSIVE, 1, false)); + EXPECT_EQ(order[14], TaskItem(RECURSIVE, 2, true)); + EXPECT_EQ(order[15], TaskItem(RECURSIVE, 2, false)); +} + void RunTest_RecursiveSupport1(MessageLoop::Type message_loop_type) { MessageLoop loop(message_loop_type); @@ -1426,6 +1474,12 @@ TEST(MessageLoopTest, RecursiveDenial1) { RunTest_RecursiveDenial1(MessageLoop::TYPE_IO); } +TEST(MessageLoopTest, RecursiveDenial3) { + RunTest_RecursiveDenial3(MessageLoop::TYPE_DEFAULT); + RunTest_RecursiveDenial3(MessageLoop::TYPE_UI); + RunTest_RecursiveDenial3(MessageLoop::TYPE_IO); +} + TEST(MessageLoopTest, RecursiveSupport1) { RunTest_RecursiveSupport1(MessageLoop::TYPE_DEFAULT); RunTest_RecursiveSupport1(MessageLoop::TYPE_UI); diff --git a/base/message_pump.h b/base/message_pump.h index a354724..866b33a 100644 --- a/base/message_pump.h +++ b/base/message_pump.h @@ -22,7 +22,8 @@ class MessagePump : public RefCountedThreadSafe<MessagePump> { // Called from within Run in response to ScheduleWork or when the message // pump would otherwise call DoDelayedWork. Returns true to indicate that - // work was done. DoDelayedWork will not be called if DoWork returns true. + // work was done. DoDelayedWork will still be called if DoWork returns + // true, but DoIdleWork will not. virtual bool DoWork() = 0; // Called from within Run in response to ScheduleDelayedWork or when the @@ -61,7 +62,8 @@ class MessagePump : public RefCountedThreadSafe<MessagePump> { // if (should_quit_) // break; // - // did_work |= delegate_->DoDelayedWork(); + // TimeTicks next_time; + // did_work |= delegate_->DoDelayedWork(&next_time); // if (should_quit_) // break; // @@ -84,9 +86,9 @@ class MessagePump : public RefCountedThreadSafe<MessagePump> { // blocks until there is more work of any type to do. // // Notice that the run loop cycles between calling DoInternalWork, DoWork, - // and DoDelayedWork methods. This helps ensure that neither work queue - // starves the other. This is important for message pumps that are used to - // drive animations, for example. + // and DoDelayedWork methods. This helps ensure that none of these work + // queues starve the others. This is important for message pumps that are + // used to drive animations, for example. // // Notice also that after each callout to foreign code, the run loop checks // to see if it should quit. The Quit method is responsible for setting this diff --git a/base/message_pump_mac.h b/base/message_pump_mac.h index c30a8ea..b903a1a 100644 --- a/base/message_pump_mac.h +++ b/base/message_pump_mac.h @@ -86,24 +86,17 @@ class MessagePumpCFRunLoopBase : public MessagePump { private: // Timer callback scheduled by ScheduleDelayedWork. This does not do any - // work, but it signals delayed_work_source_ so that delayed work can be - // performed within the appropriate priority constraints. + // work, but it signals work_source_ so that delayed work can be performed + // within the appropriate priority constraints. static void RunDelayedWorkTimer(CFRunLoopTimerRef timer, void* info); // Perform highest-priority work. This is associated with work_source_ - // signalled by ScheduleWork. The static method calls the instance method; - // the instance method returns true if work was done. + // signalled by ScheduleWork or RunDelayedWorkTimer. The static method calls + // the instance method; the instance method returns true if it resignalled + // work_source_ to be called again from the loop. static void RunWorkSource(void* info); bool RunWork(); - // Perform delayed-priority work. This is associated with - // delayed_work_source_ signalled by RunDelayedWorkTimer, and is responsible - // for calling ScheduleDelayedWork again if appropriate. The static method - // calls the instance method; the instance method returns true if more - // delayed work is available. - static void RunDelayedWorkSource(void* info); - bool RunDelayedWork(); - // Perform idle-priority work. This is normally called by PreWaitObserver, // but is also associated with idle_work_source_. When this function // actually does perform idle work, it will resignal that source. The @@ -162,7 +155,6 @@ class MessagePumpCFRunLoopBase : public MessagePump { // callbacks. CFRunLoopTimerRef delayed_work_timer_; CFRunLoopSourceRef work_source_; - CFRunLoopSourceRef delayed_work_source_; CFRunLoopSourceRef idle_work_source_; CFRunLoopSourceRef nesting_deferred_work_source_; CFRunLoopObserverRef pre_wait_observer_; @@ -202,7 +194,6 @@ class MessagePumpCFRunLoopBase : public MessagePump { // any call to Run on the stack. The Run method will check for delegateless // work on entry and redispatch it as needed once a delegate is available. bool delegateless_work_; - bool delegateless_delayed_work_; bool delegateless_idle_work_; DISALLOW_COPY_AND_ASSIGN(MessagePumpCFRunLoopBase); diff --git a/base/message_pump_mac.mm b/base/message_pump_mac.mm index 8c5461c..b9f188e 100644 --- a/base/message_pump_mac.mm +++ b/base/message_pump_mac.mm @@ -51,7 +51,6 @@ MessagePumpCFRunLoopBase::MessagePumpCFRunLoopBase() run_nesting_level_(0), deepest_nesting_level_(0), delegateless_work_(false), - delegateless_delayed_work_(false), delegateless_idle_work_(false) { run_loop_ = CFRunLoopGetCurrent(); CFRetain(run_loop_); @@ -78,15 +77,9 @@ MessagePumpCFRunLoopBase::MessagePumpCFRunLoopBase() &source_context); CFRunLoopAddSource(run_loop_, work_source_, kCFRunLoopCommonModes); - source_context.perform = RunDelayedWorkSource; - delayed_work_source_ = CFRunLoopSourceCreate(NULL, // allocator - 2, // priority - &source_context); - CFRunLoopAddSource(run_loop_, delayed_work_source_, kCFRunLoopCommonModes); - source_context.perform = RunIdleWorkSource; idle_work_source_ = CFRunLoopSourceCreate(NULL, // allocator - 3, // priority + 2, // priority &source_context); CFRunLoopAddSource(run_loop_, idle_work_source_, kCFRunLoopCommonModes); @@ -169,9 +162,6 @@ MessagePumpCFRunLoopBase::~MessagePumpCFRunLoopBase() { CFRunLoopRemoveSource(run_loop_, idle_work_source_, kCFRunLoopCommonModes); CFRelease(idle_work_source_); - CFRunLoopRemoveSource(run_loop_, delayed_work_source_, kCFRunLoopCommonModes); - CFRelease(delayed_work_source_); - CFRunLoopRemoveSource(run_loop_, work_source_, kCFRunLoopCommonModes); CFRelease(work_source_); @@ -199,10 +189,6 @@ void MessagePumpCFRunLoopBase::Run(Delegate* delegate) { CFRunLoopSourceSignal(work_source_); delegateless_work_ = false; } - if (delegateless_delayed_work_) { - CFRunLoopSourceSignal(delayed_work_source_); - delegateless_delayed_work_ = false; - } if (delegateless_idle_work_) { CFRunLoopSourceSignal(idle_work_source_); delegateless_idle_work_ = false; @@ -260,11 +246,10 @@ void MessagePumpCFRunLoopBase::RunDelayedWorkTimer(CFRunLoopTimerRef timer, self->delayed_work_fire_time_ = kCFTimeIntervalMax; // CFRunLoopTimers fire outside of the priority scheme for CFRunLoopSources. - // In order to establish the proper priority where delegate_->DoDelayedWork - // can only be called if delegate_->DoWork returns false, the timer used - // to schedule delayed work must signal a CFRunLoopSource set at a lower - // priority than the one used for delegate_->DoWork. - CFRunLoopSourceSignal(self->delayed_work_source_); + // In order to establish the proper priority in which work and delayed work + // are processed one for one, the timer used to schedule delayed work must + // signal a CFRunLoopSource used to dispatch both work and delayed work. + CFRunLoopSourceSignal(self->work_source_); } // Called from the run loop. @@ -291,58 +276,39 @@ bool MessagePumpCFRunLoopBase::RunWork() { // released promptly even in the absence of UI events. MessagePumpScopedAutoreleasePool autorelease_pool(this); - // Call DoWork once, and if something was done, arrange to come back here - // again as long as the loop is still running. + // Call DoWork and DoDelayedWork once, and if something was done, arrange to + // come back here again as long as the loop is still running. bool did_work = delegate_->DoWork(); - if (did_work) { - CFRunLoopSourceSignal(work_source_); - } - - return did_work; -} - -// Called from the run loop. -// static -void MessagePumpCFRunLoopBase::RunDelayedWorkSource(void* info) { - MessagePumpCFRunLoopBase* self = static_cast<MessagePumpCFRunLoopBase*>(info); - self->RunDelayedWork(); -} - -// Called by MessagePumpCFRunLoopBase::RunDelayedWorkSource. -bool MessagePumpCFRunLoopBase::RunDelayedWork() { - if (!delegate_) { - // This point can be reached with a NULL delegate_ if Run is not on the - // stack but foreign code is spinning the CFRunLoop. Arrange to come back - // here when a delegate is available. - delegateless_delayed_work_ = true; - return false; - } - - // The NSApplication-based run loop only drains the autorelease pool at each - // UI event (NSEvent). The autorelease pool is not drained for each - // CFRunLoopSource target that's run. Use a local pool for any autoreleased - // objects if the app is not currently handling a UI event to ensure they're - // released promptly even in the absence of UI events. - MessagePumpScopedAutoreleasePool autorelease_pool(this); + bool resignal_work_source = did_work; TimeTicks next_time; delegate_->DoDelayedWork(&next_time); - - bool more_work = !next_time.is_null(); - if (more_work) { - TimeDelta delay = next_time - TimeTicks::Now(); - if (delay > TimeDelta()) { - // There's more delayed work to be done in the future. - ScheduleDelayedWork(next_time); - } else { - // There's more delayed work to be done, and its time is in the past. - // Arrange to come back here directly as long as the loop is still - // running. - CFRunLoopSourceSignal(delayed_work_source_); + if (!did_work) { + // Determine whether there's more delayed work, and if so, if it needs to + // be done at some point in the future or if it's already time to do it. + // Only do these checks if did_work is false. If did_work is true, this + // function, and therefore any additional delayed work, will get another + // chance to run before the loop goes to sleep. + bool more_delayed_work = !next_time.is_null(); + if (more_delayed_work) { + TimeDelta delay = next_time - TimeTicks::Now(); + if (delay > TimeDelta()) { + // There's more delayed work to be done in the future. + ScheduleDelayedWork(next_time); + } else { + // There's more delayed work to be done, and its time is in the past. + // Arrange to come back here directly as long as the loop is still + // running. + resignal_work_source = true; + } } } - return more_work; + if (resignal_work_source) { + CFRunLoopSourceSignal(work_source_); + } + + return resignal_work_source; } // Called from the run loop. @@ -398,19 +364,12 @@ bool MessagePumpCFRunLoopBase::RunNestingDeferredWork() { // Immediately try work in priority order. if (!RunWork()) { - if (!RunDelayedWork()) { - if (!RunIdleWork()) { - return false; - } - } else { - // There was no work, and delayed work was done. Arrange for the loop - // to try non-nestable idle work on a subsequent pass. - CFRunLoopSourceSignal(idle_work_source_); + if (!RunIdleWork()) { + return false; } } else { - // Work was done. Arrange for the loop to try non-nestable delayed and - // idle work on a subsequent pass. - CFRunLoopSourceSignal(delayed_work_source_); + // Work was done. Arrange for the loop to try non-nestable idle work on + // a subsequent pass. CFRunLoopSourceSignal(idle_work_source_); } |