Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(2949)

Unified Diff: base/message_loop/message_loop.cc

Issue 17567007: Made MessagePump a non-thread safe class. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: - Created 7 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: base/message_loop/message_loop.cc
diff --git a/base/message_loop/message_loop.cc b/base/message_loop/message_loop.cc
index 4e0c5f6801e9e15d36a5e7d220d0b956ff91f7a9..fe5fe194d21dde0e26d2170ce5947510084ed18f 100644
--- a/base/message_loop/message_loop.cc
+++ b/base/message_loop/message_loop.cc
@@ -167,7 +167,7 @@ MessageLoop::MessageLoop(Type type)
#define MESSAGE_PUMP_UI NULL
// ipc_channel_nacl.cc uses a worker thread to do socket reads currently, and
// doesn't require extra support for watching file descriptors.
-#define MESSAGE_PUMP_IO new MessagePumpDefault();
+#define MESSAGE_PUMP_IO new MessagePumpDefault()
#elif defined(OS_POSIX) // POSIX but not MACOSX.
#define MESSAGE_PUMP_UI new MessagePumpForUI()
#define MESSAGE_PUMP_IO new MessagePumpLibevent()
@@ -177,14 +177,14 @@ MessageLoop::MessageLoop(Type type)
if (type_ == TYPE_UI) {
if (message_pump_for_ui_factory_)
- pump_ = message_pump_for_ui_factory_();
+ pump_.reset(message_pump_for_ui_factory_());
else
- pump_ = MESSAGE_PUMP_UI;
+ pump_.reset(MESSAGE_PUMP_UI);
} else if (type_ == TYPE_IO) {
- pump_ = MESSAGE_PUMP_IO;
+ pump_.reset(MESSAGE_PUMP_IO);
} else {
DCHECK_EQ(TYPE_DEFAULT, type_);
- pump_ = new MessagePumpDefault();
+ pump_.reset(new MessagePumpDefault());
}
}
@@ -273,18 +273,14 @@ void MessageLoop::PostTask(
const tracked_objects::Location& from_here,
const Closure& task) {
DCHECK(!task.is_null()) << from_here.ToString();
- PendingTask pending_task(
- from_here, task, CalculateDelayedRuntime(TimeDelta()), true);
- AddToIncomingQueue(&pending_task, false);
+ message_loop_proxy_->AddToIncomingQueue(from_here, task, TimeDelta(), true);
}
bool MessageLoop::TryPostTask(
const tracked_objects::Location& from_here,
const Closure& task) {
DCHECK(!task.is_null()) << from_here.ToString();
- PendingTask pending_task(
- from_here, task, CalculateDelayedRuntime(TimeDelta()), true);
- return AddToIncomingQueue(&pending_task, true);
+ return message_loop_proxy_->TryAddToIncomingQueue(from_here, task);
}
void MessageLoop::PostDelayedTask(
@@ -292,18 +288,14 @@ void MessageLoop::PostDelayedTask(
const Closure& task,
TimeDelta delay) {
DCHECK(!task.is_null()) << from_here.ToString();
- PendingTask pending_task(
- from_here, task, CalculateDelayedRuntime(delay), true);
- AddToIncomingQueue(&pending_task, false);
+ message_loop_proxy_->AddToIncomingQueue(from_here, task, delay, true);
}
void MessageLoop::PostNonNestableTask(
const tracked_objects::Location& from_here,
const Closure& task) {
DCHECK(!task.is_null()) << from_here.ToString();
- PendingTask pending_task(
- from_here, task, CalculateDelayedRuntime(TimeDelta()), false);
- AddToIncomingQueue(&pending_task, false);
+ message_loop_proxy_->AddToIncomingQueue(from_here, task, TimeDelta(), false);
}
void MessageLoop::PostNonNestableDelayedTask(
@@ -311,9 +303,7 @@ void MessageLoop::PostNonNestableDelayedTask(
const Closure& task,
TimeDelta delay) {
DCHECK(!task.is_null()) << from_here.ToString();
- PendingTask pending_task(
- from_here, task, CalculateDelayedRuntime(delay), false);
- AddToIncomingQueue(&pending_task, false);
+ message_loop_proxy_->AddToIncomingQueue(from_here, task, delay, false);
}
void MessageLoop::Run() {
@@ -357,6 +347,10 @@ Closure MessageLoop::QuitWhenIdleClosure() {
return Bind(&QuitCurrentWhenIdle);
}
+scoped_refptr<MessageLoopProxy> MessageLoop::message_loop_proxy() {
rvargas (doing something else) 2013/06/28 03:00:51 why de-inline this?
alexeypa (please no reviews) 2013/06/28 17:00:57 "base/message_loop/message_loop_proxy_impl.h" cann
+ return message_loop_proxy_;
+}
+
void MessageLoop::SetNestableTasksAllowed(bool allowed) {
if (nestable_tasks_allowed_ != allowed) {
nestable_tasks_allowed_ = allowed;
@@ -387,7 +381,7 @@ void MessageLoop::RemoveTaskObserver(TaskObserver* task_observer) {
void MessageLoop::AssertIdle() const {
rvargas (doing something else) 2013/06/28 03:00:51 This should probably be a ForTest method.
alexeypa (please no reviews) 2013/06/28 17:00:57 Done.
// We only check |incoming_queue_|, since we don't want to lock |work_queue_|.
- AutoLock lock(incoming_queue_lock_);
+ AutoLock lock(message_loop_proxy_->message_loop_lock_);
rvargas (doing something else) 2013/06/28 03:00:51 we should do something to avoid grabbing this lock
alexeypa (please no reviews) 2013/06/28 17:00:57 Done.
DCHECK(incoming_queue_.empty());
}
@@ -520,7 +514,7 @@ void MessageLoop::ReloadWorkQueue() {
// Acquire all we can from the inter-thread queue with one lock acquisition.
{
- AutoLock lock(incoming_queue_lock_);
+ AutoLock lock(message_loop_proxy_->message_loop_lock_);
rvargas (doing something else) 2013/06/28 03:00:51 Maybe message_loop_proxy_->LoadPendingTasks(); M
alexeypa (please no reviews) 2013/06/28 17:00:57 Done.
if (incoming_queue_.empty())
return;
incoming_queue_.Swap(&work_queue_); // Constant time
@@ -596,46 +590,35 @@ TimeTicks MessageLoop::CalculateDelayedRuntime(TimeDelta delay) {
}
// Possibly called on a background thread!
-bool MessageLoop::AddToIncomingQueue(PendingTask* pending_task,
- bool use_try_lock) {
+void MessageLoop::AddToIncomingQueue(const tracked_objects::Location& from_here,
+ const Closure& task,
+ TimeDelta delay,
+ bool nestable) {
// Warning: Don't try to short-circuit, and handle this thread's tasks more
// directly, as it could starve handling of foreign threads. Put every task
// into this queue.
- scoped_refptr<MessagePump> pump;
- {
- if (use_try_lock) {
- if (!incoming_queue_lock_.Try()) {
- pending_task->task.Reset();
- return false;
- }
- } else {
- incoming_queue_lock_.Acquire();
- }
- AutoLock locked(incoming_queue_lock_, AutoLock::AlreadyAcquired());
- // Initialize the sequence number. The sequence number is used for delayed
- // tasks (to faciliate FIFO sorting when two tasks have the same
- // delayed_run_time value) and for identifying the task in about:tracing.
- pending_task->sequence_num = next_sequence_num_++;
-
- TRACE_EVENT_FLOW_BEGIN0("task", "MessageLoop::PostTask",
- TRACE_ID_MANGLE(GetTaskTraceID(*pending_task, this)));
-
- bool was_empty = incoming_queue_.empty();
- incoming_queue_.push(*pending_task);
- pending_task->task.Reset();
- if (!was_empty)
- return true; // Someone else should have started the sub-pump.
-
- pump = pump_;
- }
- // Since the incoming_queue_ may contain a task that destroys this message
- // loop, we cannot exit incoming_queue_lock_ until we are done with |this|.
- // We use a stack-based reference to the message pump so that we can call
- // ScheduleWork outside of incoming_queue_lock_.
+ // This should only be called while the lock is taken.
+ message_loop_proxy_->message_loop_lock_.AssertAcquired();
- pump->ScheduleWork();
- return true;
+ PendingTask pending_task(
+ from_here, task, CalculateDelayedRuntime(delay), nestable);
+
+ // Initialize the sequence number. The sequence number is used for delayed
+ // tasks (to faciliate FIFO sorting when two tasks have the same
+ // delayed_run_time value) and for identifying the task in about:tracing.
+ pending_task.sequence_num = next_sequence_num_++;
+
+ TRACE_EVENT_FLOW_BEGIN0("task", "MessageLoop::PostTask",
+ TRACE_ID_MANGLE(GetTaskTraceID(pending_task, this)));
+
+ bool was_empty = incoming_queue_.empty();
+ incoming_queue_.push(pending_task);
+ pending_task.task.Reset();
+
+ // Wake up the pump.
+ if (was_empty)
+ pump_->ScheduleWork();
}
//------------------------------------------------------------------------------

Powered by Google App Engine
This is Rietveld 408576698