[Thread-safe] Apply base::Passed to WebThread::Task

content/child/webthread_impl.cc

Index: content/child/webthread_impl.cc
diff --git a/content/child/webthread_impl.cc b/content/child/webthread_impl.cc
index d29b36cb9878c3a15f4265e9c52182da17778eb8..f88de53257dd40b1648b819d09df4b0128aeb237 100644
--- a/content/child/webthread_impl.cc
+++ b/content/child/webthread_impl.cc
@@ -60,15 +60,23 @@ WebThreadImpl::WebThreadImpl(const char* name)
   thread_->Start();
 }
 
+static void RunWebThreadTask(scoped_ptr<blink::WebThread::Task> task)

[dcheng, 2014/12/18 19:31:38]

This code is really subtle. Can we expand this comment to describe why this is
necessary instead of just referencing a bug?

Also, if this task never runs at all, won't it potentially still be destroyed on
the wrong thread?

[hiroshige, 2014/12/19 09:16:11]

Added comments.
Yes, |WebThread::Task| can be destoyed on any thread, but in that case there is
no race condition, as described in the comment I added.
I think deleting |WebThread::Task| (and thus its parameters including
|StringImpl|) on a different thread from the thread where the task is intended
to be executed is not a problem. (if there is any specific problem please let me
know...)

+// Take ownership of blink::WebThread::Task and delete it on first |run|

[nasko, 2014/12/18 17:53:25]

nit: Comment should be before the method starts.

[hiroshige, 2014/12/19 09:16:11]

Done.

+// for thread-safety. http://crbug.com/390851, Comment #19
+{

[nasko, 2014/12/18 17:53:25]

Brace goes on the same line as the method params ending.

[hiroshige, 2014/12/19 09:16:11]

Done.

+    task->run();
+}
+
 void WebThreadImpl::postTask(Task* task) {
   thread_->message_loop()->PostTask(
-      FROM_HERE, base::Bind(&blink::WebThread::Task::run, base::Owned(task)));
+      FROM_HERE,
+      base::Bind(RunWebThreadTask, base::Passed(scoped_ptr<Task>(task))));

[dcheng, 2014/12/18 19:31:38]

It's slightly shorter to use make_scoped_ptr.

[hiroshige, 2014/12/19 09:16:11]

Done.

 }
 
 void WebThreadImpl::postDelayedTask(Task* task, long long delay_ms) {
   thread_->message_loop()->PostDelayedTask(
       FROM_HERE,
-      base::Bind(&blink::WebThread::Task::run, base::Owned(task)),
+      base::Bind(RunWebThreadTask, base::Passed(scoped_ptr<Task>(task))),
       base::TimeDelta::FromMilliseconds(delay_ms));
 }
 
@@ -103,14 +111,15 @@ WebThreadImplForMessageLoop::WebThreadImplForMessageLoop(
 
 void WebThreadImplForMessageLoop::postTask(Task* task) {
   main_thread_task_runner_->PostTask(
-      FROM_HERE, base::Bind(&blink::WebThread::Task::run, base::Owned(task)));
+      FROM_HERE,
+      base::Bind(RunWebThreadTask, base::Passed(scoped_ptr<Task>(task))));
 }
 
 void WebThreadImplForMessageLoop::postDelayedTask(Task* task,
                                                   long long delay_ms) {
   main_thread_task_runner_->PostDelayedTask(
       FROM_HERE,
-      base::Bind(&blink::WebThread::Task::run, base::Owned(task)),
+      base::Bind(RunWebThreadTask, base::Passed(scoped_ptr<Task>(task))),
       base::TimeDelta::FromMilliseconds(delay_ms));
 }