Make ozone/drm/mojo more immune to startup races

ui/ozone/platform/drm/mus_thread_proxy.cc

Index: ui/ozone/platform/drm/mus_thread_proxy.cc
diff --git a/ui/ozone/platform/drm/mus_thread_proxy.cc b/ui/ozone/platform/drm/mus_thread_proxy.cc
index a28e9009aac46dedfaf7b02b4f121566672c6d71..7b548c83e121817fbea3e4dfef943f1a6e2d9f50 100644
--- a/ui/ozone/platform/drm/mus_thread_proxy.cc
+++ b/ui/ozone/platform/drm/mus_thread_proxy.cc
@@ -9,6 +9,7 @@
 #include "base/task_runner.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "ui/ozone/platform/drm/common/drm_util.h"
+#include "ui/ozone/platform/drm/cursor_proxy_mojo.h"
 #include "ui/ozone/platform/drm/gpu/drm_thread.h"
 #include "ui/ozone/platform/drm/gpu/proxy_helpers.h"
 #include "ui/ozone/platform/drm/host/drm_display_host_manager.h"
@@ -16,6 +17,27 @@
 
 namespace ui {
 
+namespace {
+
+// Forwarding proxy to handle ownership semantics.
+class CursorProxyThread : public DrmCursorProxy {
+ public:
+  explicit CursorProxyThread(MusThreadProxy* mus_thread_proxy);
+  ~CursorProxyThread() override;
+
+ private:
+  // DrmCursorProxy.
+  void CursorSet(gfx::AcceleratedWidget window,
+                 const std::vector<SkBitmap>& bitmaps,
+                 const gfx::Point& point,
+                 int frame_delay_ms) override;
+  void Move(gfx::AcceleratedWidget window, const gfx::Point& point) override;
+  void InitializeOnEvdevIfNecessary() override;
+  void SendToDelegate(DrmCursorProxy* delegate) override;
+  MusThreadProxy* const mus_thread_proxy_;  // Not owned.
+  DISALLOW_COPY_AND_ASSIGN(CursorProxyThread);
+};
+
 CursorProxyThread::CursorProxyThread(MusThreadProxy* mus_thread_proxy)
     : mus_thread_proxy_(mus_thread_proxy) {}
 CursorProxyThread::~CursorProxyThread() {}
@@ -30,16 +52,75 @@ void CursorProxyThread::Move(gfx::AcceleratedWidget window,
                              const gfx::Point& point) {
   mus_thread_proxy_->Move(window, point);
 }
-void CursorProxyThread::InitializeOnEvdev() {
-  mus_thread_proxy_->InitializeOnEvdev();
+void CursorProxyThread::InitializeOnEvdevIfNecessary() {
+  mus_thread_proxy_->InitializeOnEvdevIfNecessary();
+}
+void CursorProxyThread::SendToDelegate(DrmCursorProxy* delegate) {}
+
+// Mushrome simple display management races the launch of the evdev thread and
+// the gpu service. |NullProxy| in |DrmCursor| will safely discard all incoming
+// cursor requests until the DRM thread launches. Unfortunately, this includes

[dnicoara, 2017/06/05 15:04:43]

Isn't cursor updates on GPU start covered by DrmWindowHost::OnGpuThreadReady()?

The callstack is:
DrmWindowHost::OnGpuThreadReady()
DrmWindowHost::SendBoundsChange()
DrmCursor::CommitBoundsChange()
DrmCursor::SendCursorShowLocked()

[rjkroege, 2017/06/05 20:58:27]

No. Mushrome sets the cursor (once) when it sets up its WindowTreeClient which
is before the GPU start. This then arrives at ui::DrmCursor before the GPU
process starts.

[dnicoara, 2017/06/05 21:10:54]

But ui::DrmCursor is a host side object, not GPU. It should exist when Mushrome
sets the cursor (and it caches the bitmap).

+// the mesages that set the initial cursor bitmaps. This |DrmCursorProxy|
+// implementation  stashes the last |CursorSet| operation and submits on the
+// launch of the DRM thread.
+class CursorProxyStashing : public DrmCursorProxy {
+ public:
+  explicit CursorProxyStashing();
+  ~CursorProxyStashing() override;
+
+ private:
+  // DrmCursorProxy.
+  void CursorSet(gfx::AcceleratedWidget window,
+                 const std::vector<SkBitmap>& bitmaps,
+                 const gfx::Point& point,
+                 int frame_delay_ms) override;
+  void Move(gfx::AcceleratedWidget window, const gfx::Point& point) override;
+  void InitializeOnEvdevIfNecessary() override;
+  void SendToDelegate(DrmCursorProxy* delegate) override;
+
+  gfx::AcceleratedWidget stashed_window_ = 0;
+  std::vector<SkBitmap> stashed_bitmaps_;
+  gfx::Point stashed_point_;
+  int stashed_frame_delay_ms_;
+
+  DISALLOW_COPY_AND_ASSIGN(CursorProxyStashing);
+};
+
+CursorProxyStashing::CursorProxyStashing() {}
+CursorProxyStashing::~CursorProxyStashing() {}
+
+void CursorProxyStashing::CursorSet(gfx::AcceleratedWidget window,
+                                    const std::vector<SkBitmap>& bitmaps,
+                                    const gfx::Point& point,
+                                    int frame_delay_ms) {
+  stashed_window_ = window;
+  stashed_bitmaps_ = bitmaps;
+  stashed_point_ = point;
+  stashed_frame_delay_ms_ = frame_delay_ms;
+}
+void CursorProxyStashing::Move(gfx::AcceleratedWidget window,
+                               const gfx::Point& point) {}
+void CursorProxyStashing::InitializeOnEvdevIfNecessary() {}
+void CursorProxyStashing::SendToDelegate(DrmCursorProxy* delegate) {
+  if (stashed_window_ > 0) {
+    delegate->CursorSet(stashed_window_, stashed_bitmaps_, stashed_point_,
+                        stashed_frame_delay_ms_);
+  }
 }
 
-MusThreadProxy::MusThreadProxy()
+}  // namespace
+
+MusThreadProxy::MusThreadProxy(DrmCursor* cursor,
+                               service_manager::Connector* connector)
     : ws_task_runner_(base::ThreadTaskRunnerHandle::Get()),
       drm_thread_(nullptr),
-      weak_ptr_factory_(this) {}
-
-void MusThreadProxy::InitializeOnEvdev() {}
+      cursor_(cursor),
+      connector_(connector),
+      weak_ptr_factory_(this) {
+  if (connector_ != nullptr) {
+    cursor_->SetDrmCursorProxy(base::MakeUnique<CursorProxyStashing>());
+  }
+}
 
 MusThreadProxy::~MusThreadProxy() {
   DCHECK(on_window_server_thread_.CalledOnValidThread());
@@ -59,6 +140,7 @@ void MusThreadProxy::ProvideManagers(DrmDisplayHostManager* display_manager,
   overlay_manager_ = overlay_manager;
 }
 
+// Runs on Gpu thread.
 void MusThreadProxy::StartDrmThread() {
   DCHECK(drm_thread_);
   drm_thread_->Start();
@@ -81,6 +163,25 @@ void MusThreadProxy::RunObservers() {
     observer.OnGpuProcessLaunched();
     observer.OnGpuThreadReady();
   }
+
+  // The cursor is special since it will process input events on the IO thread
+  // and can by-pass the UI thread. This means that we need to special case it
+  // and notify it after all other observers/handlers are notified.
+  if (connector_ == nullptr) {
+    // CursorProxyThread does not need to use delegate because the non-mojo
+    // MusThreadProxy is only used in tests that do not operate the cursor.
+    // Future refactoring will unify the mojo and in-process modes.
+    cursor_->SetDrmCursorProxy(base::MakeUnique<CursorProxyThread>(this));
+  } else {
+    std::unique_ptr<DrmCursorProxy> new_proxy =
+        base::MakeUnique<CursorProxyMojo>(connector_);
+    auto new_proxy_ptr = new_proxy.get();
+    auto old_proxy = cursor_->SetDrmCursorProxy(std::move(new_proxy));
+    old_proxy->SendToDelegate(new_proxy_ptr);
+  }
+
+  // TODO(rjkroege): Call ResetDrmCursorProxy when the mojo connection to the
+  // DRM thread is broken.
 }
 
 void MusThreadProxy::AddGpuThreadObserver(GpuThreadObserver* observer) {
@@ -121,8 +222,9 @@ void MusThreadProxy::UnRegisterHandlerForDrmDisplayHostManager() {
 }
 
 bool MusThreadProxy::GpuCreateWindow(gfx::AcceleratedWidget widget) {
-  DCHECK(drm_thread_->IsRunning());
   DCHECK(on_window_server_thread_.CalledOnValidThread());
+  if (!drm_thread_ || !drm_thread_->IsRunning())
+    return false;
   drm_thread_->task_runner()->PostTask(
       FROM_HERE, base::Bind(&DrmThread::CreateWindow,
                             base::Unretained(drm_thread_), widget));
@@ -130,8 +232,9 @@ bool MusThreadProxy::GpuCreateWindow(gfx::AcceleratedWidget widget) {
 }
 
 bool MusThreadProxy::GpuDestroyWindow(gfx::AcceleratedWidget widget) {
-  DCHECK(drm_thread_->IsRunning());
   DCHECK(on_window_server_thread_.CalledOnValidThread());
+  if (!drm_thread_ || !drm_thread_->IsRunning())
+    return false;
   drm_thread_->task_runner()->PostTask(
       FROM_HERE, base::Bind(&DrmThread::DestroyWindow,
                             base::Unretained(drm_thread_), widget));
@@ -140,20 +243,23 @@ bool MusThreadProxy::GpuDestroyWindow(gfx::AcceleratedWidget widget) {
 
 bool MusThreadProxy::GpuWindowBoundsChanged(gfx::AcceleratedWidget widget,
                                             const gfx::Rect& bounds) {
-  DCHECK(drm_thread_->IsRunning());
   DCHECK(on_window_server_thread_.CalledOnValidThread());
+  if (!drm_thread_ || !drm_thread_->IsRunning())
+    return false;
   drm_thread_->task_runner()->PostTask(
       FROM_HERE, base::Bind(&DrmThread::SetWindowBounds,
                             base::Unretained(drm_thread_), widget, bounds));
   return true;
 }
 
+// Services needed for DrmCursorProxy.
 void MusThreadProxy::CursorSet(gfx::AcceleratedWidget widget,
                                const std::vector<SkBitmap>& bitmaps,
                                const gfx::Point& location,
                                int frame_delay_ms) {
-  DCHECK(drm_thread_->IsRunning());
   DCHECK(on_window_server_thread_.CalledOnValidThread());
+  if (!drm_thread_ || !drm_thread_->IsRunning())
+    return;
   drm_thread_->task_runner()->PostTask(
       FROM_HERE,
       base::Bind(&DrmThread::SetCursor, base::Unretained(drm_thread_), widget,
@@ -163,12 +269,17 @@ void MusThreadProxy::CursorSet(gfx::AcceleratedWidget widget,
 void MusThreadProxy::Move(gfx::AcceleratedWidget widget,
                           const gfx::Point& location) {
   // NOTE: Input events skip the main thread to avoid jank.
-  DCHECK(drm_thread_->IsRunning());
+  if (!drm_thread_ || !drm_thread_->IsRunning())
+    return;
   drm_thread_->task_runner()->PostTask(
       FROM_HERE, base::Bind(&DrmThread::MoveCursor,
                             base::Unretained(drm_thread_), widget, location));
 }
 
+void MusThreadProxy::InitializeOnEvdevIfNecessary() {}
+
+void MusThreadProxy::SendToDelegate(DrmCursorProxy* delegate) {}
+
 // Services needed for DrmOverlayManager.
 void MusThreadProxy::RegisterHandlerForDrmOverlayManager(
     DrmOverlayManager* handler) {
@@ -184,8 +295,9 @@ void MusThreadProxy::UnRegisterHandlerForDrmOverlayManager() {
 bool MusThreadProxy::GpuCheckOverlayCapabilities(
     gfx::AcceleratedWidget widget,
     const std::vector<OverlayCheck_Params>& overlays) {
-  DCHECK(drm_thread_->IsRunning());
   DCHECK(on_window_server_thread_.CalledOnValidThread());
+  if (!drm_thread_ || !drm_thread_->IsRunning())
+    return false;
   auto callback =
       base::BindOnce(&MusThreadProxy::GpuCheckOverlayCapabilitiesCallback,
                      weak_ptr_factory_.GetWeakPtr());
@@ -198,8 +310,9 @@ bool MusThreadProxy::GpuCheckOverlayCapabilities(
 }
 
 bool MusThreadProxy::GpuRefreshNativeDisplays() {
-  DCHECK(drm_thread_->IsRunning());
   DCHECK(on_window_server_thread_.CalledOnValidThread());
+  if (!drm_thread_ || !drm_thread_->IsRunning())
+    return false;
   auto callback =
       base::BindOnce(&MusThreadProxy::GpuRefreshNativeDisplaysCallback,
                      weak_ptr_factory_.GetWeakPtr());
@@ -214,8 +327,9 @@ bool MusThreadProxy::GpuRefreshNativeDisplays() {
 bool MusThreadProxy::GpuConfigureNativeDisplay(int64_t id,
                                                const DisplayMode_Params& pmode,
                                                const gfx::Point& origin) {
-  DCHECK(drm_thread_->IsRunning());
   DCHECK(on_window_server_thread_.CalledOnValidThread());
+  if (!drm_thread_ || !drm_thread_->IsRunning())
+    return false;
 
   auto mode = CreateDisplayModeFromParams(pmode);
   auto callback =
@@ -231,8 +345,9 @@ bool MusThreadProxy::GpuConfigureNativeDisplay(int64_t id,
 }
 
 bool MusThreadProxy::GpuDisableNativeDisplay(int64_t id) {
-  DCHECK(drm_thread_->IsRunning());
   DCHECK(on_window_server_thread_.CalledOnValidThread());
+  if (!drm_thread_ || !drm_thread_->IsRunning())
+    return false;
   auto callback =
       base::BindOnce(&MusThreadProxy::GpuDisableNativeDisplayCallback,
                      weak_ptr_factory_.GetWeakPtr());
@@ -245,8 +360,9 @@ bool MusThreadProxy::GpuDisableNativeDisplay(int64_t id) {
 }
 
 bool MusThreadProxy::GpuTakeDisplayControl() {
-  DCHECK(drm_thread_->IsRunning());
   DCHECK(on_window_server_thread_.CalledOnValidThread());
+  if (!drm_thread_ || !drm_thread_->IsRunning())
+    return false;
   auto callback = base::BindOnce(&MusThreadProxy::GpuTakeDisplayControlCallback,
                                  weak_ptr_factory_.GetWeakPtr());
   auto safe_callback = CreateSafeOnceCallback(std::move(callback));
@@ -258,8 +374,9 @@ bool MusThreadProxy::GpuTakeDisplayControl() {
 }
 
 bool MusThreadProxy::GpuRelinquishDisplayControl() {
-  DCHECK(drm_thread_->IsRunning());
   DCHECK(on_window_server_thread_.CalledOnValidThread());
+  if (!drm_thread_ || !drm_thread_->IsRunning())
+    return false;
   auto callback =
       base::BindOnce(&MusThreadProxy::GpuRelinquishDisplayControlCallback,
                      weak_ptr_factory_.GetWeakPtr());
@@ -273,8 +390,9 @@ bool MusThreadProxy::GpuRelinquishDisplayControl() {
 
 bool MusThreadProxy::GpuAddGraphicsDevice(const base::FilePath& path,
                                           const base::FileDescriptor& fd) {
-  DCHECK(drm_thread_->IsRunning());
   DCHECK(on_window_server_thread_.CalledOnValidThread());
+  if (!drm_thread_ || !drm_thread_->IsRunning())
+    return false;
   drm_thread_->task_runner()->PostTask(
       FROM_HERE, base::Bind(&DrmThread::AddGraphicsDevice,
                             base::Unretained(drm_thread_), path, fd));
@@ -282,8 +400,9 @@ bool MusThreadProxy::GpuAddGraphicsDevice(const base::FilePath& path,
 }
 
 bool MusThreadProxy::GpuRemoveGraphicsDevice(const base::FilePath& path) {
-  DCHECK(drm_thread_->IsRunning());
   DCHECK(on_window_server_thread_.CalledOnValidThread());
+  if (!drm_thread_ || !drm_thread_->IsRunning())
+    return false;
   drm_thread_->task_runner()->PostTask(
       FROM_HERE, base::Bind(&DrmThread::RemoveGraphicsDevice,
                             base::Unretained(drm_thread_), path));
@@ -291,8 +410,9 @@ bool MusThreadProxy::GpuRemoveGraphicsDevice(const base::FilePath& path) {
 }
 
 bool MusThreadProxy::GpuGetHDCPState(int64_t display_id) {
-  DCHECK(drm_thread_->IsRunning());
   DCHECK(on_window_server_thread_.CalledOnValidThread());
+  if (!drm_thread_ || !drm_thread_->IsRunning())
+    return false;
   auto callback = base::BindOnce(&MusThreadProxy::GpuGetHDCPStateCallback,
                                  weak_ptr_factory_.GetWeakPtr());
   auto safe_callback = CreateSafeOnceCallback(std::move(callback));
@@ -306,7 +426,8 @@ bool MusThreadProxy::GpuGetHDCPState(int64_t display_id) {
 bool MusThreadProxy::GpuSetHDCPState(int64_t display_id,
                                      display::HDCPState state) {
   DCHECK(on_window_server_thread_.CalledOnValidThread());
-  DCHECK(drm_thread_->IsRunning());
+  if (!drm_thread_ || !drm_thread_->IsRunning())
+    return false;
   auto callback = base::BindOnce(&MusThreadProxy::GpuSetHDCPStateCallback,
                                  weak_ptr_factory_.GetWeakPtr());
   auto safe_callback = CreateSafeOnceCallback(std::move(callback));
@@ -322,8 +443,9 @@ bool MusThreadProxy::GpuSetColorCorrection(
     const std::vector<display::GammaRampRGBEntry>& degamma_lut,
     const std::vector<display::GammaRampRGBEntry>& gamma_lut,
     const std::vector<float>& correction_matrix) {
-  DCHECK(drm_thread_->IsRunning());
   DCHECK(on_window_server_thread_.CalledOnValidThread());
+  if (!drm_thread_ || !drm_thread_->IsRunning())
+    return false;
   drm_thread_->task_runner()->PostTask(
       FROM_HERE,
       base::Bind(&DrmThread::SetColorCorrection, base::Unretained(drm_thread_),