cc: BlockingTaskRunner without MessageLoopProxy

cc/trees/blocking_task_runner.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "cc/trees/blocking_task_runner.h"
 
 #include <utility>
 
 #include "base/logging.h"
 #include "base/memory/singleton.h"
 #include "base/message_loop/message_loop_proxy.h"
 
 namespace cc {
 
-typedef std::pair<base::SingleThreadTaskRunner*,
-                  scoped_refptr<BlockingTaskRunner> > TaskRunnerPair;
-
 struct TaskRunnerPairs {
   static TaskRunnerPairs* GetInstance() {
     return Singleton<TaskRunnerPairs>::get();
   }
 
   base::Lock lock;
-  std::vector<TaskRunnerPair> pairs;
+  std::vector<scoped_refptr<BlockingTaskRunner> > runners;
 
  private:
   friend struct DefaultSingletonTraits<TaskRunnerPairs>;
 };
 
 // static
 scoped_refptr<BlockingTaskRunner> BlockingTaskRunner::current() {
   TaskRunnerPairs* task_runners = TaskRunnerPairs::GetInstance();
+  base::PlatformThreadId thread_id = base::PlatformThread::CurrentId();
 
   base::AutoLock lock(task_runners->lock);
 
-  for (size_t i = 0; i < task_runners->pairs.size(); ++i) {
-    if (task_runners->pairs[i].first->HasOneRef()) {
-      // The SingleThreadTaskRunner is kept alive by its MessageLoop, and we
-      // hold a second reference in the TaskRunnerPairs array. If the
-      // SingleThreadTaskRunner has one ref, then it is being held alive only
-      // by the BlockingTaskRunner and the MessageLoop is gone, so drop the
-      // BlockingTaskRunner from the TaskRunnerPairs array along with the
-      // SingleThreadTaskRunner.
-      task_runners->pairs.erase(task_runners->pairs.begin() + i);
-      --i;
+  scoped_refptr<BlockingTaskRunner> current_task_runner;
+
+  for (size_t i = 0; i < task_runners->runners.size(); ++i) {
+    if (task_runners->runners[i]->thread_id_ == thread_id) {
+      current_task_runner = task_runners->runners[i];
+    } else if (task_runners->runners[i]->HasOneRef()) {
+      task_runners->runners.erase(task_runners->runners.begin() + i);
+      i--;
     }
   }
 
-  scoped_refptr<base::SingleThreadTaskRunner> current =
-      base::MessageLoopProxy::current();
-  for (size_t i = 0; i < task_runners->pairs.size(); ++i) {
-    if (task_runners->pairs[i].first == current.get())
-      return task_runners->pairs[i].second.get();
-  }
+  if (current_task_runner)
+    return current_task_runner;
 
-  scoped_refptr<BlockingTaskRunner> runner = new BlockingTaskRunner(current);
-  task_runners->pairs.push_back(TaskRunnerPair(current, runner));
+  scoped_refptr<BlockingTaskRunner> runner =
+      new BlockingTaskRunner(base::MessageLoopProxy::current());
+  task_runners->runners.push_back(runner);
   return runner;
 }
 
 BlockingTaskRunner::BlockingTaskRunner(
     scoped_refptr<base::SingleThreadTaskRunner> task_runner)
-    : task_runner_(task_runner), capture_(0) {}
+    : thread_id_(base::PlatformThread::CurrentId()),
+      task_runner_(task_runner),
+      capture_(0) {
+}
 
 BlockingTaskRunner::~BlockingTaskRunner() {}
 
+bool BlockingTaskRunner::BelongsToCurrentThread() {
+  return base::PlatformThread::CurrentId() == thread_id_;
+}
+
 bool BlockingTaskRunner::PostTask(const tracked_objects::Location& from_here,
                                   const base::Closure& task) {
   base::AutoLock lock(lock_);
   if (!capture_)
     return task_runner_->PostTask(from_here, task);

[piman, 2014/05/17 00:32:31]

How is this going to work if you don't have a MessageLoopProxy?

[boliu, 2014/05/17 00:34:25]

It can't. All PostTask must be wrapped by CapturePostTasks.

[piman, 2014/05/17 00:51:47]

Is that true today for SingleThreadProxy? (It can't be true for ThreadProxy).

It's hard to audit... the places that call into BlockingTaskRunner::PostTask are
far removed from places that create a CapturePostTask... We'll need to write a
lot of tests.


We should explicitly DCHECK(capture_ || task_runner_) here.


An alternative may be to keep the tasks, and run them on the next
SetCapture(true).

[boliu, 2014/05/17 01:08:54]

Right, this definitely can't work for threaded compositor. Things like Scheduler
and TileManager/RasterWorkerPools definitely depend on a working MessageLoop.

For single thread, with the simple case of a single DelegatedRendererLayer, init
and composite works. Tear down I fixed just now:
https://codereview.chromium.org/284233005/ Definitely need an end-to-end test
for this case.

How webview uses the single-thread parent compositor is mostly here:
https://codereview.chromium.org/266353003/diff/140001/android_webview/browser...

I'm not sure if there are layers that release resources outside of commits.
Those of course won't work.
I think the danger is BlockingTaskRunner could hold on to these tasks when
destroyed, and destruction can happen on any thread, so can't run these tasks on
destruction either.

DCHECK should be fine I think.

   captured_tasks_.push_back(task);
   return true;
 }
 
 void BlockingTaskRunner::SetCapture(bool capture) {
   DCHECK(BelongsToCurrentThread());
 
   std::vector<base::Closure> tasks;
 
   {
@@ skipping 14 matching lines @@
 BlockingTaskRunner::CapturePostTasks::CapturePostTasks()
     : blocking_runner_(BlockingTaskRunner::current()) {
   blocking_runner_->SetCapture(true);
 }
 
 BlockingTaskRunner::CapturePostTasks::~CapturePostTasks() {
   blocking_runner_->SetCapture(false);
 }
 
 }  // namespace cc