[webkit-reviews] review granted: [Bug 100639] While Using WebGL, MiniBrowser segfaults on Refreshing the page. : [Attachment 171191] proposed-patch

[bugzilla-daemon at webkit.org]

Kenneth Rohde Christiansen <kenneth at webkit.org> has granted Kalyan
<kalyan.kondapally at intel.com>'s request for review:
Bug 100639: While Using WebGL, MiniBrowser segfaults on Refreshing the page.
https://bugs.webkit.org/show_bug.cgi?id=100639

Attachment 171191: proposed-patch
https://bugs.webkit.org/attachment.cgi?id=171191&action=review

------- Additional Comments from Kenneth Rohde Christiansen
<kenneth at webkit.org>
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