Relax RunTaskAfterWindowReady to block only on drm_device

Browser UI thread can be blocked on viz compositor or viz main. RunTaskAfterWindowReady blocks viz main on DRM thread when it is posted by PostSyncTask. These tasks need browser to call CreateWindow through mojo pipe to continue.
Deadlocks are caused by this. RunTaskAfterWindowReady needs to be relaxed to lower crash rates.

Bug: 1050676
Change-Id: Ie205300fa68300f129c1bb1e7d746c6399a65013
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2135128
Commit-Queue: Kramer Ge <fangzhoug@chromium.org>
Reviewed-by: Michael Spang <spang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#756756}

ui/ozone/platform/drm/gpu/drm_thread.cc

@@ -92,17 +92,15 @@ void DrmThread::Start(base::OnceClosure receiver_completer,
     LOG(FATAL) << "Failed to create DRM thread";
 }
 
-void DrmThread::RunTaskAfterWindowReady(gfx::AcceleratedWidget window,
-                                        base::OnceClosure task,
+void DrmThread::RunTaskAfterDeviceReady(base::OnceClosure task,
                                         base::WaitableEvent* done) {
-  if (!device_manager_->GetDrmDevices().empty() &&
-      window <= last_created_window_) {
+  if (!device_manager_->GetDrmDevices().empty()) {
     std::move(task).Run();
     if (done)
       done->Signal();
     return;
   }
-  pending_tasks_[window].emplace_back(std::move(task), done);
+  pending_tasks_.emplace_back(std::move(task), done);
 }
 
 void DrmThread::Init() {
@@ -411,17 +409,13 @@ void DrmThread::AddDrmDeviceReceiver(
 void DrmThread::ProcessPendingTasks() {
   DCHECK(!device_manager_->GetDrmDevices().empty());
 
-  auto it = pending_tasks_.begin();
-  for (; it != pending_tasks_.end() && it->first <= last_created_window_;
-       ++it) {
-    for (auto& task_info : it->second) {
-      std::move(task_info.task).Run();
-      if (task_info.done)
-        task_info.done->Signal();
-    }
+  for (auto& task_info : pending_tasks_) {
+    std::move(task_info.task).Run();
+    if (task_info.done)
+      task_info.done->Signal();
   }
 
-  pending_tasks_.erase(pending_tasks_.begin(), it);
+  pending_tasks_.clear();
 }
 
 }  // namespace ui

ui/ozone/platform/drm/gpu/drm_thread.h

@@ -74,10 +74,9 @@ class DrmThread : public base::Thread,
   void Start(base::OnceClosure receiver_completer,
              std::unique_ptr<DrmDeviceGenerator> device_generator);
 
-  // Runs |task| once a DrmDevice is registered and |window| was created via
-  // CreateWindow(). |done| will be signaled if it's not null.
-  void RunTaskAfterWindowReady(gfx::AcceleratedWidget window,
-                               base::OnceClosure task,
+  // Runs |task| once a DrmDevice is registered. |done|
+  // will be signaled if it's not null.
+  void RunTaskAfterDeviceReady(base::OnceClosure task,
                                base::WaitableEvent* done);
 
   // Must be called on the DRM thread. All methods for use from the GPU thread.
@@ -218,9 +217,8 @@ class DrmThread : public base::Thread,
   // The AcceleratedWidget from the last call to CreateWindow.
   gfx::AcceleratedWidget last_created_window_ = gfx::kNullAcceleratedWidget;
 
-  // The tasks that are blocked on a DrmDevice and a certain AcceleratedWidget
-  // becoming available.
-  base::flat_map<gfx::AcceleratedWidget, std::vector<TaskInfo>> pending_tasks_;
+  // The tasks that are blocked on a DrmDevice becoming available.
+  std::vector<TaskInfo> pending_tasks_;
 
   // Holds the DrmDeviceGenerator that DrmDeviceManager will use. Will be passed
   // on to DrmDeviceManager after the thread starts.

ui/ozone/platform/drm/gpu/drm_thread_proxy.cc

@@ -93,10 +93,9 @@ void DrmThreadProxy::CreateBuffer(gfx::AcceleratedWidget widget,
   base::OnceClosure task = base::BindOnce(
       &DrmThread::CreateBuffer, base::Unretained(&drm_thread_), widget, size,
       framebuffer_size, format, usage, flags, buffer, framebuffer);
-  PostSyncTask(
-      drm_thread_.task_runner(),
-      base::BindOnce(&DrmThread::RunTaskAfterWindowReady,
-                     base::Unretained(&drm_thread_), widget, std::move(task)));
+  PostSyncTask(drm_thread_.task_runner(),
+               base::BindOnce(&DrmThread::RunTaskAfterDeviceReady,
+                              base::Unretained(&drm_thread_), std::move(task)));
 }
 
 void DrmThreadProxy::CreateBufferAsync(gfx::AcceleratedWidget widget,
@@ -112,15 +111,10 @@ void DrmThreadProxy::CreateBufferAsync(gfx::AcceleratedWidget widget,
       size, format, usage, flags,
       base::BindOnce(OnBufferCreatedOnDrmThread,
                      base::ThreadTaskRunnerHandle::Get(), std::move(callback)));
-  // Since browser's UI thread blocks until a buffer is returned, we shouldn't
-  // block on |widget| because a blocked UI thread cannot register |widget| and
-  // causes a deadlock. We still want to block on a graphics device, though.
-  // TODO(samans): Remove this hack once OOP-D launches.
-  gfx::AcceleratedWidget blocking_widget = gfx::kNullAcceleratedWidget;
   drm_thread_.task_runner()->PostTask(
-      FROM_HERE, base::BindOnce(&DrmThread::RunTaskAfterWindowReady,
-                                base::Unretained(&drm_thread_), blocking_widget,
-                                std::move(task), nullptr));
+      FROM_HERE,
+      base::BindOnce(&DrmThread::RunTaskAfterDeviceReady,
+                     base::Unretained(&drm_thread_), std::move(task), nullptr));
 }
 
 void DrmThreadProxy::CreateBufferFromHandle(
@@ -135,10 +129,9 @@ void DrmThreadProxy::CreateBufferFromHandle(
   base::OnceClosure task = base::BindOnce(
       &DrmThread::CreateBufferFromHandle, base::Unretained(&drm_thread_),
       widget, size, format, std::move(handle), buffer, framebuffer);
-  PostSyncTask(
-      drm_thread_.task_runner(),
-      base::BindOnce(&DrmThread::RunTaskAfterWindowReady,
-                     base::Unretained(&drm_thread_), widget, std::move(task)));
+  PostSyncTask(drm_thread_.task_runner(),
+               base::BindOnce(&DrmThread::RunTaskAfterDeviceReady,
+                              base::Unretained(&drm_thread_), std::move(task)));
 }
 
 void DrmThreadProxy::SetClearOverlayCacheCallback(
@@ -162,9 +155,9 @@ void DrmThreadProxy::CheckOverlayCapabilities(
       widget, candidates, CreateSafeOnceCallback(std::move(callback)));
 
   drm_thread_.task_runner()->PostTask(
-      FROM_HERE, base::BindOnce(&DrmThread::RunTaskAfterWindowReady,
-                                base::Unretained(&drm_thread_), widget,
-                                std::move(task), nullptr));
+      FROM_HERE,
+      base::BindOnce(&DrmThread::RunTaskAfterDeviceReady,
+                     base::Unretained(&drm_thread_), std::move(task), nullptr));
 }
 
 std::vector<OverlayStatus> DrmThreadProxy::CheckOverlayCapabilitiesSync(
@@ -177,10 +170,9 @@ std::vector<OverlayStatus> DrmThreadProxy::CheckOverlayCapabilitiesSync(
   base::OnceClosure task = base::BindOnce(
       &DrmThread::CheckOverlayCapabilitiesSync, base::Unretained(&drm_thread_),
       widget, candidates, &result);
-  PostSyncTask(
-      drm_thread_.task_runner(),
-      base::BindOnce(&DrmThread::RunTaskAfterWindowReady,
-                     base::Unretained(&drm_thread_), widget, std::move(task)));
+  PostSyncTask(drm_thread_.task_runner(),
+               base::BindOnce(&DrmThread::RunTaskAfterDeviceReady,
+                              base::Unretained(&drm_thread_), std::move(task)));
   return result;
 }
 

ui/ozone/platform/drm/gpu/drm_thread_unittest.cc

@@ -50,25 +50,24 @@ class DrmThreadTest : public testing::Test {
     drm_thread_.FlushForTesting();
   }
 
-  std::unique_ptr<base::WaitableEvent> PostStubTaskWithWaitableEvent(
-      gfx::AcceleratedWidget window) {
+  std::unique_ptr<base::WaitableEvent> PostStubTaskWithWaitableEvent() {
     base::OnceClosure task = base::BindOnce(StubTask);
     auto event = std::make_unique<base::WaitableEvent>(
         base::WaitableEvent::ResetPolicy::AUTOMATIC,
         base::WaitableEvent::InitialState::NOT_SIGNALED);
     drm_thread_.task_runner()->PostTask(
-        FROM_HERE, base::BindOnce(&DrmThread::RunTaskAfterWindowReady,
-                                  base::Unretained(&drm_thread_), window,
+        FROM_HERE, base::BindOnce(&DrmThread::RunTaskAfterDeviceReady,
+                                  base::Unretained(&drm_thread_),
                                   std::move(task), event.get()));
     return event;
   }
 
-  void PostStubTask(gfx::AcceleratedWidget window, bool* done) {
+  void PostStubTask(bool* done) {
     *done = false;
     base::OnceClosure task = base::BindOnce(StubTaskWithDoneFeedback, done);
     drm_thread_.task_runner()->PostTask(
-        FROM_HERE, base::BindOnce(&DrmThread::RunTaskAfterWindowReady,
-                                  base::Unretained(&drm_thread_), window,
+        FROM_HERE, base::BindOnce(&DrmThread::RunTaskAfterDeviceReady,
+                                  base::Unretained(&drm_thread_),
                                   std::move(task), nullptr));
   }
 
@@ -84,87 +83,30 @@ class DrmThreadTest : public testing::Test {
   mojo::Remote<ozone::mojom::DrmDevice> drm_device_;
 };
 
-TEST_F(DrmThreadTest, RunTaskAfterWindowReady) {
-  constexpr gfx::Rect bounds(10, 10);
-  bool called1 = false, called2 = false;
-  gfx::AcceleratedWidget widget1 = 1, widget2 = 2;
+TEST_F(DrmThreadTest, RunTaskAfterDeviceReady) {
+  bool called = false;
 
-  // Post a task not blocked on any window. It should still block on a graphics
-  // device becoming available.
-  PostStubTask(gfx::kNullAcceleratedWidget, &called1);
+  // Post 2 tasks. One with WaitableEvent and one without. They should block on
+  // a graphics device becoming available.
+  std::unique_ptr<base::WaitableEvent> event = PostStubTaskWithWaitableEvent();
+  PostStubTask(&called);
   drm_thread_.FlushForTesting();
-  EXPECT_FALSE(called1);
+  EXPECT_FALSE(event->IsSignaled());
+  EXPECT_FALSE(called);
 
-  // Add the graphics device. The task should run.
+  // Add the graphics device. The tasks should run.
   AddGraphicsDevice();
   drm_thread_.FlushForTesting();
-  ASSERT_TRUE(called1);
-
-  // Now that a graphics device is available, further tasks that don't block on
-  // any window should execute immediately.
-  PostStubTask(gfx::kNullAcceleratedWidget, &called1);
-  drm_thread_.FlushForTesting();
-  ASSERT_TRUE(called1);
-
-  // Post a task blocked on |widget1|. It shouldn't run.
-  PostStubTask(widget1, &called1);
-  drm_thread_.FlushForTesting();
-  ASSERT_FALSE(called1);
-
-  // Post two tasks blocked on |widget2|, one with a WaitableEvent and one
-  // without. They shouldn't run.
-  std::unique_ptr<base::WaitableEvent> event =
-      PostStubTaskWithWaitableEvent(widget2);
-  PostStubTask(widget2, &called2);
-  drm_thread_.FlushForTesting();
-  ASSERT_FALSE(event->IsSignaled());
-  ASSERT_FALSE(called2);
-
-  // Now create |widget1|. The first task should run.
-  drm_device_->CreateWindow(widget1, bounds);
-  drm_thread_.FlushForTesting();
-  ASSERT_TRUE(called1);
-  ASSERT_FALSE(event->IsSignaled());
-  ASSERT_FALSE(called2);
-
-  // Now that |widget1| is created. any further task depending on it should run
-  // immediately.
-  PostStubTask(widget1, &called1);
-  drm_thread_.FlushForTesting();
-  ASSERT_TRUE(called1);
-  ASSERT_FALSE(event->IsSignaled());
-  ASSERT_FALSE(called2);
-
-  // Destroy |widget1| and post a task blocked on it. The task should still run
-  // immediately even though the window is destroyed.
-  drm_device_->DestroyWindow(widget1);
-  PostStubTask(widget1, &called1);
-  drm_thread_.FlushForTesting();
-  ASSERT_TRUE(called1);
-  ASSERT_FALSE(event->IsSignaled());
-  ASSERT_FALSE(called2);
-
-  // Create |widget2|. The two blocked tasks should run.
-  drm_device_->CreateWindow(widget2, bounds);
-  drm_thread_.FlushForTesting();
   ASSERT_TRUE(event->IsSignaled());
-  ASSERT_TRUE(called2);
+  ASSERT_TRUE(called);
 
-  // Post another task blocked on |widget1| with a WaitableEvent. It should run
+  // Now that a graphics device is available, further tasks should execute
   // immediately.
-  event = PostStubTaskWithWaitableEvent(widget1);
+  event = PostStubTaskWithWaitableEvent();
+  PostStubTask(&called);
   drm_thread_.FlushForTesting();
   ASSERT_TRUE(event->IsSignaled());
-
-  // Post another task blocked on |widget2| with a WaitableEvent. It should run
-  // immediately.
-  event = PostStubTaskWithWaitableEvent(widget2);
-  drm_thread_.FlushForTesting();
-  ASSERT_TRUE(event->IsSignaled());
-
-  // Destroy |widget2| to avoid failures during tear down.
-  drm_device_->DestroyWindow(widget2);
-  drm_thread_.FlushForTesting();
+  ASSERT_TRUE(called);
 }
 
 // Verifies that we gracefully handle the case where CheckOverlayCapabilities()

ui/ozone/platform/drm/gpu/drm_window_proxy.cc

@@ -35,9 +35,9 @@ void DrmWindowProxy::SchedulePageFlip(
       std::move(planes), CreateSafeOnceCallback(std::move(submission_callback)),
       CreateSafeOnceCallback(std::move(presentation_callback)));
   drm_thread_->task_runner()->PostTask(
-      FROM_HERE, base::BindOnce(&DrmThread::RunTaskAfterWindowReady,
-                                base::Unretained(drm_thread_), widget_,
-                                std::move(task), nullptr));
+      FROM_HERE,
+      base::BindOnce(&DrmThread::RunTaskAfterDeviceReady,
+                     base::Unretained(drm_thread_), std::move(task), nullptr));
 }
 
 bool DrmWindowProxy::SupportsGpuFences() const {
@@ -45,10 +45,9 @@ bool DrmWindowProxy::SupportsGpuFences() const {
   base::OnceClosure task =
       base::BindOnce(&DrmThread::IsDeviceAtomic, base::Unretained(drm_thread_),
                      widget_, &is_atomic);
-  PostSyncTask(
-      drm_thread_->task_runner(),
-      base::BindOnce(&DrmThread::RunTaskAfterWindowReady,
-                     base::Unretained(drm_thread_), widget_, std::move(task)));
+  PostSyncTask(drm_thread_->task_runner(),
+               base::BindOnce(&DrmThread::RunTaskAfterDeviceReady,
+                              base::Unretained(drm_thread_), std::move(task)));
   return is_atomic && !base::CommandLine::ForCurrentProcess()->HasSwitch(
                           switches::kDisableExplicitDmaFences);
 }