[Ozone-GBM] Adding NativeWindowDelegate to IPC window changes to the GPU

ui/ozone/platform/dri/native_window_delegate_proxy.cc

Index: ui/ozone/platform/dri/native_window_delegate_proxy.cc
diff --git a/ui/ozone/platform/dri/native_window_delegate_proxy.cc b/ui/ozone/platform/dri/native_window_delegate_proxy.cc
new file mode 100644
index 0000000000000000000000000000000000000000..de5017e9badc4969a56580dae1b83494a4585bdc
--- /dev/null
+++ b/ui/ozone/platform/dri/native_window_delegate_proxy.cc
@@ -0,0 +1,47 @@
+// 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/dri/native_window_delegate_proxy.h"
+
+#include "ui/ozone/common/gpu/ozone_gpu_messages.h"
+#include "ui/ozone/platform/dri/gpu_platform_support_host_gbm.h"
+#include "ui/ozone/platform/dri/native_window_manager.h"
+
+namespace ui {
+
+NativeWindowDelegateProxy::NativeWindowDelegateProxy(
+    NativeWindowManager* window_manager,
+    GpuPlatformSupportHostGbm* sender)
+    : widget_(window_manager->AllocateAcceleratedWidget()),
+      window_manager_(window_manager),
+      sender_(sender) {
+  window_manager_->AddNativeWindowDelegate(widget_, this);
+  bool status =
+      sender_->Send(new OzoneGpuMsg_CreateNativeWindowDelegate(widget_));

[alexst (slow to review), 2014/08/18 17:43:47]

Please don't use IPC in the constructor. Non-trivial operations here always lead
to pain down the road. I expect constructors to always succeed quickly and
anything else that has to happen should do so separately. 

[dnicoara, 2014/08/18 18:55:24]

Given that the IPCs are async, there's nothing slow in here. I'd rather not add
an Initialize/Shutdown pattern for this just for the sake of scheduling a
message (especially since they are strongly correlated with object
creation/destruction.

If we can't send this message, then we're pretty much screwed anyways (hence the
following DCHECK).

[alexst (slow to review), 2014/08/18 20:30:34]

I know they are async, but my comment is about this usage pattern in general.
Also, it's in the code guide.
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Doing_Work_in_...

[dnicoara, 2014/08/19 15:53:38]

I've kept that in mind, but this doesn't break that rule since I'm not calling a
virtual function and I'm not failing initialization in any way. Having a "void
Initialize()" called right after the constructor is just extra complexity and
doesn't add any value.

[alexst (slow to review), 2014/08/19 16:02:09]

You are arguing the specifics of the guide that says "Avoid doing complex
initialization in constructors"

If we are going to be specific about details, "in particular, initialization
that can fail"

sender_->Send() can very much fail. 

[dnicoara, 2014/08/19 17:54:02]

Done.

+  DCHECK(status);
+}
+
+NativeWindowDelegateProxy::~NativeWindowDelegateProxy() {
+  window_manager_->RemoveNativeWindowDelegate(widget_);
+  bool status =
+      sender_->Send(new OzoneGpuMsg_DestoryNativeWindowDelegate(widget_));

[alexst (slow to review), 2014/08/18 17:43:47]

As above, please keep the destructor as simple as possible, I prefer we didn't
IPC here.

[dnicoara, 2014/08/19 17:54:02]

Done.

+  DCHECK(status);
+}
+
+gfx::AcceleratedWidget NativeWindowDelegateProxy::GetWidget() {
+  return widget_;
+}
+
+HardwareDisplayController* NativeWindowDelegateProxy::GetController() {
+  NOTREACHED();
+  return NULL;
+}
+
+void NativeWindowDelegateProxy::OnBoundsChanged(const gfx::Rect& bounds) {
+  bool status = sender_->Send(
+      new OzoneGpuMsg_NativeWindowBoundsChanged(widget_, bounds));
+  DCHECK(status);
+}
+
+}  // namespace ui