91525 2012-07-17 11:17:31 -0700 REGRESSION(r122215) - RenderObject::willRenderImage crashes on null view() 2012-07-17 16:07:50 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 enne enne enne eric jchaffraix koivisto skyul webkit.review.bot oldest_to_newest 671424 0 enne 2012-07-17 11:17:31 -0700 REGRESSION(r122215) - RenderObject::willRenderImage crashes on null view() 671524 1 152812 enne 2012-07-17 13:13:08 -0700 Created attachment 152812 Patch 671545 2 152812 eric 2012-07-17 13:41:55 -0700 Comment on attachment 152812 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=152812&action=review > Source/WebCore/rendering/RenderObject.cpp:2697 > + if (!view()) So this is RenderView... And document()->view() is FrameView? 671559 3 152812 eric 2012-07-17 13:48:10 -0700 Comment on attachment 152812 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=152812&action=review >> Source/WebCore/rendering/RenderObject.cpp:2697 >> + if (!view()) > > So this is RenderView... And document()->view() is FrameView? Yes, it is. And we should really make that less confusing some day. I'm confused what callstack would hit this? I guess document destruction? IT's not clear what you mean by "not been inserted into the tree", do you mean the document or the RenderImage? 671590 4 enne 2012-07-17 14:17:58 -0700 (In reply to comment #3) > (From update of attachment 152812 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=152812&action=review > > >> Source/WebCore/rendering/RenderObject.cpp:2697 > >> + if (!view()) > > > > So this is RenderView... And document()->view() is FrameView? > > Yes, it is. And we should really make that less confusing some day. > > I'm confused what callstack would hit this? I guess document destruction? IT's not clear what you mean by "not been inserted into the tree", do you mean the document or the RenderImage? Maybe I shouldn't have included any comment at all. ;) Document destruction is one case, since the Document's renderer is cleared during that step. Document attachment is the other, since that's when the renderer gets initially set. I'm guessing at this being the issue given the callstack in the crashreport in the linked Chromium bug. I wasn't able to repro this locally. 671635 5 152841 enne 2012-07-17 15:11:48 -0700 Created attachment 152841 Reword comment 671645 6 152841 skyul 2012-07-17 15:18:06 -0700 Comment on attachment 152841 Reword comment View in context: https://bugs.webkit.org/attachment.cgi?id=152841&action=review > Source/WebCore/rendering/RenderObject.cpp:2699 > + The patch looks good to me. I'm sorry to miss this in r122215. 671661 7 152841 jchaffraix 2012-07-17 15:31:42 -0700 Comment on attachment 152841 Reword comment View in context: https://bugs.webkit.org/attachment.cgi?id=152841&action=review > Source/WebCore/ChangeLog:10 > + It would be nice to mention why you couldn't produce a test case in your ChangeLog. > Source/WebCore/rendering/RenderObject.cpp:2696 > + // If the document is being destroyed or has not been attached, then this > + // RenderObject will not be rendered. I am fine with the comment as-is though Eric may want to comment further. 671699 8 enne 2012-07-17 16:06:55 -0700 Committed r122886: <http://trac.webkit.org/changeset/122886> 671702 9 enne 2012-07-17 16:07:50 -0700 (In reply to comment #7) > (From update of attachment 152841 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=152841&action=review > > > Source/WebCore/ChangeLog:10 > > + > > It would be nice to mention why you couldn't produce a test case in your ChangeLog. Done. > > Source/WebCore/rendering/RenderObject.cpp:2696 > > + // If the document is being destroyed or has not been attached, then this > > + // RenderObject will not be rendered. > > I am fine with the comment as-is though Eric may want to comment further. Sorry for going ahead and landing this, but I wanted to have a chance to get this in tomorrow's canary and see if it reduced the crash rate. I'm happy to change the comment in a follow-up patch if you think that can be reworded better. :) 152812 2012-07-17 13:13:08 -0700 2012-07-17 15:11:45 -0700 Patch bug-91525-20120717131308.patch text/plain 1620 enne U3VidmVyc2lvbiBSZXZpc2lvbjogMTIyODQ3CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggYjg0ODBiZWM5NTBiYzQy ZmQ5ZjlmYTJmZWMzNDI3OGE2ZDEzZjgxYy4uMjI3YWNiMzNmMTE3ZTZlYWY1MDU4ZmI4YTFmOTM2 YWVhMjRkZDI3YiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE2IEBACisyMDEyLTA3LTE3ICBBZHJp ZW5uZSBXYWxrZXIgIDxlbm5lQGdvb2dsZS5jb20+CisKKyAgICAgICAgUkVHUkVTU0lPTihyMTIy MjE1KSAtIFJlbmRlck9iamVjdDo6d2lsbFJlbmRlckltYWdlIGNyYXNoZXMgb24gbnVsbCB2aWV3 KCkKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTkxNTI1 CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgRml4IGJ5 IGRvaW5nIGFuIGVhcmx5IG91dCBjaGVjay4gIFRoaXMgaXMgaW50ZW5kZWQgdG8gZml4IHRoZSBj cmFzaCBpbgorICAgICAgICBodHRwOi8vY3JidWcuY29tLzEzNzE2MS4KKworICAgICAgICAqIHJl bmRlcmluZy9SZW5kZXJPYmplY3QuY3BwOgorICAgICAgICAoV2ViQ29yZTo6UmVuZGVyT2JqZWN0 Ojp3aWxsUmVuZGVySW1hZ2UpOgorCiAyMDEyLTA3LTE2ICBQYXZlbCBGZWxkbWFuICA8cGZlbGRt YW5AY2hyb21pdW0ub3JnPgogCiAgICAgICAgIFdlYiBJbnNwZWN0b3I6IGltcGxlbWVudCBzZWFy Y2ggLyByZXBsYWNlIGluIHNvdXJjZSBmaWxlcyAoYmVoaW5kIGV4cGVyaW1lbnQgZmxhZykKZGlm ZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL3JlbmRlcmluZy9SZW5kZXJPYmplY3QuY3BwIGIvU291 cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRlck9iamVjdC5jcHAKaW5kZXggN2FkZWRhOTkyYmNk NzZiYjlkMzkyMjcwOGIyZDNiZDFmMGQ2OWU4Yi4uNTU1MGZiMWU2NGNlNmFjYTg3MWM1OTBlNWM5 NmE0Y2Y0ZjQ0YTdmMCAxMDA3NTUKLS0tIGEvU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRl ck9iamVjdC5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRlck9iamVjdC5j cHAKQEAgLTI2OTIsNiArMjY5MiwxMSBAQCBib29sIFJlbmRlck9iamVjdDo6d2lsbFJlbmRlcklt YWdlKENhY2hlZEltYWdlKikKICAgICBpZiAoZG9jdW1lbnQoKS0+aW5QYWdlQ2FjaGUoKSB8fCBk b2N1bWVudCgpLT52aWV3KCktPmlzT2Zmc2NyZWVuKCkpCiAgICAgICAgIHJldHVybiBmYWxzZTsK IAorICAgIC8vIElmIHRoZSByZW5kZXJlcidzIGRvY3VtZW50IGlzIGJlaW5nIGRlc3Ryb3llZCBv ciBoYXMgbm90IGJlZW4gaW5zZXJ0ZWQKKyAgICAvLyBpbnRvIHRoZSB0cmVlLCB0aGVuIHRoaXMg d29uJ3QgYmUgcmVuZGVyZWQuCisgICAgaWYgKCF2aWV3KCkpCisgICAgICAgIHJldHVybiBmYWxz ZTsKKwogICAgIC8vIElmIGEgcmVuZGVyZXIgaXMgb3V0c2lkZSB0aGUgdmlld3BvcnQsIHdlIHdv bid0IHJlbmRlci4KICAgICByZXR1cm4gdmlld1JlY3QoKS5pbnRlcnNlY3RzKGFic29sdXRlQ2xp cHBlZE92ZXJmbG93UmVjdCgpKTsKIH0K 152841 2012-07-17 15:11:48 -0700 2012-07-17 15:31:42 -0700 Reword comment bug-91525-20120717151148.patch text/plain 1611 enne U3VidmVyc2lvbiBSZXZpc2lvbjogMTIyODQ3CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggYjg0ODBiZWM5NTBiYzQy ZmQ5ZjlmYTJmZWMzNDI3OGE2ZDEzZjgxYy4uMjI3YWNiMzNmMTE3ZTZlYWY1MDU4ZmI4YTFmOTM2 YWVhMjRkZDI3YiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE2IEBACisyMDEyLTA3LTE3ICBBZHJp ZW5uZSBXYWxrZXIgIDxlbm5lQGdvb2dsZS5jb20+CisKKyAgICAgICAgUkVHUkVTU0lPTihyMTIy MjE1KSAtIFJlbmRlck9iamVjdDo6d2lsbFJlbmRlckltYWdlIGNyYXNoZXMgb24gbnVsbCB2aWV3 KCkKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTkxNTI1 CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgRml4IGJ5 IGRvaW5nIGFuIGVhcmx5IG91dCBjaGVjay4gIFRoaXMgaXMgaW50ZW5kZWQgdG8gZml4IHRoZSBj cmFzaCBpbgorICAgICAgICBodHRwOi8vY3JidWcuY29tLzEzNzE2MS4KKworICAgICAgICAqIHJl bmRlcmluZy9SZW5kZXJPYmplY3QuY3BwOgorICAgICAgICAoV2ViQ29yZTo6UmVuZGVyT2JqZWN0 Ojp3aWxsUmVuZGVySW1hZ2UpOgorCiAyMDEyLTA3LTE2ICBQYXZlbCBGZWxkbWFuICA8cGZlbGRt YW5AY2hyb21pdW0ub3JnPgogCiAgICAgICAgIFdlYiBJbnNwZWN0b3I6IGltcGxlbWVudCBzZWFy Y2ggLyByZXBsYWNlIGluIHNvdXJjZSBmaWxlcyAoYmVoaW5kIGV4cGVyaW1lbnQgZmxhZykKZGlm ZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL3JlbmRlcmluZy9SZW5kZXJPYmplY3QuY3BwIGIvU291 cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRlck9iamVjdC5jcHAKaW5kZXggN2FkZWRhOTkyYmNk NzZiYjlkMzkyMjcwOGIyZDNiZDFmMGQ2OWU4Yi4uNmNkYjRiMmY5NDBmOTRkYjE2NzlkMzE5MTM2 MTFhYmY2M2NmYzRiNyAxMDA3NTUKLS0tIGEvU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRl ck9iamVjdC5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRlck9iamVjdC5j cHAKQEAgLTI2OTIsNiArMjY5MiwxMSBAQCBib29sIFJlbmRlck9iamVjdDo6d2lsbFJlbmRlcklt YWdlKENhY2hlZEltYWdlKikKICAgICBpZiAoZG9jdW1lbnQoKS0+aW5QYWdlQ2FjaGUoKSB8fCBk b2N1bWVudCgpLT52aWV3KCktPmlzT2Zmc2NyZWVuKCkpCiAgICAgICAgIHJldHVybiBmYWxzZTsK IAorICAgIC8vIElmIHRoZSBkb2N1bWVudCBpcyBiZWluZyBkZXN0cm95ZWQgb3IgaGFzIG5vdCBi ZWVuIGF0dGFjaGVkLCB0aGVuIHRoaXMKKyAgICAvLyBSZW5kZXJPYmplY3Qgd2lsbCBub3QgYmUg cmVuZGVyZWQuCisgICAgaWYgKCF2aWV3KCkpCisgICAgICAgIHJldHVybiBmYWxzZTsKKwogICAg IC8vIElmIGEgcmVuZGVyZXIgaXMgb3V0c2lkZSB0aGUgdmlld3BvcnQsIHdlIHdvbid0IHJlbmRl ci4KICAgICByZXR1cm4gdmlld1JlY3QoKS5pbnRlcnNlY3RzKGFic29sdXRlQ2xpcHBlZE92ZXJm bG93UmVjdCgpKTsKIH0K