Bug 136216

Created attachment 237074 [details] Patch

Attachment 237074 [details] did not pass style-queue: ERROR: Source/WebCore/platform/graphics/wayland/WaylandDisplay.h:35: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/platform/graphics/wayland/WaylandDisplay.h:36: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 2 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style.

Comment on attachment 237074 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=237074&action=review > Source/WebCore/platform/graphics/wayland/WaylandDisplay.cpp:131 > +void WaylandDisplay::destroySurface(WaylandSurface* surface) > +{ > + if (surface) { > + eglMakeCurrent(m_eglDisplay, EGL_NO_SURFACE, EGL_NO_SURFACE, EGL_NO_CONTEXT); > + > + wl_egl_window_destroy(surface->nativeWindowHandle()); > + wl_surface_destroy(surface->surface()); > + } > +} Looking at this as I was writing the changelog, I think all this could be just executed in the WaylandSurface destructor. I'll try it out and report back, either with a new patch or an explanation on why it can't be done.

Created attachment 237101 [details] Patch

Attachment 237101 [details] did not pass style-queue: ERROR: Source/WebCore/platform/graphics/wayland/WaylandDisplay.h:35: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/platform/graphics/wayland/WaylandDisplay.h:36: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 2 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.

Created attachment 237141 [details] Patch

Attachment 237141 [details] did not pass style-queue: ERROR: Source/WebCore/platform/graphics/wayland/WaylandDisplay.h:35: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/platform/graphics/wayland/WaylandDisplay.h:36: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 2 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.

Created attachment 237299 [details] Patch

Attachment 237299 [details] did not pass style-queue: ERROR: Source/WebCore/platform/graphics/wayland/WaylandDisplay.h:35: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/platform/graphics/wayland/WaylandDisplay.h:36: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 2 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.

Comment on attachment 237299 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=237299&action=review > Source/WebCore/platform/graphics/wayland/WaylandDisplay.cpp:78 > + m_registry = wl_display_get_registry(m_display); This can be moved the initializer list. > Source/WebCore/platform/graphics/wayland/WaylandDisplay.cpp:94 > + g_warning("eglGetDisplay EGL_NO_DISPLAY"); It's probably better to issue a more descriptive warning like: "WaylandDisplay initialization failed to get EGL display." > Source/WebCore/platform/graphics/wayland/WaylandDisplay.cpp:99 > + g_warning("eglInitialize EGL_FALSE"); Ditto. > Source/WebCore/platform/graphics/wayland/WaylandDisplay.cpp:104 > + g_warning("eglBindAPI EGL_FALSE"); Ditto. When these fail how does the owner of this instance detect the failure? > Source/WebCore/platform/graphics/wayland/WaylandDisplay.cpp:110 > + EGLint n; > + if (!eglChooseConfig(m_eglDisplay, configAttributes, &m_eglConfig, 1, &n) || n != 1) > + g_warning("eglChooseConfig failed"); n -> numberOfConfigs > Source/WebCore/platform/graphics/wayland/WaylandDisplay.cpp:116 > + EGLNativeWindowType nativeWindow = wl_egl_window_create(wlSurface, std::max(1, width), std::max(1, height)); Perhaps better to assert(width > 0 && height > 0)? > Source/WebCore/platform/graphics/wayland/WaylandDisplay.h:58 > + std::unique_ptr<WaylandSurface> createSurface(int, int, int); > + > + PassOwnPtr<GLContextEGL> createSharingGLContext(); > + > +private: > + static const struct wl_registry_listener m_registryListener; > + static void globalCallback(void*, struct wl_registry*, uint32_t, const char*, uint32_t); > + static void globalRemoveCallback(void*, struct wl_registry*, uint32_t); Arguments should have a name here unless the name is simply a repetition of the type. That way one can read the header and know how to call the method. For instance, "struct wl_registry* registry" doesn't need a name, but "int height" does.

Created attachment 237417 [details] Patch Still have to check the possibility of asserting the non-zero size parameters in WaylandDisplay::createSurface() (and if IntSize could be used) and where in LayerTreeHostGtk do 0x0 surfaces come from in the first place.

Attachment 237417 [details] did not pass style-queue: ERROR: Source/WebCore/platform/graphics/wayland/WaylandDisplay.h:35: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/platform/graphics/wayland/WaylandDisplay.h:36: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 2 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.

Created attachment 237441 [details] Patch

Attachment 237441 [details] did not pass style-queue: ERROR: Source/WebCore/platform/graphics/wayland/WaylandDisplay.h:35: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/platform/graphics/wayland/WaylandDisplay.h:36: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 2 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.

Comment on attachment 237441 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=237441&action=review > Source/WebCore/platform/graphics/wayland/WaylandDisplay.cpp:126 > + // We keep the minimum size at 1x1px since Mesa returns null values in wl_egl_window_create() for zero width or height. > + EGLNativeWindowType nativeWindow = wl_egl_window_create(wlSurface, std::max(1, size.width()), std::max(1, size.height())); The 0x0px surfaces are created in LayerTreeHostGtk::initialize() using the size of the WebPage object which at that point still doesn't have the proper size. On the other hand Mesa returns a null pointer from wl_egl_window_create() if either width or height is zero. That's what's being avoided here by falling back to a 1x1px surface that is eventually resized through WaylandSurface::resize(), from LayerTreeHostGtk::sizeDidChange().

Committed r173651: <http://trac.webkit.org/changeset/173651>

It's very confusing to have an unused Wayland protocol in the source tree. I think we should get rid of this code until such time as we are ready to land patches that use it.