32363 2009-12-10 00:28:52 -0800 [Chromium] Disallow assignment of WebVector<T> to WebVector<T> 2009-12-17 09:07:00 -0800 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 1 yurys yurys dglazkov eric fishd webkit.review.bot oldest_to_newest 170352 0 yurys 2009-12-10 00:28:52 -0800 As comment in the top part of WebVector.h says WebVector::swap should be used in such cases. If you write by mistake something as follows: WebVector<WebString> a(3); WebVector<WebString> b; b = a; operator= automatically generated by compiler will be invoked as it takes precedence over template version template <typename C> WebVector<T>& operator=(const C& other) The default assignment operator will lead to a.m_ptr == b.m_ptr and to destructor being called twice for the container elements. So either operator= should have valid implementation or be disallowed explicitly. 170357 1 44598 yurys 2009-12-10 00:33:03 -0800 Created attachment 44598 patch Not sure where I could place unit test for this. 170360 2 webkit.review.bot 2009-12-10 00:36:28 -0800 style-queue ran check-webkit-style on attachment 44598 without any errors. 170363 3 44598 eric 2009-12-10 00:57:29 -0800 Comment on attachment 44598 patch We have wtf/Noncopyable for this sort of thing. But that can't be used here, so this is probably fine. 170808 4 44598 fishd 2009-12-11 10:12:16 -0800 Comment on attachment 44598 patch Actually, the intent was to allow copying just as you can copy a std::vector or a WTF::Vector. I think we should implement this assignment operator. 171409 5 44797 yurys 2009-12-14 08:06:39 -0800 Created attachment 44797 patch addressing reviewers comments 171410 6 webkit.review.bot 2009-12-14 08:10:44 -0800 style-queue ran check-webkit-style on attachment 44797 without any errors. 171413 7 yurys 2009-12-14 08:14:21 -0800 (In reply to comment #4) > (From update of attachment 44598 [details]) > Actually, the intent was to allow copying just as you can copy a std::vector or > a WTF::Vector. I think we should implement this assignment operator. Done. I've added "non-template" operator=. 172802 8 yurys 2009-12-17 09:07:00 -0800 Committing to http://svn.webkit.org/repository/webkit/trunk ... M WebKit/chromium/ChangeLog M WebKit/chromium/public/WebVector.h Committed r52255 44598 2009-12-10 00:33:03 -0800 2009-12-14 08:06:39 -0800 patch webvector.patch text/plain 947 yurys ZGlmZiAtLWdpdCBhL1dlYktpdC9jaHJvbWl1bS9DaGFuZ2VMb2cgYi9XZWJLaXQvY2hyb21pdW0v Q2hhbmdlTG9nCmluZGV4IDcwZTk2YzEuLjUxYmZjMjMgMTAwNjQ0Ci0tLSBhL1dlYktpdC9jaHJv bWl1bS9DaGFuZ2VMb2cKKysrIGIvV2ViS2l0L2Nocm9taXVtL0NoYW5nZUxvZwpAQCAtMSwzICsx LDEzIEBACisyMDA5LTEyLTEwICBZdXJ5IFNlbWlraGF0c2t5ICA8eXVyeXNAY2hyb21pdW0ub3Jn PgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIERpc2Fs bG93IGFzc2lnbmluZyBXZWJWZWN0b3IgdG8gV2ViVmVjdG9yIGluc3RhbnRpYXRlZCB3aXRoIHRo ZSBzYW1lIHR5cGUuCisKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcu Y2dpP2lkPTMyMzYzCisKKyAgICAgICAgKiBwdWJsaWMvV2ViVmVjdG9yLmg6CisKIDIwMDktMTIt MDkgIERhcmluIEZpc2hlciAgPGRhcmluQGNocm9taXVtLm9yZz4KIAogICAgICAgICBSZXZpZXdl ZCBieSBFcmljIFNlaWRlbC4KZGlmZiAtLWdpdCBhL1dlYktpdC9jaHJvbWl1bS9wdWJsaWMvV2Vi VmVjdG9yLmggYi9XZWJLaXQvY2hyb21pdW0vcHVibGljL1dlYlZlY3Rvci5oCmluZGV4IGQxZjk5 YjQuLjY3ZWU5Y2MgMTAwNjQ0Ci0tLSBhL1dlYktpdC9jaHJvbWl1bS9wdWJsaWMvV2ViVmVjdG9y LmgKKysrIGIvV2ViS2l0L2Nocm9taXVtL3B1YmxpYy9XZWJWZWN0b3IuaApAQCAtMTU1LDYgKzE1 NSw5IEBAIHByaXZhdGU6CiAKICAgICBUKiBtX3B0cjsKICAgICBzaXplX3QgbV9zaXplOworCisg ICAgLy8gRGlzYWxsb3cgYXNzaWdubWVudCBmcm9tIHRoZSBzYW1lIHR5cGUgY29udGFpbmVyLiBV c2Ugc3dhcCBpbnN0ZWFkLgorICAgIFdlYlZlY3RvciYgb3BlcmF0b3I9KGNvbnN0IFdlYlZlY3Rv ciYpOwogfTsKIAogfSAgLy8gbmFtZXNwYWNlIFdlYktpdAo= 44797 2009-12-14 08:06:39 -0800 2009-12-17 07:53:25 -0800 patch addressing reviewers comments webvector2.patch text/plain 1057 yurys ZGlmZiAtLWdpdCBhL1dlYktpdC9jaHJvbWl1bS9DaGFuZ2VMb2cgYi9XZWJLaXQvY2hyb21pdW0v Q2hhbmdlTG9nCmluZGV4IDcwZTk2YzEuLjA3NDU1ZmYgMTAwNjQ0Ci0tLSBhL1dlYktpdC9jaHJv bWl1bS9DaGFuZ2VMb2cKKysrIGIvV2ViS2l0L2Nocm9taXVtL0NoYW5nZUxvZwpAQCAtMSwzICsx LDEzIEBACisyMDA5LTEyLTEwICBZdXJ5IFNlbWlraGF0c2t5ICA8eXVyeXNAY2hyb21pdW0ub3Jn PgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFByb3Zp ZGUgZXhwbGljaXQgaW1wbGVtZW50YXRpb24gb2YgJ25vbi10ZW1wbGF0ZScgV2ViVmVjdG9yPFQ+ OjpvcGVyYXRvcj0KKworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5j Z2k/aWQ9MzIzNjMKKworICAgICAgICAqIHB1YmxpYy9XZWJWZWN0b3IuaDoKKwogMjAwOS0xMi0w OSAgRGFyaW4gRmlzaGVyICA8ZGFyaW5AY2hyb21pdW0ub3JnPgogCiAgICAgICAgIFJldmlld2Vk IGJ5IEVyaWMgU2VpZGVsLgpkaWZmIC0tZ2l0IGEvV2ViS2l0L2Nocm9taXVtL3B1YmxpYy9XZWJW ZWN0b3IuaCBiL1dlYktpdC9jaHJvbWl1bS9wdWJsaWMvV2ViVmVjdG9yLmgKaW5kZXggZDFmOTli NC4uMDUyMDg5NSAxMDA2NDQKLS0tIGEvV2ViS2l0L2Nocm9taXVtL3B1YmxpYy9XZWJWZWN0b3Iu aAorKysgYi9XZWJLaXQvY2hyb21pdW0vcHVibGljL1dlYlZlY3Rvci5oCkBAIC04NCw2ICs4NCwx MyBAQCBwdWJsaWM6CiAgICAgICAgIGluaXRpYWxpemVGcm9tKG90aGVyLnNpemUoKSA/ICZvdGhl clswXSA6IDAsIG90aGVyLnNpemUoKSk7CiAgICAgfQogCisgICAgV2ViVmVjdG9yJiBvcGVyYXRv cj0oY29uc3QgV2ViVmVjdG9yJiBvdGhlcikKKyAgICB7CisgICAgICAgIGlmICh0aGlzICE9ICZv dGhlcikKKyAgICAgICAgICAgIGFzc2lnbihvdGhlcik7CisgICAgICAgIHJldHVybiAqdGhpczsK KyAgICB9CisKICAgICB0ZW1wbGF0ZSA8dHlwZW5hbWUgQz4KICAgICBXZWJWZWN0b3I8VD4mIG9w ZXJhdG9yPShjb25zdCBDJiBvdGhlcikKICAgICB7Cg==