gpu: media: RenderingHelper: wait for the display & window to be ready

content/common/gpu/media/rendering_helper.cc

 // Copyright 2013 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 "content/common/gpu/media/rendering_helper.h"
 
 #include <algorithm>
 #include <numeric>
 #include <vector>
 
 #include "base/bind.h"
 #include "base/callback_helpers.h"
 #include "base/command_line.h"
 #include "base/mac/scoped_nsautorelease_pool.h"
 #include "base/message_loop/message_loop.h"
+#include "base/run_loop.h"
 #include "base/strings/stringize_macros.h"
 #include "base/synchronization/waitable_event.h"
 #include "base/time/time.h"
 #include "ui/gl/gl_context.h"
 #include "ui/gl/gl_implementation.h"
 #include "ui/gl/gl_surface.h"
 
 #if defined(OS_WIN)
 #include <windows.h>
 #endif
 
 #if defined(USE_X11)
 #include "ui/gfx/x/x11_types.h"
 #endif
 
 #if defined(ARCH_CPU_X86_FAMILY) && defined(USE_X11)
 #include "ui/gl/gl_surface_glx.h"
 #define GL_VARIANT_GLX 1
 #else
 #include "ui/gl/gl_surface_egl.h"
 #define GL_VARIANT_EGL 1
 #endif
 
 #if defined(USE_OZONE)
 #if defined(OS_CHROMEOS)
 #include "ui/display/chromeos/display_configurator.h"
+#include "ui/display/types/display_constants.h"
 #endif  // defined(OS_CHROMEOS)
 #include "ui/ozone/public/ozone_platform.h"
 #include "ui/ozone/public/ui_thread_gpu.h"
 #include "ui/platform_window/platform_window.h"
 #include "ui/platform_window/platform_window_delegate.h"
 #endif  // defined(USE_OZONE)
 
 // Helper for Shader creation.
 static void CreateShader(GLuint program,
                          GLenum type,
@@ skipping 169 matching lines @@
                           XHeightOfScreen(screen),
                           0 /* border width */,
                           depth,
                           CopyFromParent /* class */,
                           CopyFromParent /* visual */,
                           (CWBackPixel | CWOverrideRedirect),
                           &window_attributes);
   XStoreName(display, window_, "VideoDecodeAcceleratorTest");
   XSelectInput(display, window_, ExposureMask);
   XMapWindow(display, window_);
 #elif defined(USE_OZONE)

[dnicoara, 2015/01/21 19:21:35]

Spoke with spang@ and now I understand what's going on. 

 - OzonePlatform is initialized on the main message loop declared in "int
main()" in the unittests.
 - RenderingHelper::Initialize is posted on the rendering helper thread.

This is an issue since Ozone expects all calls into the platform to be performed
from the same thread. I'm guessing that the window creation (and in Ozone's
case, display configuration) need to be performed separately on the main thread.
Then pass |window_| as a parameter to RenderingHelper::Initialize().

[Owen Lin, 2015/01/22 03:46:44]

In addition, maybe we can move some code to InitilizeOneOff? That's in the main
thread. And I think we need to do the display configuration only once.

[llandwerlin-old, 2015/01/22 11:03:18]

Doing the window creation in InitializeOnOff means we're going to keep the
window as a global variable.

Maybe we can split the Initialize method in 2, with one part executed on the
main thread (window creation), one part on the rendering thread (gl context
setup).

[llandwerlin-old, 2015/01/22 11:03:18]

Thanks for the explanation.

[Owen Lin, 2015/01/23 08:57:38]

Good idea.

   platform_window_delegate_.reset(new RenderingHelper::StubOzoneDelegate());
   window_ = platform_window_delegate_->accelerated_widget();
 #if defined(OS_CHROMEOS)
-  display_configurator_.reset(new ui::DisplayConfigurator());
+  // We hold onto the main loop here to wait for the DisplayController
+  // to give us the size of the display so we can create a window of
+  // the same size.
+  base::MessageLoop::ScopedNestableTaskAllower nest_loop(

[dnicoara, 2015/01/21 19:21:35]

You shouldn't need this anymore.

[llandwerlin-old, 2015/01/22 11:03:18]

Acknowledged.

+      base::MessageLoop::current());
+  display_configurator_.reset(new ui::DisplayConfigurator);
   display_configurator_->Init(true);
   display_configurator_->ForceInitialConfigure(0);
+  // Make sure all the display configuration is applied.
+  base::RunLoop().RunUntilIdle();

[Owen Lin, 2015/01/22 03:46:44]

The correctness of using "RunUntilIdle()" here is highly depending on the
implementation inside display_configurator. If there is another thread involved,
it becomes flaky.
I think we should avoid using it.

[llandwerlin-old, 2015/01/22 11:03:18]

I agree, posting a message to a different thread and waiting for the response
might leave the message loop empty.

+  display_configurator_->SetDisplayMode(ui::MULTIPLE_DISPLAY_STATE_SINGLE);
+  // Make sure all the display configuration is applied.
+  base::RunLoop().RunUntilIdle();
   platform_window_delegate_->platform_window()->SetBounds(
       gfx::Rect(display_configurator_->framebuffer_size()));
 #else
   platform_window_delegate_->platform_window()->SetBounds(gfx::Rect(800, 600));
 #endif
+  // Wait for the window to be resized.
+  base::RunLoop().RunUntilIdle();
 #else
 #error unknown platform
 #endif
   CHECK(window_ != gfx::kNullAcceleratedWidget);
 
   gl_surface_ = gfx::GLSurface::CreateViewGLSurface(window_);
   screen_size_ = gl_surface_->GetSize();
 
   gl_context_ = gfx::GLContext::CreateGLContext(
       NULL, gl_surface_.get(), gfx::PreferIntegratedGpu);
@@ skipping 529 matching lines @@
   // When the rendering falls behind, drops frames.
   while (scheduled_render_time_ < target) {
     scheduled_render_time_ += frame_duration_;
     DropOneFrameForAllVideos();
   }
 
   message_loop_->PostDelayedTask(
       FROM_HERE, render_task_.callback(), target - now);
 }
 }  // namespace content