164636 2016-11-11 01:46:16 -0800 Warning added in r208542 2016-11-15 22:51:58 -0800 1 1 1 Unclassified WebKit WebCore Misc. WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 alex alex ap commit-queue eric.carlson jer.noble mcatanzaro youennf oldest_to_newest 1250205 0 alex 2016-11-11 01:46:16 -0800 We have to call the base class explicetely in the copy constructors. 1250207 1 294485 alex 2016-11-11 01:50:33 -0800 Created attachment 294485 Patch 1250241 2 294485 youennf 2016-11-11 08:43:14 -0800 Comment on attachment 294485 Patch Change is fine I guess but it seems a bit strange to have a copy constructor of a refcounted class. It might be better to remove the copy constructor here. Instead, we could have a create() that would take the four parameters (source, track, and the booleans) with default values if needed. Then clone would use create() instead of the copy constructor. 1250272 3 ap 2016-11-11 10:08:12 -0800 I agree, this code smells wrong. When is the copy constructor used? 1250314 4 eric.carlson 2016-11-11 11:36:37 -0800 (In reply to comment #2) > Comment on attachment 294485 [details] > Patch > > Change is fine I guess but it seems a bit strange to have a copy constructor > of a refcounted class. > > It might be better to remove the copy constructor here. > Instead, we could have a create() that would take the four parameters > (source, track, and the booleans) with default values if needed. > Then clone would use create() instead of the copy constructor. I agree, Youenn's suggestion makes much more sense. 1250936 5 alex 2016-11-13 22:53:23 -0800 *** Bug 164688 has been marked as a duplicate of this bug. *** 1250946 6 alex 2016-11-14 01:14:46 -0800 (In reply to comment #4) > (In reply to comment #2) > > Comment on attachment 294485 [details] > > Patch > > > > Change is fine I guess but it seems a bit strange to have a copy constructor > > of a refcounted class. > > > > It might be better to remove the copy constructor here. > > Instead, we could have a create() that would take the four parameters > > (source, track, and the booleans) with default values if needed. > > Then clone would use create() instead of the copy constructor. > > I agree, Youenn's suggestion makes much more sense. Right, as ap was saying I'm not sure if this is even used, just fixing the refactoring problem added, I seemed fishy that is why I wanted the review for fixing the warning. I'll check it in more detail. Thanks for the review. 1251421 7 294832 alex 2016-11-15 03:47:57 -0800 Created attachment 294832 Patch 1251815 8 294832 commit-queue 2016-11-15 22:51:54 -0800 Comment on attachment 294832 Patch Clearing flags on attachment: 294832 Committed r208786: <http://trac.webkit.org/changeset/208786> 1251816 9 commit-queue 2016-11-15 22:51:58 -0800 All reviewed patches have been landed. Closing bug. 294485 2016-11-11 01:50:33 -0800 2016-11-15 03:47:49 -0800 Patch bug-164636-20161111104618.patch text/plain 1585 alex U3VidmVyc2lvbiBSZXZpc2lvbjogMjA4NTg0CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggODQzZjBkNDVjMWI2MzQ0 ZjdlZjRlMDllYmZjOTY2NjkwMGVhNTIzNi4uOTVkMzI0ODk3MDFmMTc1Yjg1MTVjZjc5MzQ1N2Ez MGFkY2YxZThjNSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE1IEBACisyMDE2LTExLTExICBBbGVq YW5kcm8gRy4gQ2FzdHJvICA8YWxleEBpZ2FsaWEuY29tPgorCisgICAgICAgIFdhcm5pbmcgYWRk ZWQgaW4gcjIwODU0MgorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5j Z2k/aWQ9MTY0NjM2CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAg ICAgICAgV2UgaGF2ZSB0byBjYWxsIHRoZSBiYXNlIGNsYXNzIGV4cGxpY2V0ZWx5IGluIHRoZSBj b3B5IGNvbnN0cnVjdG9ycy4KKworICAgICAgICAqIHBsYXRmb3JtL21lZGlhc3RyZWFtL01lZGlh U3RyZWFtVHJhY2tQcml2YXRlLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6Ok1lZGlhU3RyZWFtVHJh Y2tQcml2YXRlOjpNZWRpYVN0cmVhbVRyYWNrUHJpdmF0ZSk6CisKIDIwMTYtMTEtMTAgIFl1c3Vr ZSBTdXp1a2kgIDx1dGF0YW5lLnRlYUBnbWFpbC5jb20+CiAKICAgICAgICAgW0RPTUpJVF0gRE9N SklUIGFjY2Vzc29yIGF0dHJpYnV0ZSBpbiBJREwgc2hvdWxkIHNheSBsaWtlIERPTUpJVD1HZXR0 ZXIKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL21lZGlhc3RyZWFtL01lZGlh U3RyZWFtVHJhY2tQcml2YXRlLmNwcCBiL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL21lZGlhc3Ry ZWFtL01lZGlhU3RyZWFtVHJhY2tQcml2YXRlLmNwcAppbmRleCBiMjIyYmVkMjM3NjNkOTg3OWEw NDY3NmVkYmJhYTU4ODliZjNkZGFhLi41ZmZhZWRjOTM4YzUyNmYxMjZlMzU0Zjk4YWJhMWRhNDg4 ZWUzNTA1IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9tZWRpYXN0cmVhbS9N ZWRpYVN0cmVhbVRyYWNrUHJpdmF0ZS5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0v bWVkaWFzdHJlYW0vTWVkaWFTdHJlYW1UcmFja1ByaXZhdGUuY3BwCkBAIC00OCw3ICs0OCw4IEBA IFJlZjxNZWRpYVN0cmVhbVRyYWNrUHJpdmF0ZT4gTWVkaWFTdHJlYW1UcmFja1ByaXZhdGU6OmNy ZWF0ZShSZWY8UmVhbHRpbWVNZWRpYVNvCiB9CiAKIE1lZGlhU3RyZWFtVHJhY2tQcml2YXRlOjpN ZWRpYVN0cmVhbVRyYWNrUHJpdmF0ZShjb25zdCBNZWRpYVN0cmVhbVRyYWNrUHJpdmF0ZSYgb3Ro ZXIpCi0gICAgOiBtX3NvdXJjZShvdGhlci5tX3NvdXJjZS5jb3B5UmVmKCkpCisgICAgOiBSZWZD b3VudGVkKCkKKyAgICAsIG1fc291cmNlKG90aGVyLm1fc291cmNlLmNvcHlSZWYoKSkKICAgICAs IG1faWQoY3JlYXRlQ2Fub25pY2FsVVVJRFN0cmluZygpKQogICAgICwgbV9pc0VuYWJsZWQob3Ro ZXIuZW5hYmxlZCgpKQogICAgICwgbV9pc0VuZGVkKG90aGVyLmVuZGVkKCkpCg== 294832 2016-11-15 03:47:57 -0800 2016-11-15 22:51:54 -0800 Patch bug-164636-20161115124754.patch text/plain 3270 alex U3VidmVyc2lvbiBSZXZpc2lvbjogMjA4NzI4CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggZjczOTZiZWM3N2MyYmQw ZTQ1NzQ0YzcxZTNmYjEzZTJhN2E0NjY5MC4uZjIzMWIzN2JmMGJiNTVkMTdkMDExODdiMTVmZWUw YmRmOWEwMjcwZiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIwIEBACisyMDE2LTExLTE1ICBBbGVq YW5kcm8gRy4gQ2FzdHJvICA8YWxleEBpZ2FsaWEuY29tPgorCisgICAgICAgIFdhcm5pbmcgYWRk ZWQgaW4gcjIwODU0MgorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5j Z2k/aWQ9MTY0NjM2CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAg ICAgICAgRGVsZXRlZCB0aGUgY29weSBjb25zdHJ1Y3Rvciwgd2Ugbm93IGV4cGxpY2l0bHkgY29w eSB0aGUgb2JqZWN0IGluCisgICAgICAgIHRoZSBjbG9uZSBtZXRob2QuCisKKyAgICAgICAgKiBw bGF0Zm9ybS9tZWRpYXN0cmVhbS9NZWRpYVN0cmVhbVRyYWNrUHJpdmF0ZS5jcHA6IERlbGV0ZSB0 aGUKKyAgICAgICAgY29weSBjb25zdHJ1Y3Rvci4KKyAgICAgICAgKFdlYkNvcmU6Ok1lZGlhU3Ry ZWFtVHJhY2tQcml2YXRlOjpjbG9uZSk6IENvcHkgdGhlIHJlbGV2YW50CisgICAgICAgIGluZm9y bWF0aW9uIGZvciB0aGUgY2xvbmUgb3BlcmF0aW9uIGluIHRoZSBBUEkuCisgICAgICAgICogcGxh dGZvcm0vbWVkaWFzdHJlYW0vTWVkaWFTdHJlYW1UcmFja1ByaXZhdGUuaDogRGVsZXRlIHRoZSBj b3B5CisgICAgICAgIGNvbnN0cnVjdG9yLgorCiAyMDE2LTExLTE0ICBBbGV4IENocmlzdGVuc2Vu ICA8YWNocmlzdGVuc2VuQHdlYmtpdC5vcmc+CiAKICAgICAgICAgTW92ZSBTZWN1cml0eU9yaWdp bjo6Y3JlYXRlRnJvbURhdGFiYXNlSWRlbnRpZmllciB0byBTZWN1cml0eU9yaWdpbkRhdGEKZGlm ZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL21lZGlhc3RyZWFtL01lZGlhU3RyZWFt VHJhY2tQcml2YXRlLmNwcCBiL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL21lZGlhc3RyZWFtL01l ZGlhU3RyZWFtVHJhY2tQcml2YXRlLmNwcAppbmRleCBiMjIyYmVkMjM3NjNkOTg3OWEwNDY3NmVk YmJhYTU4ODliZjNkZGFhLi4zZjZlMGUyMWI3ZTcwYjA4MTQwYjlmMGZkM2M2ZGU1YTFhNDhmOGZk IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9tZWRpYXN0cmVhbS9NZWRpYVN0 cmVhbVRyYWNrUHJpdmF0ZS5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vbWVkaWFz dHJlYW0vTWVkaWFTdHJlYW1UcmFja1ByaXZhdGUuY3BwCkBAIC00NywxNSArNDcsNiBAQCBSZWY8 TWVkaWFTdHJlYW1UcmFja1ByaXZhdGU+IE1lZGlhU3RyZWFtVHJhY2tQcml2YXRlOjpjcmVhdGUo UmVmPFJlYWx0aW1lTWVkaWFTbwogICAgIHJldHVybiBhZG9wdFJlZigqbmV3IE1lZGlhU3RyZWFt VHJhY2tQcml2YXRlKFdURk1vdmUoc291cmNlKSwgV1RGTW92ZShpZCkpKTsKIH0KIAotTWVkaWFT dHJlYW1UcmFja1ByaXZhdGU6Ok1lZGlhU3RyZWFtVHJhY2tQcml2YXRlKGNvbnN0IE1lZGlhU3Ry ZWFtVHJhY2tQcml2YXRlJiBvdGhlcikKLSAgICA6IG1fc291cmNlKG90aGVyLm1fc291cmNlLmNv cHlSZWYoKSkKLSAgICAsIG1faWQoY3JlYXRlQ2Fub25pY2FsVVVJRFN0cmluZygpKQotICAgICwg bV9pc0VuYWJsZWQob3RoZXIuZW5hYmxlZCgpKQotICAgICwgbV9pc0VuZGVkKG90aGVyLmVuZGVk KCkpCi17Ci0gICAgbV9zb3VyY2UtPmFkZE9ic2VydmVyKHRoaXMpOwotfQotCiBNZWRpYVN0cmVh bVRyYWNrUHJpdmF0ZTo6TWVkaWFTdHJlYW1UcmFja1ByaXZhdGUoUmVmPFJlYWx0aW1lTWVkaWFT b3VyY2U+JiYgc291cmNlLCBTdHJpbmcmJiBpZCkKICAgICA6IG1fc291cmNlKFdURk1vdmUoc291 cmNlKSkKICAgICAsIG1faWQoV1RGTW92ZShpZCkpCkBAIC0xMzIsNyArMTIzLDEyIEBAIHZvaWQg TWVkaWFTdHJlYW1UcmFja1ByaXZhdGU6OmVuZFRyYWNrKCkKIAogUmVmPE1lZGlhU3RyZWFtVHJh Y2tQcml2YXRlPiBNZWRpYVN0cmVhbVRyYWNrUHJpdmF0ZTo6Y2xvbmUoKQogewotICAgIHJldHVy biBhZG9wdFJlZigqbmV3IE1lZGlhU3RyZWFtVHJhY2tQcml2YXRlKCp0aGlzKSk7CisgICAgYXV0 byBjbG9uZWRNZWRpYVN0cmVhbVRyYWNrUHJpdmF0ZSA9IGNyZWF0ZShtX3NvdXJjZS5jb3B5UmVm KCkpOworICAgIGNsb25lZE1lZGlhU3RyZWFtVHJhY2tQcml2YXRlLT5tX2lzRW5hYmxlZCA9IHRo aXMtPm1faXNFbmFibGVkOworICAgIGNsb25lZE1lZGlhU3RyZWFtVHJhY2tQcml2YXRlLT5tX2lz RW5kZWQgPSB0aGlzLT5tX2lzRW5kZWQ7CisgICAgY2xvbmVkTWVkaWFTdHJlYW1UcmFja1ByaXZh dGUtPm1fY29uc3RyYWludHMgPSB0aGlzLT5tX2NvbnN0cmFpbnRzOworCisgICAgcmV0dXJuIGNs b25lZE1lZGlhU3RyZWFtVHJhY2tQcml2YXRlOwogfQogCiBSZWFsdGltZU1lZGlhU291cmNlOjpU eXBlIE1lZGlhU3RyZWFtVHJhY2tQcml2YXRlOjp0eXBlKCkgY29uc3QKZGlmZiAtLWdpdCBhL1Nv dXJjZS9XZWJDb3JlL3BsYXRmb3JtL21lZGlhc3RyZWFtL01lZGlhU3RyZWFtVHJhY2tQcml2YXRl LmggYi9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9tZWRpYXN0cmVhbS9NZWRpYVN0cmVhbVRyYWNr UHJpdmF0ZS5oCmluZGV4IDVmNDJiNDA3OTY3M2U2ZjA3YjRiOTYwYjU3NGVjMjJkNThiYmM3ZjYu LmFiMmNiMzkzZjM1NTNkNGEwYTAxMzNmNDgxMTc3YjEzZmZkMTVmMTMgMTAwNjQ0Ci0tLSBhL1Nv dXJjZS9XZWJDb3JlL3BsYXRmb3JtL21lZGlhc3RyZWFtL01lZGlhU3RyZWFtVHJhY2tQcml2YXRl LmgKKysrIGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vbWVkaWFzdHJlYW0vTWVkaWFTdHJlYW1U cmFja1ByaXZhdGUuaApAQCAtOTgsNyArOTgsNiBAQCBwdWJsaWM6CiAgICAgdm9pZCBwYWludEN1 cnJlbnRGcmFtZUluQ29udGV4dChHcmFwaGljc0NvbnRleHQmLCBjb25zdCBGbG9hdFJlY3QmKTsK IAogcHJpdmF0ZToKLSAgICBleHBsaWNpdCBNZWRpYVN0cmVhbVRyYWNrUHJpdmF0ZShjb25zdCBN ZWRpYVN0cmVhbVRyYWNrUHJpdmF0ZSYpOwogICAgIE1lZGlhU3RyZWFtVHJhY2tQcml2YXRlKFJl ZjxSZWFsdGltZU1lZGlhU291cmNlPiYmLCBTdHJpbmcmJiBpZCk7CiAKICAgICAvLyBSZWFsdGlt ZU1lZGlhU291cmNlT2JzZXJ2ZXIK