generic-library/webrtc
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Revert "When clearing the priority message queue, don't copy an item to itself."

Browse Source 
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-Commit-Position: refs/heads/master@{#8450}
git-svn-id: http://webrtc.googlecode.com/svn/trunk@8450 4adac7df-926f-26a2-2b94-8c16560cd09d
This commit is contained in:
decurtis@webrtc.org 2015-02-21 01:59:50 +00:00
parent 2bffc3cb72
commit 2af3057b24
2 changed files with 18 additions and 153 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
webrtc/base/messagequeue.cc
View File

@ -361,6 +361,7 @@ void MessageQueue::Clear(MessageHandler *phandler, uint32 id,
} }
// Remove from priority queue. Not directly iterable, so use this approach // Remove from priority queue. Not directly iterable, so use this approach
PriorityQueue::container_type::iterator new_end = dmsgq_.container().begin(); PriorityQueue::container_type::iterator new_end = dmsgq_.container().begin();
for (PriorityQueue::container_type::iterator it = new_end; for (PriorityQueue::container_type::iterator it = new_end;
it != dmsgq_.container().end(); ++it) { it != dmsgq_.container().end(); ++it) {
@ -371,10 +372,7 @@ void MessageQueue::Clear(MessageHandler *phandler, uint32 id,
delete it->msg_.pdata; delete it->msg_.pdata;
} }
} else { } else {
if (new_end != it) { *new_end++ = *it;
*new_end = *it;
}
new_end++;
} }
} }
dmsgq_.container().erase(new_end, dmsgq_.container().end()); dmsgq_.container().erase(new_end, dmsgq_.container().end());

165
webrtc/base/messagequeue_unittest.cc
View File

@ -20,7 +20,7 @@
using namespace rtc; using namespace rtc;
class MessageQueueForTest : public MessageQueue { class MessageQueueTest: public testing::Test, public MessageQueue {
public: public:
bool IsLocked_Worker() { bool IsLocked_Worker() {
if (!crit_.TryEnter()) { if (!crit_.TryEnter()) {
@ -29,38 +29,24 @@ class MessageQueueForTest : public MessageQueue {
crit_.Leave(); crit_.Leave();
return false; return false;
} }
bool IsLocked() { bool IsLocked() {
// We have to do this on a worker thread, or else the TryEnter will // We have to do this on a worker thread, or else the TryEnter will
// succeed, since our critical sections are reentrant. // succeed, since our critical sections are reentrant.
Thread worker; Thread worker;
worker.Start(); worker.Start();
return worker.Invoke<bool>( 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 { struct DeletedLockChecker {
DeletedLockChecker(MessageQueueForTest* q, bool* was_locked, bool* deleted) DeletedLockChecker(MessageQueueTest* test, bool* was_locked, bool* deleted)
: q_(q), was_locked(was_locked), deleted(deleted) { } : test(test), was_locked(was_locked), deleted(deleted) { }
~DeletedLockChecker() { ~DeletedLockChecker() {
*deleted = true; *deleted = true;
*was_locked = q_->IsLocked(); *was_locked = test->IsLocked();
} }
MessageQueueForTest* q_; MessageQueueTest* test;
bool* was_locked; bool* was_locked;
bool* deleted; bool* deleted;
}; };
@ -87,7 +73,8 @@ static void DelayedPostsWithIdenticalTimesAreProcessedInFifoOrder(
TEST_F(MessageQueueTest, TEST_F(MessageQueueTest,
DelayedPostsWithIdenticalTimesAreProcessedInFifoOrder) { DelayedPostsWithIdenticalTimesAreProcessedInFifoOrder) {
DelayedPostsWithIdenticalTimesAreProcessedInFifoOrder(&q_); MessageQueue q;
DelayedPostsWithIdenticalTimesAreProcessedInFifoOrder(&q);
NullSocketServer nullss; NullSocketServer nullss;
MessageQueue q_nullss(&nullss); MessageQueue q_nullss(&nullss);
DelayedPostsWithIdenticalTimesAreProcessedInFifoOrder(&q_nullss); DelayedPostsWithIdenticalTimesAreProcessedInFifoOrder(&q_nullss);
@ -96,10 +83,10 @@ TEST_F(MessageQueueTest,
TEST_F(MessageQueueTest, DisposeNotLocked) { TEST_F(MessageQueueTest, DisposeNotLocked) {
bool was_locked = true; bool was_locked = true;
bool deleted = false; bool deleted = false;
DeletedLockChecker* d = new DeletedLockChecker(&q_, &was_locked, &deleted); DeletedLockChecker* d = new DeletedLockChecker(this, &was_locked, &deleted);
q_.Dispose(d); Dispose(d);
Message msg; Message msg;
EXPECT_FALSE(q_.Get(&msg, 0)); EXPECT_FALSE(Get(&msg, 0));
EXPECT_TRUE(deleted); EXPECT_TRUE(deleted);
EXPECT_FALSE(was_locked); EXPECT_FALSE(was_locked);
} }
@ -115,138 +102,18 @@ class DeletedMessageHandler : public MessageHandler {
bool* deleted_; bool* deleted_;
}; };
// TODO(decurtis): Test that ordering of elements is done properly. TEST_F(MessageQueueTest, DiposeHandlerWithPostedMessagePending) {
// TODO(decurtis): Test that timestamps are being properly set.
TEST_F(MessageQueueTest, DisposeHandlerWithPostedMessagePending) {
bool deleted = false; bool deleted = false;
DeletedMessageHandler *handler = new DeletedMessageHandler(&deleted); DeletedMessageHandler *handler = new DeletedMessageHandler(&deleted);
// First, post a dispose. // First, post a dispose.
q_.Dispose(handler); Dispose(handler);
// Now, post a message, which should *not* be returned by Get(). // Now, post a message, which should *not* be returned by Get().
q_.Post(handler, 1); Post(handler, 1);
Message msg; Message msg;
EXPECT_FALSE(q_.Get(&msg, 0)); EXPECT_FALSE(Get(&msg, 0));
EXPECT_TRUE(deleted); 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 { struct UnwrapMainThreadScope {
UnwrapMainThreadScope() : rewrap_(Thread::Current() != NULL) { UnwrapMainThreadScope() : rewrap_(Thread::Current() != NULL) {
if (rewrap_) ThreadManager::Instance()->UnwrapCurrentThread(); if (rewrap_) ThreadManager::Instance()->UnwrapCurrentThread();
@ -258,7 +125,7 @@ struct UnwrapMainThreadScope {
bool rewrap_; bool rewrap_;
}; };
TEST(MessageQueueManager, DeletedHandler) { TEST(MessageQueueManager, Clear) {
UnwrapMainThreadScope s; UnwrapMainThreadScope s;
if (MessageQueueManager::IsInitialized()) { if (MessageQueueManager::IsInitialized()) {
LOG(LS_INFO) << "Unable to run MessageQueueManager::Clear test, since the " LOG(LS_INFO) << "Unable to run MessageQueueManager::Clear test, since the "