73800 2011-12-04 22:54:33 -0800 [Refactoring] Accessing Node::m_document should be minimized. 2019-02-06 09:02:59 -0800 1 1 1 Unclassified WebKit DOM 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 73827 59816 1 morrita morrita ap cdumez dglazkov dominicc husky ossy rolandsteiner webkit.review.bot oldest_to_newest 514924 0 morrita 2011-12-04 22:54:33 -0800 It should be indirected by Node::document() because Node::document() is going to be a bit nontrivial. 514954 1 117851 morrita 2011-12-05 00:27:38 -0800 Created attachment 117851 Patch 514970 2 117851 tkent 2011-12-05 01:20:42 -0800 Comment on attachment 117851 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=117851&action=review > Source/WebCore/ChangeLog:12 > + [Refactoring] Accessing Node::m_document should be minimized. > + https://bugs.webkit.org/show_bug.cgi?id=73800 > + > + Reviewed by NOBODY (OOPS!). > + > + No new tests. No behavioral change. > + > + Replaced m_document reference with the document() accessor > + or temporaril variables. > + Please write a reason of the change. 514971 3 117851 webkit.review.bot 2011-12-05 01:27:10 -0800 Comment on attachment 117851 Patch Attachment 117851 did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/10688615 New failing tests: svg/custom/linking-uri-01-b.svg 514995 4 morrita 2011-12-05 03:00:36 -0800 (In reply to comment #2) > > + > > Please write a reason of the change. Hi Kent-san, thanks for quick review! I'll add some background explanation before landing. 515003 5 morrita 2011-12-05 03:15:59 -0800 Committed r101983: <http://trac.webkit.org/changeset/101983> 515056 6 husky 2011-12-05 07:02:18 -0800 I think this is causing a crash in debug builds. ~Node now calls Node::document(), which includes the following: ASSERT(m_document || (nodeType() == DOCUMENT_TYPE_NODE && !inDocument())); Node::nodeType() is pure virtual and there cannot be called in a destructor (the subclass has already been destroyed). Chromium crashes on startup with a "pure virtual method called" error. I assume this affects all WebKit ports. I'll verify with a debug build of DumpRenderTree. 515059 7 husky 2011-12-05 07:09:55 -0800 Aha, already rolled out in http://trac.webkit.org/changeset/101995 515066 8 ossy 2011-12-05 07:28:11 -0800 (In reply to comment #7) > Aha, already rolled out in http://trac.webkit.org/changeset/101995 It was rolled out because it broke all tests on GTK and on Qt in debug mode. The exact reason can be found in comment #6. 515068 9 117851 ossy 2011-12-05 07:28:29 -0800 Comment on attachment 117851 Patch removing flags from rolled out patch 515166 10 ap 2011-12-05 10:26:20 -0800 > Node::document() is going to be a bit nontrivial What is the plan for Node::document()? It directly maps to DOM concept, so changing it could introduce a lot of complexity. 515647 11 morrita 2011-12-05 22:10:41 -0800 I'm totally sorry about the last change. It was a disaster... By the way, > What is the plan for Node::document()? It directly maps to DOM concept, so changing it could introduce a lot of complexity. Basically, this series of change is adding an indirection for Node::document() call. This is preparation for Bug 59816, where I'm going to turn m_document into m_treeScope pointing TreeScope* instance, which is a superclass of Document and ShadowRoot. After the change, Node::document() will be something like: return m_treeScope->enclosingDocument(); TreeScope::m_enclosingDocument will point itself when the real class of the instance is Document. return its containing Document. Currently all elements in shadow have ElementRareData because they need ElementRareData::m_treeScope. This change eliminates ElementRareData::m_treeScope, so these nodes no longer need to have its rare data and can save memory allocation for that. 515669 12 117993 morrita 2011-12-05 23:02:29 -0800 Created attachment 117993 A fixed version 515676 13 117993 tkent 2011-12-05 23:25:33 -0800 Comment on attachment 117993 A fixed version ok. Please watch the buildbots after landing the patch. 515781 14 117993 webkit.review.bot 2011-12-06 03:31:01 -0800 Comment on attachment 117993 A fixed version Clearing flags on attachment: 117993 Committed r102119: <http://trac.webkit.org/changeset/102119> 515782 15 webkit.review.bot 2011-12-06 03:31:06 -0800 All reviewed patches have been landed. Closing bug. 1502880 16 lforschler 2019-02-06 09:02:59 -0800 Mass moving XML DOM bugs to the "DOM" Component. 117851 2011-12-05 00:27:38 -0800 2011-12-05 23:02:22 -0800 Patch bug-73800-20111205172736.patch text/plain 2925 morrita U3VidmVyc2lvbiBSZXZpc2lvbjogMTAxOTcwCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggYzkyZTkxYmRhN2IzMzQ1 YTc2ZWYzMWM3NjA3ZjdjZmExNThjMWU4Ny4uMGViNDI2OWVhZjRhNzlkMThjY2M0MWQ2MjFlYTk1 OWEyMDFiYmU0ZSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIyIEBACisyMDExLTEyLTA1ICBIYWpp bWUgTW9ycml0YSAgPG1vcnJpdGFAY2hyb21pdW0ub3JnPgorCisgICAgICAgIFtSZWZhY3Rvcmlu Z10gQWNjZXNzaW5nIE5vZGU6Om1fZG9jdW1lbnQgc2hvdWxkIGJlIG1pbmltaXplZC4KKyAgICAg ICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTczODAwCisKKyAgICAg ICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgTm8gbmV3IHRlc3RzLiBO byBiZWhhdmlvcmFsIGNoYW5nZS4KKworICAgICAgICBSZXBsYWNlZCBtX2RvY3VtZW50IHJlZmVy ZW5jZSB3aXRoIHRoZSBkb2N1bWVudCgpIGFjY2Vzc29yCisgICAgICAgIG9yIHRlbXBvcmFyaWwg dmFyaWFibGVzLgorCisgICAgICAgICogZG9tL0RvY3VtZW50Lmg6CisgICAgICAgIChXZWJDb3Jl OjpOb2RlOjpOb2RlKToKKyAgICAgICAgKiBkb20vTm9kZS5jcHA6CisgICAgICAgIChXZWJDb3Jl OjpOb2RlOjp+Tm9kZSk6CisgICAgICAgICogZG9tL05vZGUuaDoKKyAgICAgICAgKFdlYkNvcmU6 Ok5vZGU6OmluRG9jdW1lbnQpOgorCiAyMDExLTEyLTA0ICBBbmRyZWFzIEtsaW5nICA8a2xpbmdA d2Via2l0Lm9yZz4KIAogICAgICAgICBib3JkZXItd2lkdGg6IGluaXRpYWwgYW5kIGJvcmRlci1j b2xvcjogaW5pdGlhbCBjYW5ub3QgYmUgcmVtb3ZlZCB2aWEgQ1NTT00uCmRpZmYgLS1naXQgYS9T b3VyY2UvV2ViQ29yZS9kb20vRG9jdW1lbnQuaCBiL1NvdXJjZS9XZWJDb3JlL2RvbS9Eb2N1bWVu dC5oCmluZGV4IDY2N2VhZGU1ZmE2MzA0YWEyYTQ0ZTY3YTg5YjE3YmM4ZWE2YjBhY2MuLmM5YTdm NDA2Y2I0NGU2YmVkNGRiOWYzMGEzMzAxODdiZGE3YjM2ZmQgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9X ZWJDb3JlL2RvbS9Eb2N1bWVudC5oCisrKyBiL1NvdXJjZS9XZWJDb3JlL2RvbS9Eb2N1bWVudC5o CkBAIC0xNDQ5LDggKzE0NDksOCBAQCBpbmxpbmUgTm9kZTo6Tm9kZShEb2N1bWVudCogZG9jdW1l bnQsIENvbnN0cnVjdGlvblR5cGUgdHlwZSkKICAgICAsIG1fbmV4dCgwKQogICAgICwgbV9yZW5k ZXJlcigwKQogewotICAgIGlmIChtX2RvY3VtZW50KQotICAgICAgICBtX2RvY3VtZW50LT5ndWFy ZFJlZigpOworICAgIGlmIChkb2N1bWVudCkKKyAgICAgICAgZG9jdW1lbnQtPmd1YXJkUmVmKCk7 CiAjaWYgIWRlZmluZWQoTkRFQlVHKSB8fCAoZGVmaW5lZChEVU1QX05PREVfU1RBVElTVElDUykg JiYgRFVNUF9OT0RFX1NUQVRJU1RJQ1MpCiAgICAgdHJhY2tGb3JEZWJ1Z2dpbmcoKTsKICNlbmRp ZgpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvZG9tL05vZGUuY3BwIGIvU291cmNlL1dlYkNv cmUvZG9tL05vZGUuY3BwCmluZGV4IGYzZTM5NjlhM2FmNGU1ZWZjMjZiNDAwNzdiMmUxNDE1MDgw MzBiM2UuLjY2MTE5MTU1OGE0NzgzMDlkZmZiZThlMDhlMjFjN2QzY2MzNTQwMjggMTAwNjQ0Ci0t LSBhL1NvdXJjZS9XZWJDb3JlL2RvbS9Ob2RlLmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9kb20v Tm9kZS5jcHAKQEAgLTQwNCwxNiArNDA0LDE3IEBAIE5vZGU6On5Ob2RlKCkKICAgICBpZiAocmVu ZGVyZXIoKSkKICAgICAgICAgZGV0YWNoKCk7CiAKLSAgICBpZiAoQVhPYmplY3RDYWNoZTo6YWNj ZXNzaWJpbGl0eUVuYWJsZWQoKSAmJiBtX2RvY3VtZW50ICYmIG1fZG9jdW1lbnQtPmF4T2JqZWN0 Q2FjaGVFeGlzdHMoKSkKLSAgICAgICAgbV9kb2N1bWVudC0+YXhPYmplY3RDYWNoZSgpLT5yZW1v dmVOb2RlRm9yVXNlKHRoaXMpOworICAgIERvY3VtZW50KiBkb2MgPSBkb2N1bWVudCgpOworICAg IGlmIChBWE9iamVjdENhY2hlOjphY2Nlc3NpYmlsaXR5RW5hYmxlZCgpICYmIGRvYyAmJiBkb2Mt PmF4T2JqZWN0Q2FjaGVFeGlzdHMoKSkKKyAgICAgICAgZG9jLT5heE9iamVjdENhY2hlKCktPnJl bW92ZU5vZGVGb3JVc2UodGhpcyk7CiAgICAgCiAgICAgaWYgKG1fcHJldmlvdXMpCiAgICAgICAg IG1fcHJldmlvdXMtPnNldE5leHRTaWJsaW5nKDApOwogICAgIGlmIChtX25leHQpCiAgICAgICAg IG1fbmV4dC0+c2V0UHJldmlvdXNTaWJsaW5nKDApOwogCi0gICAgaWYgKG1fZG9jdW1lbnQpCi0g ICAgICAgIG1fZG9jdW1lbnQtPmd1YXJkRGVyZWYoKTsKKyAgICBpZiAoZG9jKQorICAgICAgICBk b2MtPmd1YXJkRGVyZWYoKTsKIH0KIAogI2lmZGVmIE5ERUJVRwpkaWZmIC0tZ2l0IGEvU291cmNl L1dlYkNvcmUvZG9tL05vZGUuaCBiL1NvdXJjZS9XZWJDb3JlL2RvbS9Ob2RlLmgKaW5kZXggODQ3 NDQwNTBlMjNjMzViYTkxNWQxODM1OTdlMTI1Y2E2YmNiNmMxNi4uYTgxOTg3YzM3OTBjOTE5MmQ2 ZGNjZWJiZDZhZGY1OTY5OWE5YWRkMSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvZG9tL05v ZGUuaAorKysgYi9Tb3VyY2UvV2ViQ29yZS9kb20vTm9kZS5oCkBAIC0zODgsNyArMzg4LDcgQEAg cHVibGljOgogICAgIC8vIG5vZGUgdHJlZSwgZmFsc2Ugb3RoZXJ3aXNlLgogICAgIGJvb2wgaW5E b2N1bWVudCgpIGNvbnN0IAogICAgIHsgCi0gICAgICAgIEFTU0VSVChtX2RvY3VtZW50IHx8ICFn ZXRGbGFnKEluRG9jdW1lbnRGbGFnKSk7CisgICAgICAgIEFTU0VSVChkb2N1bWVudCgpIHx8ICFn ZXRGbGFnKEluRG9jdW1lbnRGbGFnKSk7CiAgICAgICAgIHJldHVybiBnZXRGbGFnKEluRG9jdW1l bnRGbGFnKTsKICAgICB9CiAK 117993 2011-12-05 23:02:29 -0800 2011-12-06 03:31:01 -0800 A fixed version bug-73800-20111206160227.patch text/plain 2428 morrita U3VidmVyc2lvbiBSZXZpc2lvbjogMTAyMDk3CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNmFjZGYxZDUzOWFmZjlj YjVjNWUzMWRlNWU3ZjY3MGJkMzRjODgzYS4uMzQ5N2JhNDRlOTA3NDI2ZWQ4ZGJmZDdmNzdmOWM3 YTc5NmZhMjllMSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIxIEBACisyMDExLTEyLTA2ICBIYWpp bWUgTW9ycml0YSAgPG1vcnJpdGFAY2hyb21pdW0ub3JnPgorCisgICAgICAgIFtSZWZhY3Rvcmlu Z10gQWNjZXNzaW5nIE5vZGU6Om1fZG9jdW1lbnQgc2hvdWxkIGJlIG1pbmltaXplZC4KKyAgICAg ICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTczODAwCisKKyAgICAg ICAgUmV2aWV3ZWQgYnkgS2VudCBUYW11cmEuCisKKyAgICAgICAgTm8gbmV3IHRlc3RzLiBObyBi ZWhhdmlvcmFsIGNoYW5nZS4KKworICAgICAgICBSZXBsYWNlZCBtX2RvY3VtZW50IHJlZmVyZW5j ZSB3aXRoIHRoZSBkb2N1bWVudCgpIGFjY2Vzc29yCisgICAgICAgIG9yIHRlbXBvcmFyaWwgdmFy aWFibGVzLiBUaGlzIGlzIGEgcHJlcGFyYXRpb24gZm9yIHVzaW5nCisgICAgICAgIG1fZG9jdW1l bnQgc3BhY2UgdG8gcG9pbnQgYSBzaGFkb3cgcm9vdCBwb2ludGVyLgorCisgICAgICAgICogZG9t L0RvY3VtZW50Lmg6CisgICAgICAgIChXZWJDb3JlOjpOb2RlOjpOb2RlKToKKyAgICAgICAgKiBk b20vTm9kZS5jcHA6CisgICAgICAgIChXZWJDb3JlOjpOb2RlOjp+Tm9kZSk6CisKIDIwMTEtMTIt MDUgIEJlbmphbWluIFBvdWxhaW4gIDxicG91bGFpbkBhcHBsZS5jb20+CiAKICAgICAgICAgVXBz dHJlYW0gdGhlIEN1cnNvciBpbXBsZW1lbnRhdGlvbiBvZiBpT1MKZGlmZiAtLWdpdCBhL1NvdXJj ZS9XZWJDb3JlL2RvbS9Eb2N1bWVudC5oIGIvU291cmNlL1dlYkNvcmUvZG9tL0RvY3VtZW50LmgK aW5kZXggNjY3ZWFkZTVmYTYzMDRhYTJhNDRlNjdhODliMTdiYzhlYTZiMGFjYy4uYzlhN2Y0MDZj YjQ0ZTZiZWQ0ZGI5ZjMwYTMzMDE4N2JkYTdiMzZmZCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNv cmUvZG9tL0RvY3VtZW50LmgKKysrIGIvU291cmNlL1dlYkNvcmUvZG9tL0RvY3VtZW50LmgKQEAg LTE0NDksOCArMTQ0OSw4IEBAIGlubGluZSBOb2RlOjpOb2RlKERvY3VtZW50KiBkb2N1bWVudCwg Q29uc3RydWN0aW9uVHlwZSB0eXBlKQogICAgICwgbV9uZXh0KDApCiAgICAgLCBtX3JlbmRlcmVy KDApCiB7Ci0gICAgaWYgKG1fZG9jdW1lbnQpCi0gICAgICAgIG1fZG9jdW1lbnQtPmd1YXJkUmVm KCk7CisgICAgaWYgKGRvY3VtZW50KQorICAgICAgICBkb2N1bWVudC0+Z3VhcmRSZWYoKTsKICNp ZiAhZGVmaW5lZChOREVCVUcpIHx8IChkZWZpbmVkKERVTVBfTk9ERV9TVEFUSVNUSUNTKSAmJiBE VU1QX05PREVfU1RBVElTVElDUykKICAgICB0cmFja0ZvckRlYnVnZ2luZygpOwogI2VuZGlmCmRp ZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9kb20vTm9kZS5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9k b20vTm9kZS5jcHAKaW5kZXggNDZhNzJjZjlhMGJkNWUwYjhjNGUyZTA5MTUyNWJiYzc4NzZjODc2 OS4uMjIwZDBhODg2MmExMWU2OWI2NTY1M2NkYTg3M2RjYzRiMWZmN2FlNSAxMDA2NDQKLS0tIGEv U291cmNlL1dlYkNvcmUvZG9tL05vZGUuY3BwCisrKyBiL1NvdXJjZS9XZWJDb3JlL2RvbS9Ob2Rl LmNwcApAQCAtNDA0LDE2ICs0MDQsMTcgQEAgTm9kZTo6fk5vZGUoKQogICAgIGlmIChyZW5kZXJl cigpKQogICAgICAgICBkZXRhY2goKTsKIAotICAgIGlmIChBWE9iamVjdENhY2hlOjphY2Nlc3Np YmlsaXR5RW5hYmxlZCgpICYmIG1fZG9jdW1lbnQgJiYgbV9kb2N1bWVudC0+YXhPYmplY3RDYWNo ZUV4aXN0cygpKQotICAgICAgICBtX2RvY3VtZW50LT5heE9iamVjdENhY2hlKCktPnJlbW92ZU5v ZGVGb3JVc2UodGhpcyk7CisgICAgRG9jdW1lbnQqIGRvYyA9IG1fZG9jdW1lbnQ7CisgICAgaWYg KEFYT2JqZWN0Q2FjaGU6OmFjY2Vzc2liaWxpdHlFbmFibGVkKCkgJiYgZG9jICYmIGRvYy0+YXhP YmplY3RDYWNoZUV4aXN0cygpKQorICAgICAgICBkb2MtPmF4T2JqZWN0Q2FjaGUoKS0+cmVtb3Zl Tm9kZUZvclVzZSh0aGlzKTsKICAgICAKICAgICBpZiAobV9wcmV2aW91cykKICAgICAgICAgbV9w cmV2aW91cy0+c2V0TmV4dFNpYmxpbmcoMCk7CiAgICAgaWYgKG1fbmV4dCkKICAgICAgICAgbV9u ZXh0LT5zZXRQcmV2aW91c1NpYmxpbmcoMCk7CiAKLSAgICBpZiAobV9kb2N1bWVudCkKLSAgICAg ICAgbV9kb2N1bWVudC0+Z3VhcmREZXJlZigpOworICAgIGlmIChkb2MpCisgICAgICAgIGRvYy0+ Z3VhcmREZXJlZigpOwogfQogCiAjaWZkZWYgTkRFQlVHCg==