Fix two Ozone initialization races.

The first race is only for Ozone DRM. In DrmDisplayHostManager
|primary_drm_device_handle_| was being accessed from multiple threads.
The value was changed by the IO thread while the OzoneUI thread was
still using it.

Change calls to GpuThreadObserver OnGpuProcessLaunched() so it happens
on the OzoneUI thread. This delays the call slightly but it removes
the possibility for a race modifying |primary_drm_device_handle_|.

The second race was with OzonePlatform::RegisterStartupCallback(). This
is called from the IO thread and it checks if there is an OzonePlatform
instance and if Ozone UI initialization has happened. If both of those
things are true, then it runs a callback immediately, otherwise it runs
a callback after those things become true. The problem is that
|g_platform_initialized_ui| was set true on the UI thread before Ozone
UI initialization was fully finished. The callback accessed a null
OzonePlatform member variable and crashed.

Make sure that |g_platform_initialized_ui| is set after initialization
is finished and that variable change is protected by the same lock used
in RegisterStartupCallback(). The lock only protects changing
|g_platform_initialized_ui|, not all of initialization, so the IO thread
won't block for an extended period.

Bug: 824809, 828407
Change-Id: I73b9404c823c9eeaaeaba99feb1e113953a5bb1b
Reviewed-on: https://chromium-review.googlesource.com/980574Reviewed-by: Robert Kroeger <rjkroege@chromium.org>
Commit-Queue: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548481}

ui/ozone/platform/drm/host/drm_display_host_manager.cc

@@ -322,7 +322,7 @@ void DrmDisplayHostManager::OnGpuProcessLaunched() {
         MapDevPathToSysPath(primary_graphics_card_path_);
 
     if (!handle) {
-      handle.reset(new DrmDeviceHandle());
+      handle = std::make_unique<DrmDeviceHandle>();
       if (!handle->Initialize(primary_graphics_card_path_,
                               drm_devices_[primary_graphics_card_path_]))
         LOG(FATAL) << "Failed to open primary graphics card";
@@ -331,8 +331,10 @@ void DrmDisplayHostManager::OnGpuProcessLaunched() {
 
   // Send the primary device first since this is used to initialize graphics
   // state.
-  proxy_->GpuAddGraphicsDevice(drm_devices_[primary_graphics_card_path_],
-                               handle->PassFD());
+  if (!proxy_->GpuAddGraphicsDevice(drm_devices_[primary_graphics_card_path_],
+                                    handle->PassFD())) {
+    LOG(ERROR) << "Failed to add primary graphics device.";
+  }
 }
 
 void DrmDisplayHostManager::OnGpuThreadReady() {

ui/ozone/platform/drm/host/drm_gpu_platform_support_host.cc

@@ -115,7 +115,8 @@ void DrmGpuPlatformSupportHost::RemoveGpuThreadObserver(
 }
 
 bool DrmGpuPlatformSupportHost::IsConnected() {
-  return host_id_ >= 0 && channel_established_;
+  base::AutoLock auto_lock(host_id_lock_);
+  return host_id_ >= 0;
 }
 
 void DrmGpuPlatformSupportHost::OnGpuServiceLaunched(
@@ -139,27 +140,23 @@ void DrmGpuPlatformSupportHost::OnGpuProcessLaunched(
   DCHECK(!ui_runner_->BelongsToCurrentThread());
   TRACE_EVENT1("drm", "DrmGpuPlatformSupportHost::OnGpuProcessLaunched",
                "host_id", host_id);
-  host_id_ = host_id;
-  send_runner_ = std::move(send_runner);
-  send_callback_ = send_callback;
-
-  for (GpuThreadObserver& observer : gpu_thread_observers_)
-    observer.OnGpuProcessLaunched();
 
   ui_runner_->PostTask(
       FROM_HERE,
       base::BindOnce(&DrmGpuPlatformSupportHost::OnChannelEstablished,
-                     weak_ptr_));
+                     weak_ptr_, host_id, std::move(send_runner),
+                     std::move(send_callback)));
 }
 
 void DrmGpuPlatformSupportHost::OnChannelDestroyed(int host_id) {
   TRACE_EVENT1("drm", "DrmGpuPlatformSupportHost::OnChannelDestroyed",
                "host_id", host_id);
-
   if (host_id_ == host_id) {
+    {
+      base::AutoLock auto_lock(host_id_lock_);
+      host_id_ = -1;
+    }
     cursor_->ResetDrmCursorProxy();
-    host_id_ = -1;
-    channel_established_ = false;
     send_runner_ = nullptr;
     send_callback_.Reset();
     for (GpuThreadObserver& observer : gpu_thread_observers_)
@@ -200,9 +197,22 @@ void DrmGpuPlatformSupportHost::UnRegisterHandlerForDrmDisplayHostManager() {
   display_manager_ = nullptr;
 }
 
-void DrmGpuPlatformSupportHost::OnChannelEstablished() {
+void DrmGpuPlatformSupportHost::OnChannelEstablished(
+    int host_id,
+    scoped_refptr<base::SingleThreadTaskRunner> send_runner,
+    const base::Callback<void(IPC::Message*)>& send_callback) {
+  DCHECK(ui_runner_->BelongsToCurrentThread());
   TRACE_EVENT0("drm", "DrmGpuPlatformSupportHost::OnChannelEstablished");
-  channel_established_ = true;
+
+  send_runner_ = std::move(send_runner);
+  send_callback_ = send_callback;
+  {
+    base::AutoLock auto_lock(host_id_lock_);
+    host_id_ = host_id;
+  }
+
+  for (GpuThreadObserver& observer : gpu_thread_observers_)
+    observer.OnGpuProcessLaunched();
 
   for (GpuThreadObserver& observer : gpu_thread_observers_)
     observer.OnGpuThreadReady();
@@ -278,22 +288,8 @@ bool DrmGpuPlatformSupportHost::GpuRelinquishDisplayControl() {
 
 bool DrmGpuPlatformSupportHost::GpuAddGraphicsDevice(const base::FilePath& path,
                                                      base::ScopedFD fd) {
-  IPC::Message* message = new OzoneGpuMsg_AddGraphicsDevice(
-      path, base::FileDescriptor(std::move(fd)));
-
-  // This function may be called from two places:
-  // - DrmDisplayHostManager::OnGpuProcessLaunched() invoked synchronously
-  //   by GpuProcessHost::Init() on IO thread, which is the same thread as
-  //   |send_runner_|. In this case we can synchronously send the IPC;
-  // - DrmDisplayHostManager::OnAddGraphicsDevice() on UI thread. In this
-  //   case we need to post the send task to IO thread.
-  if (send_runner_ && send_runner_->BelongsToCurrentThread()) {
-    DCHECK(!send_callback_.is_null());
-    send_callback_.Run(message);
-    return true;
-  }
-
-  return Send(message);
+  return Send(new OzoneGpuMsg_AddGraphicsDevice(
+      path, base::FileDescriptor(std::move(fd))));
 }
 
 bool DrmGpuPlatformSupportHost::GpuRemoveGraphicsDevice(

ui/ozone/platform/drm/host/drm_gpu_platform_support_host.h

@@ -10,6 +10,7 @@
 #include "base/callback.h"
 #include "base/observer_list.h"
 #include "base/single_thread_task_runner.h"
+#include "base/synchronization/lock.h"
 #include "ui/display/types/display_constants.h"
 #include "ui/gfx/native_widget_types.h"
 #include "ui/ozone/common/gpu/ozone_gpu_message_params.h"
@@ -99,7 +100,10 @@ class DrmGpuPlatformSupportHost : public GpuPlatformSupportHost,
                               const gfx::Rect& bounds) override;
 
  private:
-  void OnChannelEstablished();
+  void OnChannelEstablished(
+      int host_id,
+      scoped_refptr<base::SingleThreadTaskRunner> send_runner,
+      const base::Callback<void(IPC::Message*)>& send_callback);
   bool OnMessageReceivedForDrmDisplayHostManager(const IPC::Message& message);
   void OnUpdateNativeDisplays(
       const std::vector<DisplaySnapshot_Params>& displays);
@@ -117,7 +121,7 @@ class DrmGpuPlatformSupportHost : public GpuPlatformSupportHost,
                        const std::vector<OverlayCheckReturn_Params>& returns);
 
   int host_id_ = -1;
-  bool channel_established_ = false;
+  base::Lock host_id_lock_;
 
   scoped_refptr<base::SingleThreadTaskRunner> ui_runner_;
   scoped_refptr<base::SingleThreadTaskRunner> send_runner_;

ui/ozone/platform/drm/host/gpu_thread_observer.h

@@ -7,20 +7,19 @@
 
 namespace ui {
 
-// Observes the channel state.
+// Observes the channel state. All calls should happen on the same thread that
+// OzonePlatform::InitializeForUI() is called on. This can be the browser UI
+// thread or the WS thread for mus/mash.
 class GpuThreadObserver {
  public:
   virtual ~GpuThreadObserver() {}
 
   // Called when the GPU process is launched.
-  // This is called from browser IO thread.
   virtual void OnGpuProcessLaunched() = 0;
   // Called when a GPU thread implementation has become available.
-  // This is called from browser UI thread.
   virtual void OnGpuThreadReady() = 0;
   // Called when the GPU thread implementation has ceased to be
   // available.
-  // This is called from browser UI thread.
   virtual void OnGpuThreadRetired() = 0;
 };
 

ui/ozone/public/ozone_platform.cc

@@ -42,8 +42,11 @@ void OzonePlatform::InitializeForUI(const InitParams& args) {
   EnsureInstance();
   if (g_platform_initialized_ui)
     return;
-  g_platform_initialized_ui = true;
   instance_->InitializeUI(args);
+  {
+    base::AutoLock lock(GetOzoneInstanceLock());
+    g_platform_initialized_ui = true;
+  }
   // This is deliberately created after initializing so that the platform can
   // create its own version of DDM.
   DeviceDataManager::CreateInstance();
@@ -56,8 +59,11 @@ void OzonePlatform::InitializeForGPU(const InitParams& args) {
   EnsureInstance();
   if (g_platform_initialized_gpu)
     return;
-  g_platform_initialized_gpu = true;
   instance_->InitializeGPU(args);
+  {
+    base::AutoLock lock(GetOzoneInstanceLock());
+    g_platform_initialized_gpu = true;
+  }
   if (!args.single_process && !instance_callback.Get().is_null())
     std::move(instance_callback.Get()).Run(instance_);
 }