Bug 100639 – While Using WebGL, MiniBrowser segfaults on Refreshing the page.

• Component: WebKit EFL
• Status: RESOLVED FIXED

[Kalyan]

Open any site using WebGL with MiniBrowser. Refresh the Page.This results in a seg fault in MiniBrowser.

[Kalyan]

Created attachment 171173 [details]
fix-proposal

        TextureMapperSurfaceBackingStore can import texture's from a GraphicSurface.
        In such cases GraphicsSurfaceGLX creates an XPixmap to read texture content
        from a given WindowId but doesn't create any new window.However, OffScreenRootWindow
        always tries to unmap window(in it's destruction) resulting in segfault.Now,We try to check for a valid
        Window before trying to unmap the window.

[Kalyan]

Created attachment 171181 [details]
proposed-patch

[Kalyan]

Created attachment 171183 [details]
patch

[Kalyan]

Created attachment 171191 [details]
proposed-patch

rebased

[Kenneth Rohde Christiansen]

Comment on attachment 171191 [details]
proposed-patch

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

> Source/WebCore/ChangeLog:12
> +        TextureMapperSurfaceBackingStore can import texture's from a GraphicSurface.
> +        In such cases GraphicsSurfaceGLX creates an XPixmap to read texture content
> +        from a given WindowId but doesn't create any new window.However, OffScreenRootWindow
> +        always tries to unmap window(in it's destruction) resulting in segfault.Now,We check
> +        if the window is valid before trying to unmap it.

Could you add some spaces after . and start of new sentences

> Source/WebCore/platform/graphics/surfaces/qt/GraphicsSurfaceGLX.cpp:96
>      ~OffScreenRootWindow()
>      {
>          if (!--m_refCount) {
> -            XUnmapWindow(m_display, m_window);
> -            XDestroyWindow(m_display, m_window);
> -            if (m_display)
> +            if (m_display) {
> +                if (m_window) {
> +                    XUnmapWindow(m_display, m_window);
> +                    XDestroyWindow(m_display, m_window);
> +                    m_window = 0;
> +                }
>                  XCloseDisplay(m_display);
> -            m_display = 0;
> +                m_display = 0;
> +            }
>          }
>      }

I would try avoiding indentating tha much

like

if (--m_refCount)
    return;

...

That is like the webkit way

[Kalyan]

Created attachment 171425 [details]
patch

review-changes

[WebKit Review Bot]

Comment on attachment 171425 [details]
patch

Clearing flags on attachment: 171425

Committed r132906: <http://trac.webkit.org/changeset/132906>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.