Create GPU child window on new thread.

gpu/ipc/service/child_window_surface_win.cc

Index: gpu/ipc/service/child_window_surface_win.cc
diff --git a/gpu/ipc/service/child_window_surface_win.cc b/gpu/ipc/service/child_window_surface_win.cc
index 762fbd117391233704e07375d64474912e305e1c..f9013e8f8e5038092557f30e3686c8cb4aba011a 100644
--- a/gpu/ipc/service/child_window_surface_win.cc
+++ b/gpu/ipc/service/child_window_surface_win.cc
@@ -7,6 +7,8 @@
 #include <memory>
 
 #include "base/compiler_specific.h"
+#include "base/memory/ptr_util.h"
+#include "base/threading/thread.h"
 #include "base/win/scoped_hdc.h"
 #include "base/win/wrapped_window_proc.h"
 #include "gpu/ipc/common/gpu_messages.h"
@@ -22,6 +24,16 @@
 
 namespace gpu {
 
+// This owns the thread and contains data that's shared between the threads.
+struct SharedData {
+  SharedData() : thread("Window owner thread") {}
+
+  base::Lock rect_lock;
+  gfx::Rect rect_to_clear;
+
+  base::Thread thread;
+};
+
 namespace {
 
 ATOM g_window_class;
@@ -37,15 +49,13 @@ LRESULT CALLBACK IntermediateWindowProc(HWND window,
     case WM_PAINT:
       PAINTSTRUCT paint;
       if (BeginPaint(window, &paint)) {
-        ChildWindowSurfaceWin* window_surface =
-            reinterpret_cast<ChildWindowSurfaceWin*>(
-                gfx::GetWindowUserData(window));
-        DCHECK(window_surface);
-
-        // Wait to clear the contents until a GL draw occurs, as otherwise an
-        // unsightly black flash may happen if the GL contents are still
-        // transparent.
-        window_surface->InvalidateWindowRect(gfx::Rect(paint.rcPaint));
+        SharedData* shared_data =
+            reinterpret_cast<SharedData*>(gfx::GetWindowUserData(window));
+        DCHECK(shared_data);
+
+        base::AutoLock lock(shared_data->rect_lock);
+        shared_data->rect_to_clear.Union(gfx::Rect(paint.rcPaint));

[stanisc, 2016/08/04 02:03:35]

Should the lock scope end after this line?

[jbauman, 2016/08/04 20:45:51]

Done.

+
         EndPaint(window, &paint);
       }
       return 0;
@@ -70,6 +80,34 @@ void InitializeWindowClass() {
     return;
   }
 }
+
+void CreateChildWindow(HWND parent,

[stanisc, 2016/08/04 02:03:35]

It would be good to indicate in comments which thread each method runs on.

[jbauman, 2016/08/04 20:45:51]

Done.

+                       base::WaitableEvent* event,
+                       SharedData* shared_data,

[stanisc, 2016/08/04 02:03:35]

Should this be const pointer?

[jbauman, 2016/08/04 20:45:51]

No, SetWindowUserData takes a non-const pointer.

+                       HWND* result) {
+  InitializeWindowClass();
+  DCHECK(g_window_class);
+
+  RECT windowRect;
+  GetClientRect(parent, &windowRect);
+
+  HWND window = CreateWindowEx(
+      WS_EX_NOPARENTNOTIFY, reinterpret_cast<wchar_t*>(g_window_class), L"",
+      WS_CHILDWINDOW | WS_DISABLED | WS_VISIBLE, 0, 0,
+      windowRect.right - windowRect.left, windowRect.bottom - windowRect.top,
+      ui::GetHiddenWindow(), NULL, NULL, NULL);
+  *result = window;
+  gfx::SetWindowUserData(window, shared_data);
+  event->Signal();
+}
+
+void DestroySharedData(std::unique_ptr<SharedData> shared_data) {
+  shared_data->thread.Stop();
+}
+
+void DestroyWindowOnThread(HWND window) {
+  DestroyWindow(window);
+}
 }

[stanisc, 2016/08/04 02:03:35]

Add  // namespace

[jbauman, 2016/08/04 20:45:51]

Done.

 
 ChildWindowSurfaceWin::ChildWindowSurfaceWin(GpuChannelManager* manager,
@@ -117,18 +155,19 @@ EGLConfig ChildWindowSurfaceWin::GetConfig() {
 bool ChildWindowSurfaceWin::InitializeNativeWindow() {
   if (window_)
     return true;
-  InitializeWindowClass();
-  DCHECK(g_window_class);
 
-  RECT windowRect;
-  GetClientRect(parent_window_, &windowRect);
+  shared_data_ = base::MakeUnique<SharedData>();
+
+  base::Thread::Options options(base::MessageLoop::TYPE_UI, 0);
+  shared_data_->thread.StartWithOptions(options);
+
+  base::WaitableEvent event(base::WaitableEvent::ResetPolicy::AUTOMATIC,
+                            base::WaitableEvent::InitialState::NOT_SIGNALED);
+  shared_data_->thread.task_runner()->PostTask(
+      FROM_HERE, base::Bind(&CreateChildWindow, parent_window_, &event,
+                            shared_data_.get(), &window_));
+  event.Wait();
 
-  window_ = CreateWindowEx(
-      WS_EX_NOPARENTNOTIFY, reinterpret_cast<wchar_t*>(g_window_class), L"",
-      WS_CHILDWINDOW | WS_DISABLED | WS_VISIBLE, 0, 0,
-      windowRect.right - windowRect.left, windowRect.bottom - windowRect.top,
-      ui::GetHiddenWindow(), NULL, NULL, NULL);
-  gfx::SetWindowUserData(window_, this);
   manager_->delegate()->SendAcceleratedSurfaceCreatedChildWindow(parent_window_,
                                                                  window_);
   return true;
@@ -205,26 +244,28 @@ gfx::SwapResult ChildWindowSurfaceWin::PostSubBuffer(int x,
   return result;
 }
 
-void ChildWindowSurfaceWin::InvalidateWindowRect(const gfx::Rect& rect) {
-  rect_to_clear_.Union(rect);
-}
-
 void ChildWindowSurfaceWin::ClearInvalidContents() {
-  if (!rect_to_clear_.IsEmpty()) {
+  base::AutoLock lock(shared_data_->rect_lock);
+  if (!shared_data_->rect_to_clear.IsEmpty()) {
     base::win::ScopedGetDC dc(window_);

[stanisc, 2016/08/04 02:03:35]

Is this running on the main thread?
Is it safe to get DC on a thread different than the window thread?

[jbauman, 2016/08/04 20:45:51]

Yeah, it's fine to do GetDC on any thread (or even a different process).

 
-    RECT rect = rect_to_clear_.ToRECT();
+    RECT rect = shared_data_->rect_to_clear.ToRECT();
 
     // DirectComposition composites with the contents under the SwapChain,
     // so ensure that's cleared. GDI treats black as transparent.
     FillRect(dc, &rect, reinterpret_cast<HBRUSH>(GetStockObject(BLACK_BRUSH)));
-    rect_to_clear_ = gfx::Rect();
+    shared_data_->rect_to_clear = gfx::Rect();
   }
 }
 
 ChildWindowSurfaceWin::~ChildWindowSurfaceWin() {
-  gfx::SetWindowUserData(window_, nullptr);
-  DestroyWindow(window_);
+  if (shared_data_) {
+    scoped_refptr<base::TaskRunner> task_runner =
+        shared_data_->thread.task_runner();
+    task_runner->PostTaskAndReply(
+        FROM_HERE, base::Bind(&DestroyWindowOnThread, window_),
+        base::Bind(&DestroySharedData, base::Passed(std::move(shared_data_))));

[stanisc, 2016/08/04 02:03:35]

Clever solution!

+  }
 }
 
 }  // namespace gpu