[3/4][Ozone-Drm] Use numeric ID to map to callback

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

Index: ui/ozone/platform/drm/gpu/drm_device.cc
diff --git a/ui/ozone/platform/drm/gpu/drm_device.cc b/ui/ozone/platform/drm/gpu/drm_device.cc
index fe95a622ca04eb01255310bb612255d75a3a831e..c1391891d4611d1b88badb8a5b8ca28708f492d1 100644
--- a/ui/ozone/platform/drm/gpu/drm_device.cc
+++ b/ui/ozone/platform/drm/gpu/drm_device.cc
@@ -29,6 +29,11 @@ namespace ui {
 
 namespace {
 
+typedef base::Callback<void(uint32_t /* frame */,
+                            uint32_t /* seconds */,
+                            uint32_t /* useconds */,
+                            uint64_t /* id */)> DrmEventHandler;
+
 struct PageFlipPayload {
   PageFlipPayload(const scoped_refptr<base::TaskRunner>& task_runner,
                   const DrmDevice::PageFlipCallback& callback)
@@ -73,23 +78,35 @@ void DrmDestroyDumbBuffer(int fd, uint32_t handle) {
   drmIoctl(fd, DRM_IOCTL_MODE_DESTROY_DUMB, &destroy_request);
 }
 
-void HandlePageFlipEventOnIO(int fd,
-                             unsigned int frame,
-                             unsigned int seconds,
-                             unsigned int useconds,
-                             void* data) {
-  scoped_ptr<PageFlipPayload> payload(static_cast<PageFlipPayload*>(data));
-  payload->task_runner->PostTask(
-      FROM_HERE, base::Bind(payload->callback, frame, seconds, useconds));
-}
+void ProcessDrmEvent(int fd, const DrmEventHandler& callback) {
+  char buffer[1024];
+  int len = read(fd, buffer, sizeof(buffer));
+  if (len == 0)
+    return;
+
+  if (len < static_cast<int>(sizeof(drm_event))) {
+    PLOG(ERROR) << "Failed to read DRM event";
+    return;
+  }
 
-void HandlePageFlipEventOnUI(int fd,
-                             unsigned int frame,
-                             unsigned int seconds,
-                             unsigned int useconds,
-                             void* data) {
-  scoped_ptr<PageFlipPayload> payload(static_cast<PageFlipPayload*>(data));
-  payload->callback.Run(frame, seconds, useconds);
+  int idx = 0;
+  while (idx < len) {
+    drm_event* event = reinterpret_cast<drm_event*>(&buffer[idx]);

[spang, 2015/04/27 17:23:38]

You can't cast a char* to an arbitrary struct. It's undefined behavior.

You can memcpy() into a drm_event instead.

[dnicoara, 2015/04/27 20:22:22]

Why would it be undefined behavior to cast between 2 pointer types? Are you
afraid of bounds checks? DRM is supposed to guarantee complete events, but I can
add some checks.
Seems silly to copy the event twice.

[dnicoara, 2015/04/28 14:42:49]

Done.

+    switch (event->type) {
+      case DRM_EVENT_FLIP_COMPLETE: {
+        drm_event_vblank* vblank = reinterpret_cast<drm_event_vblank*>(event);
+        callback.Run(vblank->sequence, vblank->tv_sec, vblank->tv_usec,
+                     vblank->user_data);
+      } break;
+      case DRM_EVENT_VBLANK:
+        break;
+      default:
+        NOTREACHED();
+        break;
+    }
+
+    idx += event->length;
+  }
 }
 
 bool CanQueryForResources(int fd) {
@@ -102,13 +119,54 @@ bool CanQueryForResources(int fd) {
 
 }  // namespace
 
+class DrmDevice::PageFlipManager
+    : public base::RefCountedThreadSafe<DrmDevice::PageFlipManager> {
+ public:
+  PageFlipManager() : next_id_(0) {}
+  ~PageFlipManager() {}
+
+  void OnPageFlip(uint32_t frame,
+                  uint32_t seconds,
+                  uint32_t useconds,
+                  uint64_t id) {
+    auto it = callback_map_.find(id);
+    if (it == callback_map_.end()) {
+      LOG(WARNING) << "Could not find callback for page flip id=" << id;
+      return;
+    }
+
+    it->second.Run(frame, seconds, useconds);
+    callback_map_.erase(it);
+  }
+
+  uint64_t GetNextId() { return next_id_++; }
+
+  void RegisterCallback(uint64_t id,
+                        const DrmDevice::PageFlipCallback& callback) {
+    DCHECK(callback_map_.find(id) == callback_map_.end());
+    callback_map_.insert(std::make_pair(id, callback));
+  }
+
+ private:
+  uint64_t next_id_;
+
+  std::map<uint64_t, DrmDevice::PageFlipCallback> callback_map_;

[spang, 2015/04/27 17:23:38]

vector?

[dnicoara, 2015/04/27 20:22:22]

Why vector? ... I suppose the number of callbacks registered won't be large, but
this is easier to read.

[spang, 2015/04/27 20:53:42]

You'll only have 1-3 elements right? vector is a better choice in that case.

[dnicoara, 2015/04/28 14:42:48]

Done.

+
+  DISALLOW_COPY_AND_ASSIGN(PageFlipManager);
+};
+
 class DrmDevice::IOWatcher
     : public base::RefCountedThreadSafe<DrmDevice::IOWatcher>,
       public base::MessagePumpLibevent::Watcher {
  public:
   IOWatcher(int fd,
-            const scoped_refptr<base::SingleThreadTaskRunner>& io_task_runner)
-      : io_task_runner_(io_task_runner), paused_(true), fd_(fd) {}
+            const scoped_refptr<base::SingleThreadTaskRunner>& io_task_runner,
+            const scoped_refptr<DrmDevice::PageFlipManager>& page_flip_manager)
+      : main_task_runner_(base::ThreadTaskRunnerHandle::Get()),
+        io_task_runner_(io_task_runner),
+        page_flip_manager_(page_flip_manager),
+        paused_(true),
+        fd_(fd) {}
 
   void SetPaused(bool paused) {
     if (paused_ == paused)
@@ -152,23 +210,32 @@ class DrmDevice::IOWatcher
     done->Signal();
   }
 
+  void OnPageFlipOnIO(uint32_t frame,
+                      uint32_t seconds,
+                      uint32_t useconds,
+                      uint64_t id) {
+    main_task_runner_->PostTask(
+        FROM_HERE,
+        base::Bind(&DrmDevice::PageFlipManager::OnPageFlip, page_flip_manager_,
+                   frame, seconds, useconds, id));
+  }
+
   // base::MessagePumpLibevent::Watcher overrides:
   void OnFileCanReadWithoutBlocking(int fd) override {
     DCHECK(base::MessageLoopForIO::IsCurrent());
     TRACE_EVENT1("drm", "OnDrmEvent", "socket", fd);
 
-    drmEventContext event;
-    event.version = DRM_EVENT_CONTEXT_VERSION;
-    event.page_flip_handler = HandlePageFlipEventOnIO;
-    event.vblank_handler = nullptr;
-
-    drmHandleEvent(fd, &event);
+    ProcessDrmEvent(fd,
+                    base::Bind(&DrmDevice::IOWatcher::OnPageFlipOnIO, this));
   }
 
   void OnFileCanWriteWithoutBlocking(int fd) override { NOTREACHED(); }
 
+  scoped_refptr<base::SingleThreadTaskRunner> main_task_runner_;
   scoped_refptr<base::SingleThreadTaskRunner> io_task_runner_;
 
+  scoped_refptr<DrmDevice::PageFlipManager> page_flip_manager_;
+
   base::MessagePumpLibevent::FileDescriptorWatcher controller_;
 
   bool paused_;
@@ -181,14 +248,17 @@ DrmDevice::DrmDevice(const base::FilePath& device_path)
     : device_path_(device_path),
       file_(device_path,
             base::File::FLAG_OPEN | base::File::FLAG_READ |
-                base::File::FLAG_WRITE) {
+                base::File::FLAG_WRITE),
+      page_flip_manager_(new PageFlipManager()) {
   LOG_IF(FATAL, !file_.IsValid())
       << "Failed to open '" << device_path_.value()
       << "': " << base::File::ErrorToString(file_.error_details());
 }
 
 DrmDevice::DrmDevice(const base::FilePath& device_path, base::File file)
-    : device_path_(device_path), file_(file.Pass()) {
+    : device_path_(device_path),
+      file_(file.Pass()),
+      page_flip_manager_(new PageFlipManager()) {
 }
 
 DrmDevice::~DrmDevice() {
@@ -223,7 +293,8 @@ void DrmDevice::InitializeTaskRunner(
     const scoped_refptr<base::SingleThreadTaskRunner>& task_runner) {
   DCHECK(!task_runner_);
   task_runner_ = task_runner;
-  watcher_ = new IOWatcher(file_.GetPlatformFile(), task_runner_);
+  watcher_ =
+      new IOWatcher(file_.GetPlatformFile(), task_runner_, page_flip_manager_);
 }
 
 ScopedDrmCrtcPtr DrmDevice::GetCrtc(uint32_t crtc_id) {
@@ -307,24 +378,20 @@ bool DrmDevice::PageFlip(uint32_t crtc_id,
 
   // NOTE: Calling drmModeSetCrtc will immediately update the state, though
   // callbacks to already scheduled page flips will be honored by the kernel.
-  scoped_ptr<PageFlipPayload> payload(
-      new PageFlipPayload(base::ThreadTaskRunnerHandle::Get(), callback));
+  uint64_t id = page_flip_manager_->GetNextId();
   if (!drmModePageFlip(file_.GetPlatformFile(), crtc_id, framebuffer,
-                       DRM_MODE_PAGE_FLIP_EVENT, payload.get())) {
+                       DRM_MODE_PAGE_FLIP_EVENT, reinterpret_cast<void*>(id))) {
     // If successful the payload will be removed by a PageFlip event.
-    ignore_result(payload.release());
+    page_flip_manager_->RegisterCallback(id, callback);
 
     // If the flip was requested synchronous or if no watcher has been installed
     // yet, then synchronously handle the page flip events.
     if (is_sync || !watcher_) {
       TRACE_EVENT1("drm", "OnDrmEvent", "socket", file_.GetPlatformFile());
 
-      drmEventContext event;
-      event.version = DRM_EVENT_CONTEXT_VERSION;
-      event.page_flip_handler = HandlePageFlipEventOnUI;
-      event.vblank_handler = nullptr;
-
-      drmHandleEvent(file_.GetPlatformFile(), &event);
+      ProcessDrmEvent(
+          file_.GetPlatformFile(),
+          base::Bind(&PageFlipManager::OnPageFlip, page_flip_manager_));
     }
 
     return true;