GPU scheduler thread safety fixes

Two fixes:
1) Create a weak ptr on main thread before use to prevent data race on
   calling GetWeakPtr() from multiple threads.
2) Ensure sync token release callback is called on main thread so that
   the weak ptr is not dereferenced on another thread.  Sync tokens
   aren't released on any other thread today, but that might change with
   ChromeOS image decode accelerator work, and using WaitNonThreadSafe()
   will not post a task if on the same thread anyway.

Bug: 908669
Change-Id: I0bd9e0d56fe94c750513bc792e9e632bb83b30b9
Reviewed-on: https://chromium-review.googlesource.com/c/1352858
Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612471}

gpu/command_buffer/service/scheduler.cc

@@ -293,6 +293,8 @@ Scheduler::Scheduler(scoped_refptr<base::SingleThreadTaskRunner> task_runner,
       sync_point_manager_(sync_point_manager),
       weak_factory_(this) {
   DCHECK(thread_checker_.CalledOnValidThread());
+  // Store weak ptr separately because calling GetWeakPtr() is not thread safe.
+  weak_ptr_ = weak_factory_.GetWeakPtr();
 }
 
 Scheduler::~Scheduler() {
@@ -390,11 +392,11 @@ void Scheduler::ScheduleTaskHelper(Task task) {
     Sequence* release_sequence = GetSequence(release_sequence_id);
     if (!release_sequence)
       continue;
-    if (sync_point_manager_->Wait(
-            sync_token, sequence_id, order_num,
-            base::Bind(&Scheduler::SyncTokenFenceReleased,
-                       weak_factory_.GetWeakPtr(), sync_token, order_num,
-                       release_sequence_id, sequence_id))) {
+    if (sync_point_manager_->WaitNonThreadSafe(
+            sync_token, sequence_id, order_num, task_runner_,
+            base::BindOnce(&Scheduler::SyncTokenFenceReleased, weak_ptr_,
+                           sync_token, order_num, release_sequence_id,
+                           sequence_id))) {
       sequence->AddWaitFence(sync_token, order_num, release_sequence_id,
                              release_sequence);
     }
@@ -464,8 +466,8 @@ void Scheduler::TryScheduleSequence(Sequence* sequence) {
     if (!running_) {
       TRACE_EVENT_ASYNC_BEGIN0("gpu", "Scheduler::Running", this);
       running_ = true;
-      task_runner_->PostTask(FROM_HERE, base::Bind(&Scheduler::RunNextTask,
-                                                   weak_factory_.GetWeakPtr()));
+      task_runner_->PostTask(
+          FROM_HERE, base::BindOnce(&Scheduler::RunNextTask, weak_ptr_));
     }
   }
 }
@@ -540,8 +542,8 @@ void Scheduler::RunNextTask() {
     }
   }
 
-  task_runner_->PostTask(FROM_HERE, base::Bind(&Scheduler::RunNextTask,
-                                               weak_factory_.GetWeakPtr()));
+  task_runner_->PostTask(FROM_HERE,
+                         base::BindOnce(&Scheduler::RunNextTask, weak_ptr_));
 }
 
 }  // namespace gpu

gpu/command_buffer/service/scheduler.h

@@ -333,6 +333,8 @@ class GPU_EXPORT Scheduler {
 
   base::ThreadChecker thread_checker_;
 
+  // Invalidated on main thread.
+  base::WeakPtr<Scheduler> weak_ptr_;
   base::WeakPtrFactory<Scheduler> weak_factory_;
 
  private: