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 11 matching lines @@
   }
   glAttachShader(program, shader);
   glDeleteShader(shader);
   CHECK_EQ(static_cast<int>(glGetError()), GL_NO_ERROR);
 }
 
 namespace content {
 
 #if defined(USE_OZONE)
 
+class DisplayConfiguratorObserver : public ui::DisplayConfigurator::Observer {
+ public:
+  DisplayConfiguratorObserver(base::RunLoop* loop) : loop_(loop) {}
+  ~DisplayConfiguratorObserver() override {}
+
+ private:
+  // ui::DisplayConfigurator::Observer overrides:
+  void OnDisplayModeChanged(
+      const ui::DisplayConfigurator::DisplayStateList& outputs) override {
+    loop_->Quit();
+  }
+  void OnDisplayModeChangeFailed(
+      ui::MultipleDisplayState failed_new_state) override {}

[Owen Lin, 2015/01/21 08:10:03]

Crash the program here ?

LOG(FATAL) 

[llandwerlin-old, 2015/01/21 08:44:19]

Acknowledged.

+
+  base::RunLoop* loop_;
+
+  DISALLOW_COPY_AND_ASSIGN(DisplayConfiguratorObserver);
+};
+
 class RenderingHelper::StubOzoneDelegate : public ui::PlatformWindowDelegate {
  public:
-  StubOzoneDelegate() : accelerated_widget_(gfx::kNullAcceleratedWidget) {
+  StubOzoneDelegate(base::RunLoop* loop)
+      : accelerated_widget_(gfx::kNullAcceleratedWidget), loop_(loop) {
     ui_thread_gpu_.Initialize();
     platform_window_ = ui::OzonePlatform::GetInstance()->CreatePlatformWindow(
         this, gfx::Rect());
   }
   virtual ~StubOzoneDelegate() {}
 
-  void OnBoundsChanged(const gfx::Rect& new_bounds) override {}
+  void OnBoundsChanged(const gfx::Rect& new_bounds) override { loop_->Quit(); }

[Pawel Osciak, 2015/01/21 08:13:35]

Does this pass git cl format?

[llandwerlin-old, 2015/01/21 08:44:19]

Yes.

 
   void OnDamageRect(const gfx::Rect& damaged_region) override {}
 
   void DispatchEvent(ui::Event* event) override {}
 
   void OnCloseRequest() override {}
   void OnClosed() override {}
 
   void OnWindowStateChanged(ui::PlatformWindowState new_state) override {}
 
@@ skipping 10 matching lines @@
   }
 
   gfx::Size GetSize() { return platform_window_->GetBounds().size(); }
 
   ui::PlatformWindow* platform_window() const { return platform_window_.get(); }
 
  private:
   ui::UiThreadGpu ui_thread_gpu_;
   scoped_ptr<ui::PlatformWindow> platform_window_;
   gfx::AcceleratedWidget accelerated_widget_;
+  base::RunLoop* loop_;
 
   DISALLOW_COPY_AND_ASSIGN(StubOzoneDelegate);
 };
 
 #endif  // defined(USE_OZONE)
 
 RenderingHelperParams::RenderingHelperParams()
     : rendering_fps(0), warm_up_iterations(0), render_as_thumbnails(false) {
 }
 
@@ skipping 99 matching lines @@
                           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)
-  platform_window_delegate_.reset(new RenderingHelper::StubOzoneDelegate());
+  scoped_ptr<base::RunLoop> window_loop(new base::RunLoop);

[Owen Lin, 2015/01/21 08:10:03]

Can we just use a local variable?
base::RunLoop window_loop;

Same for others.

How about renaming it to wait_resize_loop here and in the StubOzoneDelegate to
show the only purpose of the loop.

[llandwerlin-old, 2015/01/21 08:44:19]

Acknowledged.

+
+  platform_window_delegate_.reset(
+      new RenderingHelper::StubOzoneDelegate(window_loop.get()));

[Pawel Osciak, 2015/01/21 08:13:35]

Is it safe to pass a local variable pointer to the class member here?

[llandwerlin-old, 2015/01/21 08:44:19]

Thanks, I should reset the class member after the loop is stop.

   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(
+      base::MessageLoop::current());
+  scoped_ptr<base::RunLoop> display_loop(new base::RunLoop);

[Owen Lin, 2015/01/21 08:10:03]

Same. How about renaming it to wait_mode_change_loop.

[Pawel Osciak, 2015/01/21 08:13:35]

(new base::RunLoop());

+  scoped_ptr<DisplayConfiguratorObserver> display_observer(
+      new DisplayConfiguratorObserver(display_loop.get()));
+  display_configurator_.reset(new ui::DisplayConfigurator);
+  display_configurator_->AddObserver(display_observer.get());
   display_configurator_->Init(true);
   display_configurator_->ForceInitialConfigure(0);
+  display_configurator_->SetDisplayMode(ui::MULTIPLE_DISPLAY_STATE_SINGLE);

[Owen Lin, 2015/01/21 08:10:03]

Are we sure the "OnDisplayModeChanged" would be called? What if the current mode
is SINGLE already.

[llandwerlin-old, 2015/01/21 08:44:19]

As far as I can tell the ForceInitialConfigure will always notify the observers
:

https://code.google.com/p/chromium/codesearch#chromium/src/ui/display/chromeo...

+  // Wait for the display to be give probe the screen's size.
+  display_loop->Run();
+  display_configurator_->RemoveObserver(display_observer.get());
   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.

[Owen Lin, 2015/01/21 08:10:03]

Here is even more vague. Why we are sure the window will be resized ?

[llandwerlin-old, 2015/01/21 08:44:19]

We initially create the window with a gfx::Rect() -> 0x0 :

https://code.google.com/p/chromium/codesearch#chromium/src/content/common/gpu...

After calling SetBounds(), will should be notified via
PlatformWindowDelegate::OnBoundsChanged().

+  window_loop->Run();
 #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