Bug 163722 – [GTK] Avoid including egl.h headers in internal headers

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Carlos Garcia Campos]

egl.h includes eglplatform.h that decides the native types for the platform at compile time. However, we support to build with X11 and Wayland at the same time and decide what to use at runtime. Currently GLContext.h includes eglplatform.h after wayland-egl.h if Wayland is enabled. That means that the wayland native types are used by default from all cpp files including GLContext.h. It currently works on X11 because we cast the value anyway and for example EGLNativeWindowType is a pointer in Wayland that can be casted to unsigned long in X11 to represent the X Window. This is very fragile in any case, we should avoid adding egl headers in our headers and only include it in cpp files. But we also need to ensure we don't use X11 and Wayland in the same cpp file.

[Carlos Garcia Campos]

Created attachment 292166 [details]
Patch

[WebKit Commit Bot]

Attachment 292166 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/egl/GLContextEGLWayland.cpp:27:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/wayland/PlatformDisplayWayland.cpp:36:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 2 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Zan Dobersek]

Comment on attachment 292166 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=292166&action=review

> Source/WebCore/platform/graphics/egl/GLContextEGLWayland.cpp:79
> +    return std::unique_ptr<GLContextEGL>(new GLContextEGL(platformDisplay, context, surface, WTFMove(wlSurface), window));

Is std::make_unique<>() usable here?

> Source/WebCore/platform/graphics/egl/GLContextEGLX11.cpp:99
> +    return std::unique_ptr<GLContextEGL>(new GLContextEGL(platformDisplay, context, surface, WTFMove(pixmap)));

Is std::make_unique<>() usable here?

[Carlos Garcia Campos]

(In reply to comment #3)
> Comment on attachment 292166 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=292166&action=review
> 
> > Source/WebCore/platform/graphics/egl/GLContextEGLWayland.cpp:79
> > +    return std::unique_ptr<GLContextEGL>(new GLContextEGL(platformDisplay, context, surface, WTFMove(wlSurface), window));
> 
> Is std::make_unique<>() usable here?
> 
> > Source/WebCore/platform/graphics/egl/GLContextEGLX11.cpp:99
> > +    return std::unique_ptr<GLContextEGL>(new GLContextEGL(platformDisplay, context, surface, WTFMove(pixmap)));
> 
> Is std::make_unique<>() usable here?

Nope, constructors are private to ensure only the create methods can create instances.

[Carlos Garcia Campos]

Committed r207615: <http://trac.webkit.org/changeset/207615>