Break //ui/gl/ dependency on //ui/ozone.

ui/gl/gl_surface_egl.h

 // Copyright (c) 2012 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.
 
 #ifndef UI_GL_GL_SURFACE_EGL_H_
 #define UI_GL_GL_SURFACE_EGL_H_
 
 #if defined(OS_WIN)
 #include <windows.h>
 #endif
 
 #include <memory>
 #include <string>
 
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
 #include "base/macros.h"
 #include "base/time/time.h"
 #include "build/build_config.h"
 #include "ui/gfx/geometry/size.h"
 #include "ui/gfx/vsync_provider.h"
 #include "ui/gl/gl_bindings.h"
 #include "ui/gl/gl_export.h"
 #include "ui/gl/gl_surface.h"
 #include "ui/gl/gl_surface_overlay.h"
 
 namespace gl {
 
-// Get default EGL display for GLSurfaceEGL (differs by platform).
-EGLNativeDisplayType GetPlatformDefaultEGLNativeDisplay();
-
 // If adding a new type, also add it to EGLDisplayType in
 // tools/metrics/histograms/histograms.xml. Don't remove or reorder entries.
 enum DisplayType {
   DEFAULT = 0,
   SWIFT_SHADER = 1,
   ANGLE_WARP = 2,
   ANGLE_D3D9 = 3,
   ANGLE_D3D11 = 4,
   ANGLE_OPENGL = 5,
   ANGLE_OPENGLES = 6,
   DISPLAY_TYPE_MAX = 7,
 };
 
 GL_EXPORT void GetEGLInitDisplays(bool supports_angle_d3d,
                                   bool supports_angle_opengl,
                                   const base::CommandLine* command_line,
                                   std::vector<DisplayType>* init_displays);
 
 // Interface for EGL surface.
 class GL_EXPORT GLSurfaceEGL : public GLSurface {
  public:
   GLSurfaceEGL();
 
   // Implement GLSurface.
   EGLDisplay GetDisplay() override;
   EGLConfig GetConfig() override;
   GLSurface::Format GetFormat() override;
 
+  // Sets the native display for EGL. Should be set before calling
+  // InitializeOneOff() or InitializeDisplay().
+  static void SetNativeDisplay(EGLNativeDisplayType native_display);

[piman, 2016/07/06 21:02:16]

Could we instead pass the native display explicitly as a parameter to
InitializeOneOff/InitializeDisplay ?

[kylechar, 2016/07/07 13:18:05]

The problem with doing that is DriverEGL::GetPlatfromExtensions() also calls
GLSurfaceEGL::InitializeDisplay(). Given where GetPlatfromExtensions() is called
from I think it's less practical to pass the EGL native display into it.

https://cs.chromium.org/chromium/src/ui/gl/gl_bindings.cc?q=gl_bindings.cc&sq...

There are a couple of solutions to make it a bit cleaner though:
1. Have DriverEGL::GetPlatfromExtensions() call  GLSurfaceEGL::GetDisplay()
instead and add the caveat that you need to explicitly initialize GLSurfaceEGL
first.
2. Add a boolean guard set true when you call GLSurfaceEGL::SetNativeDisplay()
and checked when you call GLSurfaceEGL::InitializeOneOff() or
GLSurfaceEGL::InitializeDisplay().

[piman, 2016/07/07 20:37:17]

So, unless I'm reading things incorrectly, it looks like with your change,
GLSurfaceEGL::InitializeDisplay is called before SetNativeDisplay.

InitializeGLOneOffImplementation calls InitializeStaticGLBindings first, then
InitializeGLOneOffPlatform.

Taking ozone as an example (it's similar on other platforms):
InitializeStaticGLBindings calls InitializeStaticEGLInternal ->
InitializeStaticGLBindingsEGL -> DriverEGL::InitializeExtensionBindings ->
DriverEGL::GetPlatformExtensions -> GLSurfaceEGL::InitializeDisplay
However, InitializeGLOneOffPlatform is the one that calls SetNativeDisplay

So that means that InitializeDisplay would be called without the native display
being set, causing issues - and this is hard to track down because of the
dependency on the static state.


My suggestion is to move DriverEGL::InitializeExtensionBindings into
GLSurfaceEGL::InitializeOneOff, after the InitializeDisplay, and then do #1 that
you suggest.

[kylechar, 2016/07/08 15:55:05]

Yep, I totally missed that. Good catch. I've implemented your suggestion for
moving DriverEGL::InitializeExtensionBindings().

The CL has grown a bit big so I'll actually split it to make review easier.

+
   static bool InitializeOneOff();
   static EGLDisplay GetHardwareDisplay();
   static EGLDisplay InitializeDisplay();
   static EGLNativeDisplayType GetNativeDisplay();
 
   // These aren't particularly tied to surfaces, but since we already
   // have the static InitializeOneOff here, it's easiest to reuse its
   // initialization guards.
   static const char* GetEGLExtensions();
   static bool HasEGLExtension(const char* name);
@@ skipping 137 matching lines @@
   ~SurfacelessEGL() override;
 
  private:
   gfx::Size size_;
   DISALLOW_COPY_AND_ASSIGN(SurfacelessEGL);
 };
 
 }  // namespace gl
 
 #endif  // UI_GL_GL_SURFACE_EGL_H_