diff --git a/webrtc/base/messagequeue.cc b/webrtc/base/messagequeue.cc index e5d69a324f..74b1024deb 100644 --- a/webrtc/base/messagequeue.cc +++ b/webrtc/base/messagequeue.cc @@ -361,6 +361,7 @@ void MessageQueue::Clear(MessageHandler *phandler, uint32 id, } // Remove from priority queue. Not directly iterable, so use this approach + PriorityQueue::container_type::iterator new_end = dmsgq_.container().begin(); for (PriorityQueue::container_type::iterator it = new_end; it != dmsgq_.container().end(); ++it) { @@ -371,10 +372,7 @@ void MessageQueue::Clear(MessageHandler *phandler, uint32 id, delete it->msg_.pdata; } } else { - if (new_end != it) { - *new_end = *it; - } - new_end++; + *new_end++ = *it; } } dmsgq_.container().erase(new_end, dmsgq_.container().end()); diff --git a/webrtc/base/messagequeue_unittest.cc b/webrtc/base/messagequeue_unittest.cc index ac9affece5..871542df28 100644 --- a/webrtc/base/messagequeue_unittest.cc +++ b/webrtc/base/messagequeue_unittest.cc @@ -20,7 +20,7 @@ using namespace rtc; -class MessageQueueForTest : public MessageQueue { +class MessageQueueTest: public testing::Test, public MessageQueue { public: bool IsLocked_Worker() { if (!crit_.TryEnter()) { @@ -29,38 +29,24 @@ class MessageQueueForTest : public MessageQueue { crit_.Leave(); return false; } - bool IsLocked() { // We have to do this on a worker thread, or else the TryEnter will // succeed, since our critical sections are reentrant. Thread worker; worker.Start(); return worker.Invoke( - rtc::Bind(&MessageQueueForTest::IsLocked_Worker, this)); + rtc::Bind(&MessageQueueTest::IsLocked_Worker, this)); } - - size_t GetDmsgqSize() { - return dmsgq_.size(); - } - - const DelayedMessage& GetDmsgqTop() { - return dmsgq_.top(); - } -}; - -class MessageQueueTest : public testing::Test { - protected: - MessageQueueForTest q_; }; struct DeletedLockChecker { - DeletedLockChecker(MessageQueueForTest* q, bool* was_locked, bool* deleted) - : q_(q), was_locked(was_locked), deleted(deleted) { } + DeletedLockChecker(MessageQueueTest* test, bool* was_locked, bool* deleted) + : test(test), was_locked(was_locked), deleted(deleted) { } ~DeletedLockChecker() { *deleted = true; - *was_locked = q_->IsLocked(); + *was_locked = test->IsLocked(); } - MessageQueueForTest* q_; + MessageQueueTest* test; bool* was_locked; bool* deleted; }; @@ -87,7 +73,8 @@ static void DelayedPostsWithIdenticalTimesAreProcessedInFifoOrder( TEST_F(MessageQueueTest, DelayedPostsWithIdenticalTimesAreProcessedInFifoOrder) { - DelayedPostsWithIdenticalTimesAreProcessedInFifoOrder(&q_); + MessageQueue q; + DelayedPostsWithIdenticalTimesAreProcessedInFifoOrder(&q); NullSocketServer nullss; MessageQueue q_nullss(&nullss); DelayedPostsWithIdenticalTimesAreProcessedInFifoOrder(&q_nullss); @@ -96,10 +83,10 @@ TEST_F(MessageQueueTest, TEST_F(MessageQueueTest, DisposeNotLocked) { bool was_locked = true; bool deleted = false; - DeletedLockChecker* d = new DeletedLockChecker(&q_, &was_locked, &deleted); - q_.Dispose(d); + DeletedLockChecker* d = new DeletedLockChecker(this, &was_locked, &deleted); + Dispose(d); Message msg; - EXPECT_FALSE(q_.Get(&msg, 0)); + EXPECT_FALSE(Get(&msg, 0)); EXPECT_TRUE(deleted); EXPECT_FALSE(was_locked); } @@ -115,138 +102,18 @@ class DeletedMessageHandler : public MessageHandler { bool* deleted_; }; -// TODO(decurtis): Test that ordering of elements is done properly. -// TODO(decurtis): Test that timestamps are being properly set. - -TEST_F(MessageQueueTest, DisposeHandlerWithPostedMessagePending) { +TEST_F(MessageQueueTest, DiposeHandlerWithPostedMessagePending) { bool deleted = false; DeletedMessageHandler *handler = new DeletedMessageHandler(&deleted); // First, post a dispose. - q_.Dispose(handler); + Dispose(handler); // Now, post a message, which should *not* be returned by Get(). - q_.Post(handler, 1); + Post(handler, 1); Message msg; - EXPECT_FALSE(q_.Get(&msg, 0)); + EXPECT_FALSE(Get(&msg, 0)); EXPECT_TRUE(deleted); } -// Test Clear for removing messages that have been posted for times in -// the past. -TEST_F(MessageQueueTest, ClearPast) { - TimeStamp now = Time(); - Message msg; - - // Test removing the only element. - q_.PostAt(now - 4, NULL, 1); - q_.Clear(NULL, 1, NULL); - - // Make sure the queue is empty now. - EXPECT_FALSE(q_.Get(&msg, 0)); - - // Test removing the one element with a two element list. - q_.PostAt(now - 4, NULL, 1); - q_.PostAt(now - 2, NULL, 3); - - q_.Clear(NULL, 1, NULL); - - EXPECT_TRUE(q_.Get(&msg, 0)); - EXPECT_EQ(3U, msg.message_id); - - // Make sure the queue is empty now. - EXPECT_FALSE(q_.Get(&msg, 0)); - - - // Test removing the three element with a two element list. - q_.PostAt(now - 4, NULL, 1); - q_.PostAt(now - 2, NULL, 3); - - q_.Clear(NULL, 3, NULL); - - EXPECT_TRUE(q_.Get(&msg, 0)); - EXPECT_EQ(1U, msg.message_id); - - // Make sure the queue is empty now. - EXPECT_FALSE(q_.Get(&msg, 0)); - - - // Test removing the two element in a three element list. - q_.PostAt(now - 4, NULL, 1); - q_.PostAt(now - 3, NULL, 2); - q_.PostAt(now - 2, NULL, 3); - - q_.Clear(NULL, 2, NULL); - - EXPECT_TRUE(q_.Get(&msg, 0)); - EXPECT_EQ(1U, msg.message_id); - - EXPECT_TRUE(q_.Get(&msg, 0)); - EXPECT_EQ(3U, msg.message_id); - - // Make sure the queue is empty now. - EXPECT_FALSE(q_.Get(&msg, 0)); - - - // Test not clearing any messages. - q_.PostAt(now - 4, NULL, 1); - q_.PostAt(now - 3, NULL, 2); - q_.PostAt(now - 2, NULL, 3); - - // Remove nothing. - q_.Clear(NULL, 0, NULL); - q_.Clear(NULL, 4, NULL); - - EXPECT_TRUE(q_.Get(&msg, 0)); - EXPECT_EQ(1U, msg.message_id); - - EXPECT_TRUE(q_.Get(&msg, 0)); - EXPECT_EQ(2U, msg.message_id); - - EXPECT_TRUE(q_.Get(&msg, 0)); - EXPECT_EQ(3U, msg.message_id); - - // Make sure the queue is empty now. - EXPECT_FALSE(q_.Get(&msg, 0)); -} - -// Test clearing messages that have been posted for the future. -TEST_F(MessageQueueTest, ClearFuture) { - EXPECT_EQ(0U, q_.GetDmsgqSize()); - q_.PostDelayed(10, NULL, 4); - EXPECT_EQ(1U, q_.GetDmsgqSize()); - q_.PostDelayed(13, NULL, 4); - EXPECT_EQ(2U, q_.GetDmsgqSize()); - q_.PostDelayed(9, NULL, 2); - EXPECT_EQ(3U, q_.GetDmsgqSize()); - q_.PostDelayed(11, NULL, 10); - EXPECT_EQ(4U, q_.GetDmsgqSize()); - - EXPECT_EQ(9, q_.GetDmsgqTop().cmsDelay_); - - MessageList removed; - q_.Clear(NULL, 10, &removed); - EXPECT_EQ(1U, removed.size()); - EXPECT_EQ(3U, q_.GetDmsgqSize()); - - removed.clear(); - q_.Clear(NULL, 4, &removed); - EXPECT_EQ(2U, removed.size()); - EXPECT_EQ(1U, q_.GetDmsgqSize()); - - removed.clear(); - q_.Clear(NULL, 4, &removed); - EXPECT_EQ(0U, removed.size()); - EXPECT_EQ(1U, q_.GetDmsgqSize()); - - removed.clear(); - q_.Clear(NULL, 2, &removed); - EXPECT_EQ(1U, removed.size()); - EXPECT_EQ(0U, q_.GetDmsgqSize()); - - Message msg; - EXPECT_FALSE(q_.Get(&msg, 0)); -} - - struct UnwrapMainThreadScope { UnwrapMainThreadScope() : rewrap_(Thread::Current() != NULL) { if (rewrap_) ThreadManager::Instance()->UnwrapCurrentThread(); @@ -258,7 +125,7 @@ struct UnwrapMainThreadScope { bool rewrap_; }; -TEST(MessageQueueManager, DeletedHandler) { +TEST(MessageQueueManager, Clear) { UnwrapMainThreadScope s; if (MessageQueueManager::IsInitialized()) { LOG(LS_INFO) << "Unable to run MessageQueueManager::Clear test, since the "