[8/8][Ozone-Dri] Pass DRM FD to GPU process on hotplug event

ui/ozone/platform/dri/dri_gpu_platform_support.cc

Index: ui/ozone/platform/dri/dri_gpu_platform_support.cc
diff --git a/ui/ozone/platform/dri/dri_gpu_platform_support.cc b/ui/ozone/platform/dri/dri_gpu_platform_support.cc
index 010ef8c44ee5b81017901ccd2d1c68ebc3d1553d..c4f01c1d518579e9a9e5dc28de6c6636d0638c43 100644
--- a/ui/ozone/platform/dri/dri_gpu_platform_support.cc
+++ b/ui/ozone/platform/dri/dri_gpu_platform_support.cc
@@ -16,6 +16,7 @@
 #include "ui/ozone/platform/dri/dri_window_delegate_impl.h"
 #include "ui/ozone/platform/dri/dri_window_delegate_manager.h"
 #include "ui/ozone/platform/dri/dri_wrapper.h"
+#include "ui/ozone/platform/dri/dri_wrapper_generator.h"
 #include "ui/ozone/platform/dri/native_display_delegate_dri.h"
 #include "ui/ozone/public/ozone_switches.h"
 
@@ -173,14 +174,16 @@ DriGpuPlatformSupport::DriGpuPlatformSupport(
     DrmDeviceManager* drm_device_manager,
     DriWindowDelegateManager* window_manager,
     ScreenManager* screen_manager,
-    scoped_ptr<NativeDisplayDelegateDri> ndd)
+    scoped_ptr<NativeDisplayDelegateDri> ndd,
+    scoped_ptr<DriWrapperGenerator> wrapper_generator)
     : sender_(NULL),
-      drm_(drm),
       drm_device_manager_(drm_device_manager),
       window_manager_(window_manager),
       screen_manager_(screen_manager),
-      ndd_(ndd.Pass()) {
-  ndd_->AddGraphicsDevice(drm_);
+      ndd_(ndd.Pass()),
+      wrapper_generator_(wrapper_generator.Pass()) {
+  wrappers_[drm->device_path()] = drm;
+  ndd_->AddGraphicsDevice(drm);

[spang, 2015/02/11 21:13:21]

Why are the open devices tracked both in DriGpuPlatformSupportHost and in
NativeDisplayDelegateDri?

When can we clean this code up to avoid using the browser process "display
configurator delegate" interface in the gpu process? At this point, it's just a
liability by holding the GPU implementation to an interface that isn't
appropriate for it.

[dnicoara, 2015/02/11 22:21:14]

Just to have the list handy for the operations. Would you like an accessor on
NativeDisplayDelegateDri?

I kinda hate the fact that we just have DriGpuPlatformSupport be a collection of
everything. I'd rather have NativeDisplayDelegateDri be an IPC message handler.
Then all the display configuration messages (and related state) can be isolated
from everything else.
We can drop the interface anytime.

[spang, 2015/02/11 22:47:56]

It's more than just dropping inheritance. We also need to rationalize the
relationship between "DriGpuPlatformSupport" and NativeDisplayDelegateDri. And
it wouldn't be appropriate to continue using the current name, since its
suggestive of the browser process delegate to DisplayConfigurator.

[spang, 2015/02/11 22:52:54]

I would ideally hope these configuration tasks would be done here, or there, but
not a mix of the two.

There is no sensibly definable boundary currently.

[dnicoara, 2015/02/11 23:04:40]

I'm sorry, I was too vague. The only configuration taking place here would be
the initialization of the IO task runner. But I can push all that into NDDDri
and just forward the task runner.

[spang, 2015/02/11 23:11:55]

There's plenty of business logic for display configuration in here. Especially
in OnConfigureNativeDisplay.

This CL adds more, in OnAddGraphicsDevice.

Please define clear roles for these classes and stick to them.

   filter_ = new DriGpuPlatformSupportMessageFilter(
       window_manager, base::Bind(&DriGpuPlatformSupport::SetIOTaskRunner,
                                  base::Unretained(this)),
@@ -347,12 +350,34 @@ void DriGpuPlatformSupport::OnRelinquishDisplayControl() {
   ndd_->RelinquishDisplayControl();
 }
 
-void DriGpuPlatformSupport::OnAddGraphicsDevice(const base::FilePath& path) {
-  NOTIMPLEMENTED();
+void DriGpuPlatformSupport::OnAddGraphicsDevice(
+    const base::FilePath& path,
+    const base::FileDescriptor& fd) {
+  base::File file(fd.fd);
+  auto it = wrappers_.find(path);
+  // Device already registered.
+  if (it != wrappers_.end())
+    return;
+
+  scoped_refptr<DriWrapper> wrapper =
+      wrapper_generator_->CreateWrapper(path, file.Pass());
+  wrappers_[path] = wrapper;
+  if (io_task_runner_)
+    wrapper->InitializeTaskRunner(io_task_runner_);
+
+  ndd_->AddGraphicsDevice(wrapper);
 }
 
 void DriGpuPlatformSupport::OnRemoveGraphicsDevice(const base::FilePath& path) {
-  NOTIMPLEMENTED();
+  auto it = wrappers_.find(path);
+  if (it == wrappers_.end()) {
+    LOG(ERROR) << "Got message to remove non-existent device '" << path.value()
+               << "'";
+    return;
+  }
+
+  ndd_->RemoveGraphicsDevice(it->second);
+  wrappers_.erase(it);
 }
 
 void DriGpuPlatformSupport::RelinquishGpuResources(
@@ -366,7 +391,8 @@ void DriGpuPlatformSupport::SetIOTaskRunner(
   base::CommandLine* cmd = base::CommandLine::ForCurrentProcess();
   // Only surfaceless path supports async page flips.
   if (cmd->HasSwitch(switches::kOzoneUseSurfaceless)) {
-    drm_->InitializeTaskRunner(io_task_runner_);
+    for (const auto& pair : wrappers_)
+      pair.second->InitializeTaskRunner(io_task_runner_);
   }
 }