108195 2013-01-29 07:57:17 -0800 Move ElementShadow creation to ElementRareData 2013-01-29 10:37:07 -0800 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 esprehn esprehn cmarcelo darin dglazkov morrita ojan.autocc ojan webkit.review.bot oldest_to_newest 818949 0 esprehn 2013-01-29 07:57:17 -0800 Move ElementShadow creation to ElementRareData 818950 1 185248 esprehn 2013-01-29 07:58:44 -0800 Created attachment 185248 Patch 819028 2 185248 dglazkov 2013-01-29 09:49:52 -0800 Comment on attachment 185248 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=185248&action=review > Source/WebCore/ChangeLog:13 > + * dom/Element.cpp: It's good WebKit practice to describe changes inline here. Darin Adler once told me: "If you're not putting anything here, might as well delete this text". 819047 3 185248 esprehn 2013-01-29 10:04:44 -0800 Comment on attachment 185248 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=185248&action=review >> Source/WebCore/ChangeLog:13 >> + * dom/Element.cpp: > > It's good WebKit practice to describe changes inline here. Darin Adler once told me: "If you're not putting anything here, might as well delete this text". Okay. 819048 4 185257 esprehn 2013-01-29 10:04:54 -0800 Created attachment 185257 Patch for landing 819066 5 185257 webkit.review.bot 2013-01-29 10:23:34 -0800 Comment on attachment 185257 Patch for landing Clearing flags on attachment: 185257 Committed r141132: <http://trac.webkit.org/changeset/141132> 819067 6 webkit.review.bot 2013-01-29 10:23:38 -0800 All reviewed patches have been landed. Closing bug. 819072 7 darin 2013-01-29 10:27:38 -0800 This kind of change is OK, but the name “data” in the names of classes like NodeRareData and ElementRareData is supposed to indicate that those classes hold data but not policy, and generally speaking we want most of the code to be in the Node and Element class, not the NodeRareData and ElementRareData classes. As soon as the code was anything more than ElementShadow::create() I think we’d not want to make this sort of change. 819077 8 esprehn 2013-01-29 10:33:42 -0800 (In reply to comment #7) > This kind of change is OK, but the name “data” in the names of classes like NodeRareData and ElementRareData is supposed to indicate that those classes hold data but not policy, and generally speaking we want most of the code to be in the Node and Element class, not the NodeRareData and ElementRareData classes. As soon as the code was anything more than ElementShadow::create() I think we’d not want to make this sort of change. Okay. My change is inline with all the code in NodeRareData though: see ensureNodeLists, ensureMutationObserverData, ensureMicroDataTokenLists etc. 819084 9 darin 2013-01-29 10:37:07 -0800 (In reply to comment #8) > My change is inline with all the code in NodeRareData though: see ensureNodeLists, ensureMutationObserverData, ensureMicroDataTokenLists etc. Sure. Yours was just the one I happened to spot. I haven’t been watching all the check-ins as much as I used to. 185248 2013-01-29 07:58:44 -0800 2013-01-29 10:04:48 -0800 Patch bug-108195-20130129105530.patch text/plain 3113 esprehn U3VidmVyc2lvbiBSZXZpc2lvbjogMTQxMTEwCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNTJmY2NmODU4YmIxMDlm MmQ0NzE1NTQzNDhkYjVmODVmNDljMThlNy4uOWViMDdlNzNhMzBlNmMyNGE3NzMzNGUwNmY0ZDA5 Y2FmYjdlN2Q4MiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIzIEBACisyMDEzLTAxLTI5ICBFbGxp b3R0IFNwcmVobiAgPGVzcHJlaG5AY2hyb21pdW0ub3JnPgorCisgICAgICAgIE1vdmUgRWxlbWVu dFNoYWRvdyBjcmVhdGlvbiB0byBFbGVtZW50UmFyZURhdGEKKyAgICAgICAgaHR0cHM6Ly9idWdz LndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTEwODE5NQorCisgICAgICAgIFJldmlld2VkIGJ5 IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIE1vdmUgdGhlIGNyZWF0aW9uIG9mIEVsZW1lbnRT aGFkb3cgdG8gRWxlbWVudFJhcmVEYXRhCisgICAgICAgIGZvciBiZXR0ZXIgZW5jYXBzdWxhdGlv bi4KKworICAgICAgICBObyBuZXcgdGVzdHMsIGp1c3QgcmVmYWN0b3JpbmcuCisKKyAgICAgICAg KiBkb20vRWxlbWVudC5jcHA6CisgICAgICAgIChXZWJDb3JlOjpFbGVtZW50Ojp+RWxlbWVudCk6 CisgICAgICAgIChXZWJDb3JlOjpFbGVtZW50OjpzaGFkb3cpOgorICAgICAgICAoV2ViQ29yZTo6 RWxlbWVudDo6ZW5zdXJlU2hhZG93KToKKyAgICAgICAgKiBkb20vRWxlbWVudFJhcmVEYXRhLmg6 CisgICAgICAgIChXZWJDb3JlOjpFbGVtZW50UmFyZURhdGE6OmNsZWFyU2hhZG93KToKKyAgICAg ICAgKFdlYkNvcmU6OkVsZW1lbnRSYXJlRGF0YTo6ZW5zdXJlU2hhZG93KToKKwogMjAxMy0wMS0y OSAgR3J6ZWdvcnogQ3phamtvd3NraSAgPGcuY3phamtvd3NraUBzYW1zdW5nLmNvbT4KIAogICAg ICAgICBbRUZMXSBVbmlmaWVkIHRleHQgY2hlY2tlciBpbXBsZW1lbnRhdGlvbi4KZGlmZiAtLWdp dCBhL1NvdXJjZS9XZWJDb3JlL2RvbS9FbGVtZW50LmNwcCBiL1NvdXJjZS9XZWJDb3JlL2RvbS9F bGVtZW50LmNwcAppbmRleCA5ODk5NDA1N2ZjMzViNTNiNzUxNGYzZjUwMzE5ZjRiZTE1YTE1ZTE2 Li4zMDdmZTYxMzFiOTE3MDJiNTU4YzdhMTUwNDkzZGM1MjU5NjNmNzA2IDEwMDY0NAotLS0gYS9T b3VyY2UvV2ViQ29yZS9kb20vRWxlbWVudC5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvZG9tL0Vs ZW1lbnQuY3BwCkBAIC0xOTgsNyArMTk4LDcgQEAgRWxlbWVudDo6fkVsZW1lbnQoKQogCiAgICAg aWYgKEVsZW1lbnRTaGFkb3cqIGVsZW1lbnRTaGFkb3cgPSBzaGFkb3coKSkgewogICAgICAgICBl bGVtZW50U2hhZG93LT5yZW1vdmVBbGxTaGFkb3dSb290cygpOwotICAgICAgICBlbGVtZW50UmFy ZURhdGEoKS0+c2V0U2hhZG93KG51bGxwdHIpOworICAgICAgICBlbGVtZW50UmFyZURhdGEoKS0+ Y2xlYXJTaGFkb3coKTsKICAgICB9CiAKICAgICBpZiAoaGFzU3ludGhldGljQXR0ckNoaWxkTm9k ZXMoKSkKQEAgLTE0MzcsMjAgKzE0MzcsMTIgQEAgdm9pZCBFbGVtZW50OjpyZWNhbGNTdHlsZShT dHlsZUNoYW5nZSBjaGFuZ2UpCiAKIEVsZW1lbnRTaGFkb3cqIEVsZW1lbnQ6OnNoYWRvdygpIGNv bnN0CiB7Ci0gICAgaWYgKCFoYXNSYXJlRGF0YSgpKQotICAgICAgICByZXR1cm4gMDsKLQotICAg IHJldHVybiBlbGVtZW50UmFyZURhdGEoKS0+c2hhZG93KCk7CisgICAgcmV0dXJuIGhhc1JhcmVE YXRhKCkgPyBlbGVtZW50UmFyZURhdGEoKS0+c2hhZG93KCkgOiAwOwogfQogCiBFbGVtZW50U2hh ZG93KiBFbGVtZW50OjplbnN1cmVTaGFkb3coKQogewotICAgIGlmIChFbGVtZW50U2hhZG93KiBz aGFkb3cgPSBlbnN1cmVFbGVtZW50UmFyZURhdGEoKS0+c2hhZG93KCkpCi0gICAgICAgIHJldHVy biBzaGFkb3c7Ci0KLSAgICBFbGVtZW50UmFyZURhdGEqIGRhdGEgPSBlbGVtZW50UmFyZURhdGEo KTsKLSAgICBkYXRhLT5zZXRTaGFkb3coRWxlbWVudFNoYWRvdzo6Y3JlYXRlKCkpOwotICAgIHJl dHVybiBkYXRhLT5zaGFkb3coKTsKKyAgICByZXR1cm4gZW5zdXJlRWxlbWVudFJhcmVEYXRhKCkt PmVuc3VyZVNoYWRvdygpOwogfQogCiB2b2lkIEVsZW1lbnQ6OmRpZEFmZmVjdFNlbGVjdG9yKEFm ZmVjdGVkU2VsZWN0b3JNYXNrIG1hc2spCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9kb20v RWxlbWVudFJhcmVEYXRhLmggYi9Tb3VyY2UvV2ViQ29yZS9kb20vRWxlbWVudFJhcmVEYXRhLmgK aW5kZXggYjM0N2IyODJhOTcxNGM3MGVkNDA0YmI2ZWYyNTk1NmEyYWU1NjBmOS4uZGNmNmIzYTZl YjgyZjc2YjRiODc0MTNhMDllZmMwZGZlM2MxNmNlYSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNv cmUvZG9tL0VsZW1lbnRSYXJlRGF0YS5oCisrKyBiL1NvdXJjZS9XZWJDb3JlL2RvbS9FbGVtZW50 UmFyZURhdGEuaApAQCAtOTMsOCArOTMsMTQgQEAgcHVibGljOgogICAgIC8vIE1hbnVhbGx5IGNh bGxlZCBieSBOb2RlOjpyZXBvcnRNZW1vcnlVc2FnZS4KICAgICB2b2lkIHJlcG9ydE1lbW9yeVVz YWdlKE1lbW9yeU9iamVjdEluZm8qKSBjb25zdDsKIAorICAgIHZvaWQgY2xlYXJTaGFkb3coKSB7 IG1fc2hhZG93ID0gbnVsbHB0cjsgfQogICAgIEVsZW1lbnRTaGFkb3cqIHNoYWRvdygpIGNvbnN0 IHsgcmV0dXJuIG1fc2hhZG93LmdldCgpOyB9Ci0gICAgdm9pZCBzZXRTaGFkb3coUGFzc093blB0 cjxFbGVtZW50U2hhZG93PiBzaGFkb3cpIHsgbV9zaGFkb3cgPSBzaGFkb3c7IH0KKyAgICBFbGVt ZW50U2hhZG93KiBlbnN1cmVTaGFkb3coKQorICAgIHsKKyAgICAgICAgaWYgKCFtX3NoYWRvdykK KyAgICAgICAgICAgIG1fc2hhZG93ID0gRWxlbWVudFNoYWRvdzo6Y3JlYXRlKCk7CisgICAgICAg IHJldHVybiBtX3NoYWRvdy5nZXQoKTsKKyAgICB9CiAKICAgICBOYW1lZE5vZGVNYXAqIGF0dHJp YnV0ZU1hcCgpIGNvbnN0IHsgcmV0dXJuIG1fYXR0cmlidXRlTWFwLmdldCgpOyB9CiAgICAgdm9p ZCBzZXRBdHRyaWJ1dGVNYXAoUGFzc093blB0cjxOYW1lZE5vZGVNYXA+IGF0dHJpYnV0ZU1hcCkg eyBtX2F0dHJpYnV0ZU1hcCA9IGF0dHJpYnV0ZU1hcDsgfQo= 185257 2013-01-29 10:04:54 -0800 2013-01-29 10:23:34 -0800 Patch for landing bug-108195-20130129130140.patch text/plain 3265 esprehn U3VidmVyc2lvbiBSZXZpc2lvbjogMTQxMTEwCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNTJmY2NmODU4YmIxMDlm MmQ0NzE1NTQzNDhkYjVmODVmNDljMThlNy4uYjVjMmU5NjJkOWU1ZTM0YWEyNzU3ZWMyMjdmOWVh NmE0ZDNkMzNhZiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIzIEBACisyMDEzLTAxLTI5ICBFbGxp b3R0IFNwcmVobiAgPGVzcHJlaG5AY2hyb21pdW0ub3JnPgorCisgICAgICAgIE1vdmUgRWxlbWVu dFNoYWRvdyBjcmVhdGlvbiB0byBFbGVtZW50UmFyZURhdGEKKyAgICAgICAgaHR0cHM6Ly9idWdz LndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTEwODE5NQorCisgICAgICAgIFJldmlld2VkIGJ5 IERpbWl0cmkgR2xhemtvdi4KKworICAgICAgICBNb3ZlIHRoZSBjcmVhdGlvbiBvZiBFbGVtZW50 U2hhZG93IHRvIEVsZW1lbnRSYXJlRGF0YQorICAgICAgICBmb3IgYmV0dGVyIGVuY2Fwc3VsYXRp b24sIGFuZCBnZXQgcmlkIG9mIEVsZW1lbnRSYXJlRGF0YTo6c2V0U2hhZG93LgorCisgICAgICAg IE5vIG5ldyB0ZXN0cywganVzdCByZWZhY3RvcmluZy4KKworICAgICAgICAqIGRvbS9FbGVtZW50 LmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OkVsZW1lbnQ6On5FbGVtZW50KTogVXNlIGNsZWFyU2hh ZG93KCkgaW5zdGVhZCBvZiBzZXRTaGFkb3cgd2hpY2ggaXMgcmVtb3ZlZC4KKyAgICAgICAgKFdl YkNvcmU6OkVsZW1lbnQ6OnNoYWRvdyk6CisgICAgICAgIChXZWJDb3JlOjpFbGVtZW50OjplbnN1 cmVTaGFkb3cpOiBVc2UgRWxlbWVudFJhcmVEYXRhOjplbnN1cmVTaGFkb3coKS4KKyAgICAgICAg KiBkb20vRWxlbWVudFJhcmVEYXRhLmg6CisgICAgICAgIChXZWJDb3JlOjpFbGVtZW50UmFyZURh dGE6OmNsZWFyU2hhZG93KTogQWRkZWQuCisgICAgICAgIChXZWJDb3JlOjpFbGVtZW50UmFyZURh dGE6OmVuc3VyZVNoYWRvdyk6IEFkZGVkLgorCiAyMDEzLTAxLTI5ICBHcnplZ29yeiBDemFqa293 c2tpICA8Zy5jemFqa293c2tpQHNhbXN1bmcuY29tPgogCiAgICAgICAgIFtFRkxdIFVuaWZpZWQg dGV4dCBjaGVja2VyIGltcGxlbWVudGF0aW9uLgpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUv ZG9tL0VsZW1lbnQuY3BwIGIvU291cmNlL1dlYkNvcmUvZG9tL0VsZW1lbnQuY3BwCmluZGV4IDk4 OTk0MDU3ZmMzNWI1M2I3NTE0ZjNmNTAzMTlmNGJlMTVhMTVlMTYuLjMwN2ZlNjEzMWI5MTcwMmI1 NThjN2ExNTA0OTNkYzUyNTk2M2Y3MDYgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL2RvbS9F bGVtZW50LmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9kb20vRWxlbWVudC5jcHAKQEAgLTE5OCw3 ICsxOTgsNyBAQCBFbGVtZW50Ojp+RWxlbWVudCgpCiAKICAgICBpZiAoRWxlbWVudFNoYWRvdyog ZWxlbWVudFNoYWRvdyA9IHNoYWRvdygpKSB7CiAgICAgICAgIGVsZW1lbnRTaGFkb3ctPnJlbW92 ZUFsbFNoYWRvd1Jvb3RzKCk7Ci0gICAgICAgIGVsZW1lbnRSYXJlRGF0YSgpLT5zZXRTaGFkb3co bnVsbHB0cik7CisgICAgICAgIGVsZW1lbnRSYXJlRGF0YSgpLT5jbGVhclNoYWRvdygpOwogICAg IH0KIAogICAgIGlmIChoYXNTeW50aGV0aWNBdHRyQ2hpbGROb2RlcygpKQpAQCAtMTQzNywyMCAr MTQzNywxMiBAQCB2b2lkIEVsZW1lbnQ6OnJlY2FsY1N0eWxlKFN0eWxlQ2hhbmdlIGNoYW5nZSkK IAogRWxlbWVudFNoYWRvdyogRWxlbWVudDo6c2hhZG93KCkgY29uc3QKIHsKLSAgICBpZiAoIWhh c1JhcmVEYXRhKCkpCi0gICAgICAgIHJldHVybiAwOwotCi0gICAgcmV0dXJuIGVsZW1lbnRSYXJl RGF0YSgpLT5zaGFkb3coKTsKKyAgICByZXR1cm4gaGFzUmFyZURhdGEoKSA/IGVsZW1lbnRSYXJl RGF0YSgpLT5zaGFkb3coKSA6IDA7CiB9CiAKIEVsZW1lbnRTaGFkb3cqIEVsZW1lbnQ6OmVuc3Vy ZVNoYWRvdygpCiB7Ci0gICAgaWYgKEVsZW1lbnRTaGFkb3cqIHNoYWRvdyA9IGVuc3VyZUVsZW1l bnRSYXJlRGF0YSgpLT5zaGFkb3coKSkKLSAgICAgICAgcmV0dXJuIHNoYWRvdzsKLQotICAgIEVs ZW1lbnRSYXJlRGF0YSogZGF0YSA9IGVsZW1lbnRSYXJlRGF0YSgpOwotICAgIGRhdGEtPnNldFNo YWRvdyhFbGVtZW50U2hhZG93OjpjcmVhdGUoKSk7Ci0gICAgcmV0dXJuIGRhdGEtPnNoYWRvdygp OworICAgIHJldHVybiBlbnN1cmVFbGVtZW50UmFyZURhdGEoKS0+ZW5zdXJlU2hhZG93KCk7CiB9 CiAKIHZvaWQgRWxlbWVudDo6ZGlkQWZmZWN0U2VsZWN0b3IoQWZmZWN0ZWRTZWxlY3Rvck1hc2sg bWFzaykKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL2RvbS9FbGVtZW50UmFyZURhdGEuaCBi L1NvdXJjZS9XZWJDb3JlL2RvbS9FbGVtZW50UmFyZURhdGEuaAppbmRleCBiMzQ3YjI4MmE5NzE0 YzcwZWQ0MDRiYjZlZjI1OTU2YTJhZTU2MGY5Li5kY2Y2YjNhNmViODJmNzZiNGI4NzQxM2EwOWVm YzBkZmUzYzE2Y2VhIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9kb20vRWxlbWVudFJhcmVE YXRhLmgKKysrIGIvU291cmNlL1dlYkNvcmUvZG9tL0VsZW1lbnRSYXJlRGF0YS5oCkBAIC05Myw4 ICs5MywxNCBAQCBwdWJsaWM6CiAgICAgLy8gTWFudWFsbHkgY2FsbGVkIGJ5IE5vZGU6OnJlcG9y dE1lbW9yeVVzYWdlLgogICAgIHZvaWQgcmVwb3J0TWVtb3J5VXNhZ2UoTWVtb3J5T2JqZWN0SW5m byopIGNvbnN0OwogCisgICAgdm9pZCBjbGVhclNoYWRvdygpIHsgbV9zaGFkb3cgPSBudWxscHRy OyB9CiAgICAgRWxlbWVudFNoYWRvdyogc2hhZG93KCkgY29uc3QgeyByZXR1cm4gbV9zaGFkb3cu Z2V0KCk7IH0KLSAgICB2b2lkIHNldFNoYWRvdyhQYXNzT3duUHRyPEVsZW1lbnRTaGFkb3c+IHNo YWRvdykgeyBtX3NoYWRvdyA9IHNoYWRvdzsgfQorICAgIEVsZW1lbnRTaGFkb3cqIGVuc3VyZVNo YWRvdygpCisgICAgeworICAgICAgICBpZiAoIW1fc2hhZG93KQorICAgICAgICAgICAgbV9zaGFk b3cgPSBFbGVtZW50U2hhZG93OjpjcmVhdGUoKTsKKyAgICAgICAgcmV0dXJuIG1fc2hhZG93Lmdl dCgpOworICAgIH0KIAogICAgIE5hbWVkTm9kZU1hcCogYXR0cmlidXRlTWFwKCkgY29uc3QgeyBy ZXR1cm4gbV9hdHRyaWJ1dGVNYXAuZ2V0KCk7IH0KICAgICB2b2lkIHNldEF0dHJpYnV0ZU1hcChQ YXNzT3duUHRyPE5hbWVkTm9kZU1hcD4gYXR0cmlidXRlTWFwKSB7IG1fYXR0cmlidXRlTWFwID0g YXR0cmlidXRlTWFwOyB9Cg==