132796 2014-05-10 22:39:33 -0700 QualifiedName should use RefPtr<QualifiedNameImpl> internally. 2014-07-04 18:31:20 -0700 1 1 1 Unclassified WebKit DOM 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 kling kling commit-queue esprehn+autocc kangil.han kling oldest_to_newest 1008374 0 kling 2014-05-10 22:39:33 -0700 QualifiedName should use RefPtr<QualifiedNameImpl> internally. This'll be simpler and we can make ~QualifiedName() inline. 1008375 1 231250 kling 2014-05-10 22:42:57 -0700 Created attachment 231250 Patch 1008376 2 commit-queue 2014-05-10 22:45:07 -0700 Attachment 231250 did not pass style-queue: ERROR: Source/WebCore/dom/QualifiedName.h:74: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WebCore/dom/QualifiedName.h:81: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] Total errors found: 2 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style. 1008424 3 231250 darin 2014-05-11 10:51:09 -0700 Comment on attachment 231250 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=231250&action=review These are the Windows build errors: 1>..\dom\QualifiedName.cpp(100): error C2734: 'WebCore::anyName' : const object must be initialized if not extern 1>..\dom\QualifiedName.cpp(100): warning C4211: nonstandard extension used : redefined extern to static 1>..\dom\QualifiedName.cpp(100): error C2512: 'WebCore::QualifiedName' : no appropriate default constructor available Looks good as long as you resolve that before landing. I think QNAME_DEFAULT_CONSTRUCTOR was doing some good on Windows. > Source/WebCore/dom/QualifiedName.cpp:81 > QualifiedNameComponents components = { p.impl(), l.impl(), n.isEmpty() ? nullAtom.impl() : n.impl() }; Isn’t nullAtom.impl() the same thing as nullptr? > Source/WebCore/dom/QualifiedName.cpp:83 > + m_impl = addResult.isNewEntry ? adoptRef(*addResult.iterator) : *addResult.iterator; Can we make an inline helper function to do this work? Then we can use construction instead of assignment for m_impl, and possibly even save a branch. I also think we could go nuts and use words for the three arguments instead of letters. > Source/WebCore/dom/QualifiedName.h:74 > + explicit QualifiedName(WTF::HashTableDeletedValueType) : m_impl(WTF::HashTableDeletedValue) { } Are the explicit WTF prefixes needed here? Fine to leave them in if they are indeed needed. > Source/WebCore/dom/QualifiedName.h:76 > + ~QualifiedName() { } I believe the best way to get this effect, a public empty destructor that gets inlined, is to omit explicit declaration or definition of the destructor entirely. > Source/WebCore/dom/QualifiedName.h:78 > QualifiedName() : m_impl(0) { } Maybe nullptr this while you are in the neighborhood. 1020553 4 kling 2014-07-04 18:31:20 -0700 Committed r170816: <http://trac.webkit.org/changeset/170816> 231250 2014-05-10 22:42:57 -0700 2014-05-11 10:51:09 -0700 Patch bug-132796.diff text/plain 6491 kling ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCA0ZjM1Mjc2Li4xZTNjODM3IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMzIg QEAKKzIwMTQtMDUtMTAgIEFuZHJlYXMgS2xpbmcgIDxha2xpbmdAYXBwbGUuY29tPgorCisgICAg ICAgIFF1YWxpZmllZE5hbWUgc2hvdWxkIHVzZSBSZWZQdHI8UXVhbGlmaWVkTmFtZUltcGw+IGlu dGVybmFsbHkuCisgICAgICAgIDxodHRwczovL3dlYmtpdC5vcmcvYi8xMzI3OTY+CisKKyAgICAg ICAgV2Ugd2VyZSBub3QgaW5saW5pbmcgflF1YWxpZmllZE5hbWUoKSBhbmQgaXQgd2FzIHNob3dp bmcgdXAgaW4gc29tZQorICAgICAgICBwcm9maWxlcyAob3ZlciAxJSBvbiBkb20tYXR0ci5odG1s LikKKworICAgICAgICBUaGlzIHBhdGNoIG1vZGVybml6ZXMgUXVhbGlmaWVkTmFtZSBhIGJpdCBi eSB1c2luZyBSZWZQdHIgdG8gc3RvcmUKKyAgICAgICAgdGhlIFF1YWxpZmllZE5hbWVJbXBsIGlu c3RlYWQgb2YgZG9pbmcgbWFudWFsIHJlZigpIGFuZCBkZXJlZigpCisgICAgICAgIGluIG91dC1v Zi1saW5lIGZ1bmN0aW9ucy4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4K KworICAgICAgICAqIFdlYkNvcmUuZXhwLmluOgorICAgICAgICAqIGRvbS9RdWFsaWZpZWROYW1l LmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OlFOYW1lQ29tcG9uZW50c1RyYW5zbGF0b3I6OnRyYW5z bGF0ZSk6CisgICAgICAgIChXZWJDb3JlOjpRdWFsaWZpZWROYW1lOjpRdWFsaWZpZWROYW1lKToK KyAgICAgICAgKFdlYkNvcmU6OlF1YWxpZmllZE5hbWU6On5RdWFsaWZpZWROYW1lKTogRGVsZXRl ZC4KKyAgICAgICAgKFdlYkNvcmU6OlF1YWxpZmllZE5hbWU6OmRlcmVmKTogRGVsZXRlZC4KKyAg ICAgICAgKiBkb20vUXVhbGlmaWVkTmFtZS5oOgorICAgICAgICAoV2ViQ29yZTo6UXVhbGlmaWVk TmFtZTo6UXVhbGlmaWVkTmFtZUltcGw6OmNyZWF0ZSk6CisgICAgICAgIChXZWJDb3JlOjpRdWFs aWZpZWROYW1lOjpRdWFsaWZpZWROYW1lKToKKyAgICAgICAgKFdlYkNvcmU6OlF1YWxpZmllZE5h bWU6OmlzSGFzaFRhYmxlRGVsZXRlZFZhbHVlKToKKyAgICAgICAgKFdlYkNvcmU6OlF1YWxpZmll ZE5hbWU6On5RdWFsaWZpZWROYW1lKToKKyAgICAgICAgKFdlYkNvcmU6OlF1YWxpZmllZE5hbWU6 Om9wZXJhdG9yPSk6CisgICAgICAgIChXZWJDb3JlOjpRdWFsaWZpZWROYW1lOjppbXBsKToKKyAg ICAgICAgKFdlYkNvcmU6OlF1YWxpZmllZE5hbWU6OnJlZik6IERlbGV0ZWQuCisKIDIwMTQtMDUt MTAgIERhcmluIEFkbGVyICA8ZGFyaW5AYXBwbGUuY29tPgogCiAgICAgICAgIFJFR1JFU1NJT04g KHIxNjY4NTMpOiBmYXN0L3ByZWxvYWRlci9kb2N1bWVudC13cml0ZS5odG1sIGlzIHZlcnkgZmxh a3kKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL1dlYkNvcmUuZXhwLmluIGIvU291cmNlL1dl YkNvcmUvV2ViQ29yZS5leHAuaW4KaW5kZXggN2ZkM2MwNS4uMWU2ZjRiMCAxMDA2NDQKLS0tIGEv U291cmNlL1dlYkNvcmUvV2ViQ29yZS5leHAuaW4KKysrIGIvU291cmNlL1dlYkNvcmUvV2ViQ29y ZS5leHAuaW4KQEAgLTMwNCw3ICszMDQsNiBAQCBfX1pON1dlYkNvcmUxM05vZGVUcmF2ZXJzYWwx M2RlZXBMYXN0Q2hpbGRFUE5TXzROb2RlRQogX19aTjdXZWJDb3JlMTNOb2RlVHJhdmVyc2FsMTlu ZXh0QW5jZXN0b3JTaWJsaW5nRVBLTlNfNE5vZGVFCiBfX1pON1dlYkNvcmUxM05vZGVUcmF2ZXJz YWwxOW5leHRBbmNlc3RvclNpYmxpbmdFUEtOU180Tm9kZUVTM18KIF9fWk43V2ViQ29yZTEzUGFn ZVRocm90dGxlcjdzdGFydGVkRXYKLV9fWk43V2ViQ29yZTEzUXVhbGlmaWVkTmFtZUQxRXYKIF9f Wk43V2ViQ29yZTEzUmVzb3VyY2VFcnJvckMxRVA3TlNFcnJvcgogX19aTjdXZWJDb3JlMTNSZXNv dXJjZUVycm9yQzFFUDlfX0NGRXJyb3IKIF9fWk43V2ViQ29yZTEzU1FMUmVzdWx0RG9uZUUKZGlm ZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL2RvbS9RdWFsaWZpZWROYW1lLmNwcCBiL1NvdXJjZS9X ZWJDb3JlL2RvbS9RdWFsaWZpZWROYW1lLmNwcAppbmRleCAzNjgxYTE0Li4zYmI0NThlIDEwMDY0 NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9kb20vUXVhbGlmaWVkTmFtZS5jcHAKKysrIGIvU291cmNl L1dlYkNvcmUvZG9tL1F1YWxpZmllZE5hbWUuY3BwCkBAIC0yMSw4ICsyMSw2IEBACiAKICNpZmRl ZiBTS0lQX1NUQVRJQ19DT05TVFJVQ1RPUlNfT05fR0NDCiAjZGVmaW5lIFdFQkNPUkVfUVVBTElG SUVETkFNRV9ISURFX0dMT0JBTFMgMQotI2Vsc2UKLSNkZWZpbmUgUU5BTUVfREVGQVVMVF9DT05T VFJVQ1RPUgogI2VuZGlmCiAKICNpbmNsdWRlICJRdWFsaWZpZWROYW1lLmgiCkBAIC02OCw3ICs2 Niw3IEBAIHN0cnVjdCBRTmFtZUNvbXBvbmVudHNUcmFuc2xhdG9yIHsKICAgICB9CiAgICAgc3Rh dGljIHZvaWQgdHJhbnNsYXRlKFF1YWxpZmllZE5hbWU6OlF1YWxpZmllZE5hbWVJbXBsKiYgbG9j YXRpb24sIGNvbnN0IFF1YWxpZmllZE5hbWVDb21wb25lbnRzJiBjb21wb25lbnRzLCB1bnNpZ25l ZCkKICAgICB7Ci0gICAgICAgIGxvY2F0aW9uID0gUXVhbGlmaWVkTmFtZTo6UXVhbGlmaWVkTmFt ZUltcGw6OmNyZWF0ZShjb21wb25lbnRzLm1fcHJlZml4LCBjb21wb25lbnRzLm1fbG9jYWxOYW1l LCBjb21wb25lbnRzLm1fbmFtZXNwYWNlKS5sZWFrUmVmKCk7CisgICAgICAgIGxvY2F0aW9uID0g JlF1YWxpZmllZE5hbWU6OlF1YWxpZmllZE5hbWVJbXBsOjpjcmVhdGUoY29tcG9uZW50cy5tX3By ZWZpeCwgY29tcG9uZW50cy5tX2xvY2FsTmFtZSwgY29tcG9uZW50cy5tX25hbWVzcGFjZSkubGVh a1JlZigpOwogICAgIH0KIH07CiAKQEAgLTgyLDI0ICs4MCw3IEBAIFF1YWxpZmllZE5hbWU6OlF1 YWxpZmllZE5hbWUoY29uc3QgQXRvbWljU3RyaW5nJiBwLCBjb25zdCBBdG9taWNTdHJpbmcmIGws IGNvbnN0CiB7CiAgICAgUXVhbGlmaWVkTmFtZUNvbXBvbmVudHMgY29tcG9uZW50cyA9IHsgcC5p bXBsKCksIGwuaW1wbCgpLCBuLmlzRW1wdHkoKSA/IG51bGxBdG9tLmltcGwoKSA6IG4uaW1wbCgp IH07CiAgICAgUU5hbWVTZXQ6OkFkZFJlc3VsdCBhZGRSZXN1bHQgPSBxdWFsaWZpZWROYW1lQ2Fj aGUoKS5hZGQ8UU5hbWVDb21wb25lbnRzVHJhbnNsYXRvcj4oY29tcG9uZW50cyk7Ci0gICAgbV9p bXBsID0gKmFkZFJlc3VsdC5pdGVyYXRvcjsKLSAgICBpZiAoIWFkZFJlc3VsdC5pc05ld0VudHJ5 KQotICAgICAgICBtX2ltcGwtPnJlZigpOwotfQotCi1RdWFsaWZpZWROYW1lOjp+UXVhbGlmaWVk TmFtZSgpCi17Ci0gICAgZGVyZWYoKTsKLX0KLQotdm9pZCBRdWFsaWZpZWROYW1lOjpkZXJlZigp Ci17Ci0jaWZkZWYgUU5BTUVfREVGQVVMVF9DT05TVFJVQ1RPUgotICAgIGlmICghbV9pbXBsKQot ICAgICAgICByZXR1cm47Ci0jZW5kaWYKLSAgICBBU1NFUlQoIWlzSGFzaFRhYmxlRGVsZXRlZFZh bHVlKCkpOwotICAgIG1faW1wbC0+ZGVyZWYoKTsKKyAgICBtX2ltcGwgPSBhZGRSZXN1bHQuaXNO ZXdFbnRyeSA/IGFkb3B0UmVmKCphZGRSZXN1bHQuaXRlcmF0b3IpIDogKmFkZFJlc3VsdC5pdGVy YXRvcjsKIH0KIAogUXVhbGlmaWVkTmFtZTo6UXVhbGlmaWVkTmFtZUltcGw6On5RdWFsaWZpZWRO YW1lSW1wbCgpCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9kb20vUXVhbGlmaWVkTmFtZS5o IGIvU291cmNlL1dlYkNvcmUvZG9tL1F1YWxpZmllZE5hbWUuaAppbmRleCBjNjRhYzJkLi5lMDcx ZDg3IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9kb20vUXVhbGlmaWVkTmFtZS5oCisrKyBi L1NvdXJjZS9XZWJDb3JlL2RvbS9RdWFsaWZpZWROYW1lLmgKQEAgLTM5LDkgKzM5LDkgQEAgY2xh c3MgUXVhbGlmaWVkTmFtZSB7CiBwdWJsaWM6CiAgICAgY2xhc3MgUXVhbGlmaWVkTmFtZUltcGwg OiBwdWJsaWMgUmVmQ291bnRlZDxRdWFsaWZpZWROYW1lSW1wbD4gewogICAgIHB1YmxpYzoKLSAg ICAgICAgc3RhdGljIFBhc3NSZWZQdHI8UXVhbGlmaWVkTmFtZUltcGw+IGNyZWF0ZShjb25zdCBB dG9taWNTdHJpbmcmIHByZWZpeCwgY29uc3QgQXRvbWljU3RyaW5nJiBsb2NhbE5hbWUsIGNvbnN0 IEF0b21pY1N0cmluZyYgbmFtZXNwYWNlVVJJKQorICAgICAgICBzdGF0aWMgUGFzc1JlZjxRdWFs aWZpZWROYW1lSW1wbD4gY3JlYXRlKGNvbnN0IEF0b21pY1N0cmluZyYgcHJlZml4LCBjb25zdCBB dG9taWNTdHJpbmcmIGxvY2FsTmFtZSwgY29uc3QgQXRvbWljU3RyaW5nJiBuYW1lc3BhY2VVUkkp CiAgICAgICAgIHsKLSAgICAgICAgICAgIHJldHVybiBhZG9wdFJlZihuZXcgUXVhbGlmaWVkTmFt ZUltcGwocHJlZml4LCBsb2NhbE5hbWUsIG5hbWVzcGFjZVVSSSkpOworICAgICAgICAgICAgcmV0 dXJuIGFkb3B0UmVmKCpuZXcgUXVhbGlmaWVkTmFtZUltcGwocHJlZml4LCBsb2NhbE5hbWUsIG5h bWVzcGFjZVVSSSkpOwogICAgICAgICB9CiAKICAgICAgICAgflF1YWxpZmllZE5hbWVJbXBsKCk7 CkBAIC03MSwxNSArNzEsMTUgQEAgcHVibGljOgogICAgIH07CiAKICAgICBRdWFsaWZpZWROYW1l KGNvbnN0IEF0b21pY1N0cmluZyYgcHJlZml4LCBjb25zdCBBdG9taWNTdHJpbmcmIGxvY2FsTmFt ZSwgY29uc3QgQXRvbWljU3RyaW5nJiBuYW1lc3BhY2VVUkkpOwotICAgIGV4cGxpY2l0IFF1YWxp ZmllZE5hbWUoV1RGOjpIYXNoVGFibGVEZWxldGVkVmFsdWVUeXBlKSA6IG1faW1wbChoYXNoVGFi bGVEZWxldGVkVmFsdWUoKSkgeyB9Ci0gICAgYm9vbCBpc0hhc2hUYWJsZURlbGV0ZWRWYWx1ZSgp IGNvbnN0IHsgcmV0dXJuIG1faW1wbCA9PSBoYXNoVGFibGVEZWxldGVkVmFsdWUoKTsgfQotICAg IH5RdWFsaWZpZWROYW1lKCk7CisgICAgZXhwbGljaXQgUXVhbGlmaWVkTmFtZShXVEY6Okhhc2hU YWJsZURlbGV0ZWRWYWx1ZVR5cGUpIDogbV9pbXBsKFdURjo6SGFzaFRhYmxlRGVsZXRlZFZhbHVl KSB7IH0KKyAgICBib29sIGlzSGFzaFRhYmxlRGVsZXRlZFZhbHVlKCkgY29uc3QgeyByZXR1cm4g bV9pbXBsLmlzSGFzaFRhYmxlRGVsZXRlZFZhbHVlKCk7IH0KKyAgICB+UXVhbGlmaWVkTmFtZSgp IHsgfQogI2lmZGVmIFFOQU1FX0RFRkFVTFRfQ09OU1RSVUNUT1IKICAgICBRdWFsaWZpZWROYW1l KCkgOiBtX2ltcGwoMCkgeyB9CiAjZW5kaWYKIAotICAgIFF1YWxpZmllZE5hbWUoY29uc3QgUXVh bGlmaWVkTmFtZSYgb3RoZXIpIDogbV9pbXBsKG90aGVyLm1faW1wbCkgeyByZWYoKTsgfQotICAg IGNvbnN0IFF1YWxpZmllZE5hbWUmIG9wZXJhdG9yPShjb25zdCBRdWFsaWZpZWROYW1lJiBvdGhl cikgeyBvdGhlci5yZWYoKTsgZGVyZWYoKTsgbV9pbXBsID0gb3RoZXIubV9pbXBsOyByZXR1cm4g KnRoaXM7IH0KKyAgICBRdWFsaWZpZWROYW1lKGNvbnN0IFF1YWxpZmllZE5hbWUmIG90aGVyKSA6 IG1faW1wbChvdGhlci5tX2ltcGwpIHsgfQorICAgIGNvbnN0IFF1YWxpZmllZE5hbWUmIG9wZXJh dG9yPShjb25zdCBRdWFsaWZpZWROYW1lJiBvdGhlcikgeyBtX2ltcGwgPSBvdGhlci5tX2ltcGw7 IHJldHVybiAqdGhpczsgfQogCiAgICAgYm9vbCBvcGVyYXRvcj09KGNvbnN0IFF1YWxpZmllZE5h bWUmIG90aGVyKSBjb25zdCB7IHJldHVybiBtX2ltcGwgPT0gb3RoZXIubV9pbXBsOyB9CiAgICAg Ym9vbCBvcGVyYXRvciE9KGNvbnN0IFF1YWxpZmllZE5hbWUmIG90aGVyKSBjb25zdCB7IHJldHVy biAhKCp0aGlzID09IG90aGVyKTsgfQpAQCAtMTAwLDcgKzEwMCw3IEBAIHB1YmxpYzoKIAogICAg IFN0cmluZyB0b1N0cmluZygpIGNvbnN0OwogCi0gICAgUXVhbGlmaWVkTmFtZUltcGwqIGltcGwo KSBjb25zdCB7IHJldHVybiBtX2ltcGw7IH0KKyAgICBRdWFsaWZpZWROYW1lSW1wbCogaW1wbCgp IGNvbnN0IHsgcmV0dXJuIG1faW1wbC5nZXQoKTsgfQogI2lmIEVOQUJMRShDU1NfU0VMRUNUT1Jf SklUKQogICAgIHN0YXRpYyBwdHJkaWZmX3QgaW1wbE1lbW9yeU9mZnNldCgpIHsgcmV0dXJuIE9C SkVDVF9PRkZTRVRPRihRdWFsaWZpZWROYW1lLCBtX2ltcGwpOyB9CiAjZW5kaWYgLy8gRU5BQkxF KENTU19TRUxFQ1RPUl9KSVQpCkBAIC0xMDksMTIgKzEwOSw5IEBAIHB1YmxpYzoKICAgICBzdGF0 aWMgdm9pZCBpbml0KCk7CiAKIHByaXZhdGU6Ci0gICAgdm9pZCByZWYoKSBjb25zdCB7IG1faW1w bC0+cmVmKCk7IH0KLSAgICB2b2lkIGRlcmVmKCk7Ci0KICAgICBzdGF0aWMgUXVhbGlmaWVkTmFt ZUltcGwqIGhhc2hUYWJsZURlbGV0ZWRWYWx1ZSgpIHsgcmV0dXJuIFJlZlB0cjxRdWFsaWZpZWRO YW1lSW1wbD46Omhhc2hUYWJsZURlbGV0ZWRWYWx1ZSgpOyB9CiAgICAgCi0gICAgUXVhbGlmaWVk TmFtZUltcGwqIG1faW1wbDsKKyAgICBSZWZQdHI8UXVhbGlmaWVkTmFtZUltcGw+IG1faW1wbDsK IH07CiAKICNpZm5kZWYgV0VCQ09SRV9RVUFMSUZJRUROQU1FX0hJREVfR0xPQkFMUwo=