[1/4][Ozone-Drm] Process display hotplug events sequentially

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

 // Copyright 2014 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 "ui/ozone/platform/drm/host/drm_native_display_delegate.h"
 
 #include <stdio.h>
 
 #include "base/logging.h"
 #include "base/thread_task_runner_handle.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/threading/worker_pool.h"
 #include "ui/display/types/display_snapshot.h"
 #include "ui/display/types/native_display_observer.h"
 #include "ui/events/ozone/device/device_event.h"
 #include "ui/events/ozone/device/device_manager.h"
 #include "ui/ozone/common/display_snapshot_proxy.h"
 #include "ui/ozone/common/display_util.h"
 #include "ui/ozone/common/gpu/ozone_gpu_messages.h"
 #include "ui/ozone/platform/drm/host/display_manager.h"
 #include "ui/ozone/platform/drm/host/drm_gpu_platform_support_host.h"
 
 namespace ui {
 
 namespace {
 
 typedef base::Callback<void(const base::FilePath&, base::File)>
     OnOpenDeviceReplyCallback;
 
-void OpenDeviceOnWorkerThread(
-    const base::FilePath& path,
-    const scoped_refptr<base::TaskRunner>& reply_runner,
-    const OnOpenDeviceReplyCallback& callback) {
+const char* kDisplayActionString[] = {
+    "ADD",
+    "REMOVE",
+    "CHANGE",
+};
+
+base::File OpenDrmDevice(const base::FilePath& path) {
   base::File file(path, base::File::FLAG_OPEN | base::File::FLAG_READ |
                             base::File::FLAG_WRITE);
 
   base::File::Info info;
   file.GetInfo(&info);
 
   CHECK(!info.is_directory);
   CHECK(path.DirName() == base::FilePath("/dev/dri"));
 
-  if (file.IsValid()) {
-    reply_runner->PostTask(
-        FROM_HERE, base::Bind(callback, path, base::Passed(file.Pass())));
+  if (!file.IsValid()) {
+    LOG(ERROR) << "Failed to open " << path.value() << ": "
+               << base::File::ErrorToString(file.error_details());
   }
+
+  return file.Pass();
+}
+
+void OpenDeviceOnWorkerThread(
+    const base::FilePath& path,
+    const scoped_refptr<base::TaskRunner>& reply_runner,
+    const OnOpenDeviceReplyCallback& callback) {
+  base::File file = OpenDrmDevice(path);
+  reply_runner->PostTask(FROM_HERE,
+                         base::Bind(callback, path, base::Passed(file.Pass())));
 }
 
 class DrmDisplaySnapshotProxy : public DisplaySnapshotProxy {
  public:
   DrmDisplaySnapshotProxy(const DisplaySnapshot_Params& params,
                           DisplayManager* display_manager)
       : DisplaySnapshotProxy(params), display_manager_(display_manager) {
     display_manager_->RegisterDisplay(this);
   }
 
@@ skipping 14 matching lines @@
     DeviceManager* device_manager,
     DisplayManager* display_manager,
     const base::FilePath& primary_graphics_card_path)
     : proxy_(proxy),
       device_manager_(device_manager),
       display_manager_(display_manager),
       primary_graphics_card_path_(primary_graphics_card_path),
       has_dummy_display_(false),
       weak_ptr_factory_(this) {
   proxy_->RegisterHandler(this);
+  drm_devices_.insert(primary_graphics_card_path);
 }
 
 DrmNativeDisplayDelegate::~DrmNativeDisplayDelegate() {
   device_manager_->RemoveObserver(this);
   proxy_->UnregisterHandler(this);
 }
 
 void DrmNativeDisplayDelegate::Initialize() {
   device_manager_->AddObserver(this);
   device_manager_->ScanDevices(this);
@@ skipping 128 matching lines @@
 }
 
 void DrmNativeDisplayDelegate::RemoveObserver(NativeDisplayObserver* observer) {
   observers_.RemoveObserver(observer);
 }
 
 void DrmNativeDisplayDelegate::OnDeviceEvent(const DeviceEvent& event) {
   if (event.device_type() != DeviceEvent::DISPLAY)
     return;
 
-  switch (event.action_type()) {
-    case DeviceEvent::ADD:
-      VLOG(1) << "Got display added event for " << event.path().value();
-      // The default card is a special case since it needs to be opened early on
-      // the GPU process in order to initialize EGL. If it is opened here as
-      // well, it will cause a race with opening it in the GPU process and the
-      // GPU process may fail initialization.
-      // TODO(dnicoara) Remove this when the media stack does not require super
-      // early initialization.
-      if (event.path() == primary_graphics_card_path_)
+  event_queue_.push(DisplayEvent(event.action_type(), event.path()));
+  if (event_queue_.size() == 1)
+    ProcessEvent();
+}
+
+void DrmNativeDisplayDelegate::ProcessEvent() {
+  while (!event_queue_.empty()) {

[spang, 2015/04/27 21:54:33]

I think it would be clearer to do:

while (!event_queue.empty() && !task_pending_) {
  // ....
}

.. for appropriately set task_pending.

Then call ProcessEvent() unconditionally.

[dnicoara, 2015/04/28 14:53:04]

Done.

+    DisplayEvent event = event_queue_.front();

[spang, 2015/04/27 21:54:33]

Why can't you pop here?

[dnicoara, 2015/04/28 14:53:04]

The event would act as the |task_pending_| bool.

+    VLOG(1) << "Got display event " << kDisplayActionString[event.action_type]
+            << " for " << event.path.value();
+    switch (event.action_type) {
+      case DeviceEvent::ADD:
+        if (drm_devices_.find(event.path) == drm_devices_.end()) {
+          drm_devices_.insert(event.path);
+          base::WorkerPool::PostTask(
+              FROM_HERE,
+              base::Bind(
+                  &OpenDeviceOnWorkerThread, event.path,
+                  base::ThreadTaskRunnerHandle::Get(),
+                  base::Bind(&DrmNativeDisplayDelegate::OnAddGraphicsDevice,
+                             weak_ptr_factory_.GetWeakPtr())),
+              false /* task_is_slow */);
+
+          return;
+        }
+        break;
+      case DeviceEvent::CHANGE:
+        base::ThreadTaskRunnerHandle::Get()->PostTask(
+            FROM_HERE,
+            base::Bind(&DrmNativeDisplayDelegate::OnUpdateGraphicsDevice,
+                       weak_ptr_factory_.GetWeakPtr()));
         return;
+      case DeviceEvent::REMOVE:
+        DCHECK(event.path != primary_graphics_card_path_)
+            << "Removing primary graphics card";
+        auto it = drm_devices_.find(event.path);
+        if (it != drm_devices_.end()) {
+          drm_devices_.erase(it);
+          base::ThreadTaskRunnerHandle::Get()->PostTask(
+              FROM_HERE,
+              base::Bind(&DrmNativeDisplayDelegate::OnRemoveGraphicsDevice,
+                         weak_ptr_factory_.GetWeakPtr(), event.path));
+          return;
+        }
+        break;
+    }
 
-      base::WorkerPool::PostTask(
-          FROM_HERE,
-          base::Bind(&OpenDeviceOnWorkerThread, event.path(),
-                     base::ThreadTaskRunnerHandle::Get(),
-                     base::Bind(&DrmNativeDisplayDelegate::OnNewGraphicsDevice,
-                                weak_ptr_factory_.GetWeakPtr())),
-          false /* task_is_slow */);
-      return;
-    case DeviceEvent::CHANGE:
-      VLOG(1) << "Got display changed event for " << event.path().value();
-      break;
-    case DeviceEvent::REMOVE:
-      VLOG(1) << "Got display removed event for " << event.path().value();
-      // It shouldn't be possible to remove this device.
-      DCHECK(primary_graphics_card_path_ != event.path())
-          << "Got event to remove primary graphics card "
-          << event.path().value();
-      proxy_->Send(new OzoneGpuMsg_RemoveGraphicsDevice(event.path()));
-      break;
+    // The event was a no-op, so remove it and process the next one.
+    event_queue_.pop();
+  }
+}
+
+void DrmNativeDisplayDelegate::OnAddGraphicsDevice(const base::FilePath& path,
+                                                   base::File file) {
+  if (file.IsValid()) {
+    proxy_->Send(new OzoneGpuMsg_AddGraphicsDevice(
+        path, base::FileDescriptor(file.Pass())));
+    FOR_EACH_OBSERVER(NativeDisplayObserver, observers_,
+                      OnConfigurationChanged());
   }
 
+  event_queue_.pop();
+  ProcessEvent();
+}
+
+void DrmNativeDisplayDelegate::OnUpdateGraphicsDevice() {
   FOR_EACH_OBSERVER(NativeDisplayObserver, observers_,
                     OnConfigurationChanged());
+  event_queue_.pop();
+  ProcessEvent();
 }
 
-void DrmNativeDisplayDelegate::OnNewGraphicsDevice(const base::FilePath& path,
-                                                   base::File file) {
-  DCHECK(file.IsValid());
-  proxy_->Send(new OzoneGpuMsg_AddGraphicsDevice(
-      path, base::FileDescriptor(file.Pass())));
-
+void DrmNativeDisplayDelegate::OnRemoveGraphicsDevice(
+    const base::FilePath& path) {
+  proxy_->Send(new OzoneGpuMsg_RemoveGraphicsDevice(path));
   FOR_EACH_OBSERVER(NativeDisplayObserver, observers_,
                     OnConfigurationChanged());
+  event_queue_.pop();
+  ProcessEvent();
 }
 
 void DrmNativeDisplayDelegate::OnChannelEstablished(
     int host_id,
     scoped_refptr<base::SingleThreadTaskRunner> send_runner,
     const base::Callback<void(IPC::Message*)>& send_callback) {
+  drm_devices_.clear();
+  drm_devices_.insert(primary_graphics_card_path_);
   device_manager_->ScanDevices(this);
   FOR_EACH_OBSERVER(NativeDisplayObserver, observers_,
                     OnConfigurationChanged());
 }
 
 void DrmNativeDisplayDelegate::OnChannelDestroyed(int host_id) {
   // If the channel got destroyed in the middle of a configuration then just
   // respond with failure.
   if (!get_displays_callback_.is_null()) {
     base::ThreadTaskRunnerHandle::Get()->PostTask(
@@ skipping 71 matching lines @@
     set_hdcp_state_callback_map_.erase(it);
   }
 }
 
 void DrmNativeDisplayDelegate::RunUpdateDisplaysCallback(
     const GetDisplaysCallback& callback) const {
   callback.Run(displays_.get());
 }
 
 }  // namespace ui