before: - https://build.webkit.org/builders/EFL%20Linux%2064-bit%20Release%20WK2/builds/20777 - 46 fail, only 1 crash from it after: - https://build.webkit.org/builders/EFL%20Linux%2064-bit%20Release%20WK2/builds/20778 - 64 fails, 18 compositing test crashes from it
I got it, fix is coming.
nullptr check of m_private in GraphicsContext3D::makeContextCurrent() (Source/WebCore/platform/graphics/efl/GraphicsContext3DEfl.cpp) is missing. GraphicsContext3DCairo.cpp and GraphicsContext3DWin.cpp implementations already contain this nullptr check, EFL implementation should do the same. I checked it fixes these crashes. So r182101 is innocent, it only reveales this bug, sorry for blaming this change. :)
Created attachment 249823 [details] Patch
(In reply to comment #2) > nullptr check of m_private in GraphicsContext3D::makeContextCurrent() > (Source/WebCore/platform/graphics/efl/GraphicsContext3DEfl.cpp) is missing. > > GraphicsContext3DCairo.cpp and GraphicsContext3DWin.cpp implementations > already contain this nullptr check, EFL implementation should do the same. > > I checked it fixes these crashes. > > So r182101 is innocent, it only reveales this bug, > sorry for blaming this change. :) Thanks, Ossy. This fix looks good for me. For GraphicsContext3DCairo, the nullptr check looks redundant, because GraphicsContext3DCairo creates GraphicsContext3DPrivate unconditionally. Unfortunately, I'm not a reviewer. We need another reviewer for stamp. :)
Comment on attachment 249823 [details] Patch Clearing flags on attachment: 249823 Committed r182187: <http://trac.webkit.org/changeset/182187>
All reviewed patches have been landed. Closing bug.
(In reply to comment #5) > Comment on attachment 249823 [details] > Patch > > Clearing flags on attachment: 249823 > > Committed r182187: <http://trac.webkit.org/changeset/182187> I made something wrong, it seems it didn't fix anything. :( I was sure it fixed the crashes locally, I'll check it soon.
Unfortunately it isn't easy to debug it due to this bug: https://bugs.webkit.org/show_bug.cgi?id=123397#c5
I managed to get a better backtrace for this crash. Unfortunately the original one was a little confusing because of the optimizations. The crash happens because m_context3D is nullptr here: https://trac.webkit.org/browser/trunk/Source/WebCore/platform/graphics/texmap/BitmapTextureGL.cpp?rev=182101#L116 I'm not familiar with this part of the source. Could you give me a hint where should I continue debugging, and how can we fix it properly?
I think the problem is instantiating the BitmapTexturePool with the default constructor here: https://trac.webkit.org/browser/trunk/Source/WebCore/platform/graphics/texmap/TextureMapperImageBuffer.cpp?rev=182101#L32 The default constructor doesn't initialize m_context3D: https://trac.webkit.org/browser/trunk/Source/WebCore/platform/graphics/texmap/BitmapTexturePool.cpp?rev=182101#L42
(In reply to comment #10) > I think the problem is instantiating the BitmapTexturePool with the default > constructor here: > https://trac.webkit.org/browser/trunk/Source/WebCore/platform/graphics/ > texmap/TextureMapperImageBuffer.cpp?rev=182101#L32 > TextureMapperImageBuffer is not used in TEXTURE_MAPPER_GL. How about to check passed GraphicsContext3D to BitmapTextureGL is valid or not? Every GraphicsContext3D should be valid when we are creating BitmapTextureGL. > The default constructor doesn't initialize m_context3D: > https://trac.webkit.org/browser/trunk/Source/WebCore/platform/graphics/ > texmap/BitmapTexturePool.cpp?rev=182101#L42 The default constructor of BitmapTexturePool should not be called in TEXTURE_MAPPER_GL It would be good to disable the default constructor using !USE(TEXTURE_MAPPER_GL)
After more debugging, the problem is that the default constructor of BitmapTexturePool::BitmapTexturePool() is called which doesn't initialize m_context3D.
(In reply to comment #11) > (In reply to comment #10) > > I think the problem is instantiating the BitmapTexturePool with the default > > constructor here: > > https://trac.webkit.org/browser/trunk/Source/WebCore/platform/graphics/ > > texmap/TextureMapperImageBuffer.cpp?rev=182101#L32 > > > > TextureMapperImageBuffer is not used in TEXTURE_MAPPER_GL. How about to check > passed GraphicsContext3D to BitmapTextureGL is valid or not? Every > GraphicsContext3D > should be valid when we are creating BitmapTextureGL. Let's see the crash again. m_context3D is nullptr in BitmapTextureGL::BitmapTextureGL(PassRefPtr<GraphicsContext3D> context3D), because it is called with nullptr from PassRefPtr<BitmapTexture> BitmapTexturePool::createTexture(). And it is nullptr here, because it wasn't initialezed by the default constructor - BitmapTexturePool::BitmapTexturePool() .
(In reply to comment #11) > (In reply to comment #10) > > I think the problem is instantiating the BitmapTexturePool with the default > > constructor here: > > https://trac.webkit.org/browser/trunk/Source/WebCore/platform/graphics/ > > texmap/TextureMapperImageBuffer.cpp?rev=182101#L32 > > > > TextureMapperImageBuffer is not used in TEXTURE_MAPPER_GL. How about to check > passed GraphicsContext3D to BitmapTextureGL is valid or not? Every > GraphicsContext3D > should be valid when we are creating BitmapTextureGL. I don't know anything about this part of the code. I don't understand what you mean "TextureMapperImageBuffer is not used in TEXTURE_MAPPER_GL". ifdefink TextureMapperImageBuffer.cpp and h out caused build failure for me, because it is used in TextureMapper.cpp > > The default constructor doesn't initialize m_context3D: > > https://trac.webkit.org/browser/trunk/Source/WebCore/platform/graphics/ > > texmap/BitmapTexturePool.cpp?rev=182101#L42 > > The default constructor of BitmapTexturePool should not be called in > TEXTURE_MAPPER_GL > It would be good to disable the default constructor using > !USE(TEXTURE_MAPPER_GL) It is called from TextureMapperImageBuffer.cpp .
any additional hint where should I continue debugging?
Just to note, I marked these tests crash in http://trac.webkit.org/changeset/182312 . We should remove these ecpectations once the bug is fixed.
(In reply to comment #16) > Just to note, I marked these tests crash in > http://trac.webkit.org/changeset/182312 . We should remove these > ecpectations once the bug is fixed. I had to comment out the original expectations, because the new ones aren't overriden them - http://trac.webkit.org/changeset/182313.
(In reply to comment #15) > any additional hint where should I continue debugging? ping?
(In reply to comment #18) > (In reply to comment #15) > > any additional hint where should I continue debugging? > ping? ping2?
(In reply to comment #14) > (In reply to comment #11) > > (In reply to comment #10) > > > I think the problem is instantiating the BitmapTexturePool with the default > > > constructor here: > > > https://trac.webkit.org/browser/trunk/Source/WebCore/platform/graphics/ > > > texmap/TextureMapperImageBuffer.cpp?rev=182101#L32 > > > > > > > TextureMapperImageBuffer is not used in TEXTURE_MAPPER_GL. How about to check > > passed GraphicsContext3D to BitmapTextureGL is valid or not? Every > > GraphicsContext3D > > should be valid when we are creating BitmapTextureGL. > > I don't know anything about this part of the code. I don't understand what > you mean "TextureMapperImageBuffer is not used in TEXTURE_MAPPER_GL". > ifdefink TextureMapperImageBuffer.cpp and h out caused build failure > for me, because it is used in TextureMapper.cpp > After taking a look at EFL ports, it uses TextureMapperImageBuffer only if there is no OpenGL surface available. Basically, if we use TEXTURE_MAPPER_GL, it tries to use TextureMapperGL first, and then if it was not successful (this is what I'm facing, efl refuses llvmpipe as a GPU backend), it uses TextureMapper + TextureMapperImageBuffer. But the TextureMapperImageBuffer is not maintained for a while, and I'm planning to remove this. If EFL ports uses OpenGL backend, it uses TextureMapperGL instead of TextureMapperImageBuffer. Because it is done at runtime, you faced build failure when you remove TextureMapperImageBuffer. If you to remove TextureMapperImageBuffer and some related codes, (it is enough to remove line 43,44 in TextureMapper.cpp) and it should be run properly. > > > The default constructor doesn't initialize m_context3D: > > > https://trac.webkit.org/browser/trunk/Source/WebCore/platform/graphics/ > > > texmap/BitmapTexturePool.cpp?rev=182101#L42 > > > > The default constructor of BitmapTexturePool should not be called in > > TEXTURE_MAPPER_GL > > It would be good to disable the default constructor using > > !USE(TEXTURE_MAPPER_GL) > > It is called from TextureMapperImageBuffer.cpp .
Thanks for the info, I filed a new bug report to remove TextureMapperImageBuffer - bug143561. It fixes the crashes and many other compositing failures on EFL.
(In reply to comment #21) > Thanks for the info, I filed a new bug report to remove > TextureMapperImageBuffer > - bug143561. It fixes the crashes and many other compositing failures on EFL. It's quite interesting result. It means WebKitEFL uses TextureMapperImageBuffer and TextureMapperGL at same time, which should not be happened.
(In reply to comment #22) > (In reply to comment #21) > > Thanks for the info, I filed a new bug report to remove > > TextureMapperImageBuffer > > - bug143561. It fixes the crashes and many other compositing failures on EFL. > > It's quite interesting result. It means WebKitEFL uses > TextureMapperImageBuffer and TextureMapperGL at same time, which should not > be happened. After digging the bug a little bit deeper. The problem is that EFL still refuses using GL backend. It seemed that removing the software based TextureMapperImageBuffer solved the compositing tests crashes ... but we got 72 another crashes instead. :-/
Closing this bug because the EFL port has been removed from trunk. If you feel this bug applies to a different upstream WebKit port and was closed in error, please either update the title and reopen the bug, or leave a comment to request this.