143214 2015-03-30 03:00:58 -0700 [EFL] Fix 18 crashing compositing tests after r182101 2017-03-11 10:48:51 -0800 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED WONTFIX https://bugs.webkit.org/show_bug.cgi?id=143561 P2 Normal --- 1 ossy webkit-unassigned clopez commit-queue dino gyuyoung.kim kondapallykalyan mcatanzaro ossy roger_fong yoon zan oldest_to_newest 1081127 0 ossy 2015-03-30 03:00:58 -0700 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 1081629 1 ossy 2015-03-31 07:28:40 -0700 I got it, fix is coming. 1081631 2 ossy 2015-03-31 07:33:39 -0700 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. :) 1081632 3 249823 ossy 2015-03-31 07:34:59 -0700 Created attachment 249823 Patch 1081635 4 yoon 2015-03-31 07:52:27 -0700 (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. :) 1081674 5 249823 commit-queue 2015-03-31 10:33:34 -0700 Comment on attachment 249823 Patch Clearing flags on attachment: 249823 Committed r182187: <http://trac.webkit.org/changeset/182187> 1081675 6 commit-queue 2015-03-31 10:33:38 -0700 All reviewed patches have been landed. Closing bug. 1081910 7 ossy 2015-03-31 23:44:25 -0700 (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. 1081946 8 ossy 2015-04-01 02:25:16 -0700 Unfortunately it isn't easy to debug it due to this bug: https://bugs.webkit.org/show_bug.cgi?id=123397#c5 1081975 9 ossy 2015-04-01 05:38:36 -0700 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? 1081976 10 ossy 2015-04-01 05:47:29 -0700 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 1081984 11 yoon 2015-04-01 06:29:29 -0700 (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) 1081995 12 ossy 2015-04-01 07:24:02 -0700 After more debugging, the problem is that the default constructor of BitmapTexturePool::BitmapTexturePool() is called which doesn't initialize m_context3D. 1081999 13 ossy 2015-04-01 07:38:18 -0700 (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() . 1082000 14 ossy 2015-04-01 07:44:50 -0700 (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 . 1082501 15 ossy 2015-04-03 03:32:08 -0700 any additional hint where should I continue debugging? 1082506 16 ossy 2015-04-03 03:56:59 -0700 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. 1082510 17 ossy 2015-04-03 04:25:41 -0700 (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. 1083283 18 ossy 2015-04-07 04:16:21 -0700 (In reply to comment #15) > any additional hint where should I continue debugging? ping? 1083915 19 ossy 2015-04-09 01:20:27 -0700 (In reply to comment #18) > (In reply to comment #15) > > any additional hint where should I continue debugging? > ping? ping2? 1083916 20 yoon 2015-04-09 01:34:48 -0700 (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 . 1083933 21 ossy 2015-04-09 02:59:31 -0700 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. 1084202 22 yoon 2015-04-09 22:28:00 -0700 (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. 1084277 23 ossy 2015-04-10 05:59:17 -0700 (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. :-/ 1286702 24 mcatanzaro 2017-03-11 10:48:51 -0800 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. 249823 2015-03-31 07:34:59 -0700 2015-03-31 10:33:34 -0700 Patch bug-143214-20150331163415.patch text/plain 1390 ossy U3VidmVyc2lvbiBSZXZpc2lvbjogMTgyMTgxCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggZjAxYmMwMjM5ZGZhZjUx ODRkMTBiNzljMWQzZDc2ZGJjNTZkOTZkMy4uY2MxNmFhM2M5Y2Q3ODcyN2ZmMTUxNzE4NDE1ZTI1 MjY2OWYxZTIwNSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDEzIEBACisyMDE1LTAzLTMxICBDc2Fi YSBPc3p0cm9nb27DoWMgIDxvc3N5QHdlYmtpdC5vcmc+CisKKyAgICAgICAgW0VGTF0gQWRkIG51 bGxwdHIgY2hlY2sgdG8gR3JhcGhpY3NDb250ZXh0M0Q6Om1ha2VDb250ZXh0Q3VycmVudCgpCisg ICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xNDMyMTQKKwor ICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIHBsYXRmb3Jt L2dyYXBoaWNzL2VmbC9HcmFwaGljc0NvbnRleHQzREVmbC5jcHA6CisgICAgICAgIChXZWJDb3Jl OjpHcmFwaGljc0NvbnRleHQzRDo6bWFrZUNvbnRleHRDdXJyZW50KToKKwogMjAxNS0wMy0zMSAg WGFiaWVyIFJvZHJpZ3VleiBDYWx2YXIgPGNhbHZhcmlzQGlnYWxpYS5jb20+IGFuZCBZb3Vlbm4g RmFibGV0ICA8eW91ZW5uLmZhYmxldEBjcmYuY2Fub24uZnI+CiAKICAgICAgICAgW1N0cmVhbXMg QVBJXSBJbXBsZW1lbnQgYSBiYXJlYm9uZSBSZWFkYWJsZVN0cmVhbVJlYWRlciBpbnRlcmZhY2UK ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL2VmbC9HcmFwaGlj c0NvbnRleHQzREVmbC5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9lZmwv R3JhcGhpY3NDb250ZXh0M0RFZmwuY3BwCmluZGV4IDkzNTBjZTIwOTMyNTNkZTg4ZDdjYjAyYWFl YmI2MjBhYTI4NzllYTUuLjI3MDU2MGIzOTRlM2EwNzQzY2RlOTA3ZGEyNDM0YjhkYzIwNzliYjYg MTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL2VmbC9HcmFwaGlj c0NvbnRleHQzREVmbC5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3Mv ZWZsL0dyYXBoaWNzQ29udGV4dDNERWZsLmNwcApAQCAtMTgwLDYgKzE4MCw4IEBAIFBsYXRmb3Jt TGF5ZXIqIEdyYXBoaWNzQ29udGV4dDNEOjpwbGF0Zm9ybUxheWVyKCkgY29uc3QKIAogYm9vbCBH cmFwaGljc0NvbnRleHQzRDo6bWFrZUNvbnRleHRDdXJyZW50KCkKIHsKKyAgICBpZiAoIW1fcHJp dmF0ZSkKKyAgICAgICAgcmV0dXJuIGZhbHNlOwogICAgIHJldHVybiBtX3ByaXZhdGUtPm1ha2VD b250ZXh0Q3VycmVudCgpOwogfQogCg==