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

ui/gl/gl_surface_egl.h

Index: ui/gl/gl_surface_egl.h
diff --git a/ui/gl/gl_surface_egl.h b/ui/gl/gl_surface_egl.h
index b95508f590fa9a9117d9e33b79a15b8ca93022c3..4f67c9ece308275d3a606f40aa340f596e54b997 100644
--- a/ui/gl/gl_surface_egl.h
+++ b/ui/gl/gl_surface_egl.h
@@ -26,9 +26,6 @@
 
 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 {
@@ -57,6 +54,10 @@ class GL_EXPORT GLSurfaceEGL : public GLSurface {
   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();