Revert "When clearing the priority message queue, don't copy an item to itself."
This reverts commit 2bffc3cb72e2250cbf6ed7e5f4b399395ca046cb.
BUG=4100
R=juberti@webrtc.org,pthatcher@webrtc.org
TBR=juberti@webrtc.org,pthatcher@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/38189004
Cr-Original-Commit-Position: refs/heads/master@{#8450}
Cr-Mirrored-From: https://chromium.googlesource.com/external/webrtc
Cr-Mirrored-Commit: 2af3057b240526c9e769afc9147cfb532c07ad67
diff --git a/base/messagequeue.cc b/base/messagequeue.cc
index e5d69a3..74b1024 100644
--- a/base/messagequeue.cc
+++ b/base/messagequeue.cc
@@ -361,6 +361,7 @@
}
// 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 @@
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/base/messagequeue_unittest.cc b/base/messagequeue_unittest.cc
index ac9affe..871542d 100644
--- a/base/messagequeue_unittest.cc
+++ b/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 @@
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<bool>(
- 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 @@
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, 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 @@
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 @@
bool rewrap_;
};
-TEST(MessageQueueManager, DeletedHandler) {
+TEST(MessageQueueManager, Clear) {
UnwrapMainThreadScope s;
if (MessageQueueManager::IsInitialized()) {
LOG(LS_INFO) << "Unable to run MessageQueueManager::Clear test, since the "
