155470 2016-03-14 15:11:49 -0700 [GTK] Remove duplicate HashMap traversal and unneeded reference count churn in DataObjectGtk::forClipboard 2016-03-15 07:21:41 -0700 1 1 1 Unclassified WebKit WebKitGTK WebKit Local Build Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 jh718.park jh718.park bugs-noreply commit-queue oldest_to_newest 1174737 0 jh718.park 2016-03-14 15:11:49 -0700 Remove duplicate HashMap traversal and unneeded reference count churn in DataObjectGtk::forClipboard. 1174959 1 274088 jh718.park 2016-03-15 05:25:08 -0700 Created attachment 274088 Patch 1174962 2 274090 jh718.park 2016-03-15 05:39:19 -0700 Created attachment 274090 Patch 1174963 3 274090 cgarcia 2016-03-15 05:56:51 -0700 Comment on attachment 274090 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=274090&action=review > Source/WebCore/platform/gtk/DataObjectGtk.cpp:163 > + auto it = objectMap.find(clipboard); > > - if (!objectMap.contains(clipboard)) { > - Ref<DataObjectGtk> dataObject = DataObjectGtk::create(); > - objectMap.set(clipboard, dataObject.copyRef()); > - return dataObject.ptr(); > + if (it == objectMap.end()) { > + auto dataObject = DataObjectGtk::create(); > + DataObjectGtk* dataObjectPtr = dataObject.ptr(); > + objectMap.set(clipboard, WTFMove(dataObject)); > + return dataObjectPtr; > } > > - HashMap<GtkClipboard*, RefPtr<DataObjectGtk> >::iterator it = objectMap.find(clipboard); > return it->value.get(); > } Or even better you could use HashMap::add and check the result. Something like this: auto addResult = objectMap.add(clipboard, nullptr); if (addResult.isNewEntry) addResult.iterator->value = DataObjectGtk::create(); return addResult.iterator->value.get(); 1174964 4 jh718.park 2016-03-15 06:16:22 -0700 (In reply to comment #3) > Comment on attachment 274090 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=274090&action=review > > > Source/WebCore/platform/gtk/DataObjectGtk.cpp:163 > > + auto it = objectMap.find(clipboard); > > > > - if (!objectMap.contains(clipboard)) { > > - Ref<DataObjectGtk> dataObject = DataObjectGtk::create(); > > - objectMap.set(clipboard, dataObject.copyRef()); > > - return dataObject.ptr(); > > + if (it == objectMap.end()) { > > + auto dataObject = DataObjectGtk::create(); > > + DataObjectGtk* dataObjectPtr = dataObject.ptr(); > > + objectMap.set(clipboard, WTFMove(dataObject)); > > + return dataObjectPtr; > > } > > > > - HashMap<GtkClipboard*, RefPtr<DataObjectGtk> >::iterator it = objectMap.find(clipboard); > > return it->value.get(); > > } > > Or even better you could use HashMap::add and check the result. Something > like this: > > auto addResult = objectMap.add(clipboard, nullptr); > if (addResult.isNewEntry) > addResult.iterator->value = DataObjectGtk::create(); > return addResult.iterator->value.get(); Indeed it is. Thank you for your comment. I will change this as you wrote. 1174965 5 274091 jh718.park 2016-03-15 06:28:22 -0700 Created attachment 274091 Patch 1174970 6 274091 commit-queue 2016-03-15 07:21:36 -0700 Comment on attachment 274091 Patch Clearing flags on attachment: 274091 Committed r198209: <http://trac.webkit.org/changeset/198209> 1174971 7 commit-queue 2016-03-15 07:21:41 -0700 All reviewed patches have been landed. Closing bug. 274088 2016-03-15 05:25:08 -0700 2016-03-15 05:39:12 -0700 Patch bug-155470-20160315212455.patch text/plain 1918 jh718.park U3VidmVyc2lvbiBSZXZpc2lvbjogMTk4MjA4CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggZTI3NjhkY2VjZDRhMzI1 YjZmNGNjNjJhNWEzZjA5OTIwNmU0NTUyZC4uMzVlYWYwYWExYzlmMzBjYmFhNWUyY2VlNDI0YmNm NWM3ZWU0YWNhNiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE1IEBACisyMDE2LTAzLTE1ICBKb29u Z2h1biBQYXJrICA8amg3MTgucGFya0BzYW1zdW5nLmNvbT4KKworICAgICAgICBbR1RLXSBSZW1v dmUgZHVwbGljYXRlIEhhc2hNYXAgdHJhdmVyc2FsIGFuZCB1bm5lZWRlZCByZWZlcmVuY2UgY291 bnQgY2h1cm4gaW4gRGF0YU9iamVjdEd0azo6Zm9yQ2xpcGJvYXJkCisgICAgICAgIGh0dHBzOi8v YnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xNTU0NzAKKworICAgICAgICBSZXZpZXdl ZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBObyBuZXcgdGVzdHMsIG5vIG5ldyBiZWhh dmlvdXJzLgorCisgICAgICAgICogcGxhdGZvcm0vZ3RrL0RhdGFPYmplY3RHdGsuY3BwOgorICAg ICAgICAoV2ViQ29yZTo6RGF0YU9iamVjdEd0azo6Zm9yQ2xpcGJvYXJkKToKKwogMjAxNi0wMy0x NSAgTWFudWVsIFJlZ28gQ2FzYXNub3ZhcyAgPHJlZ29AaWdhbGlhLmNvbT4KIAogICAgICAgICBb Y3NzLWdyaWRdIFJlbmFtZSBHcmlkUmVzb2x2ZWRQb3NpdGlvbiB0byBHcmlkUG9zaXRpb25zUmVz b2x2ZXIKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2d0ay9EYXRhT2JqZWN0 R3RrLmNwcCBiL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2d0ay9EYXRhT2JqZWN0R3RrLmNwcApp bmRleCAwNjk4YWJlMTRhNWM5MmZlOGU2MDFmM2I4ZWFiMzQxYWUzNzM2N2QzLi4yNzQwZDIwNTFj OWYwNmRmNzQwODQ1MDI5MmFkNmRkOTM2M2QxMzBlIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9wbGF0Zm9ybS9ndGsvRGF0YU9iamVjdEd0ay5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvcGxh dGZvcm0vZ3RrL0RhdGFPYmplY3RHdGsuY3BwCkBAIC0xNTAsMTQgKzE1MCwxNSBAQCB2b2lkIERh dGFPYmplY3RHdGs6OmNsZWFyQWxsKCkKIERhdGFPYmplY3RHdGsqIERhdGFPYmplY3RHdGs6OmZv ckNsaXBib2FyZChHdGtDbGlwYm9hcmQqIGNsaXBib2FyZCkKIHsKICAgICBzdGF0aWMgSGFzaE1h cDxHdGtDbGlwYm9hcmQqLCBSZWZQdHI8RGF0YU9iamVjdEd0az4gPiBvYmplY3RNYXA7CisgICAg SGFzaE1hcDxHdGtDbGlwYm9hcmQqLCBSZWZQdHI8RGF0YU9iamVjdEd0az4gPjo6aXRlcmF0b3Ig aXQgPSBvYmplY3RNYXAuZmluZChjbGlwYm9hcmQpOwogCi0gICAgaWYgKCFvYmplY3RNYXAuY29u dGFpbnMoY2xpcGJvYXJkKSkgeworICAgIGlmIChpdCA9PSBvYmplY3RNYXAuZW5kKCkpIHsKICAg ICAgICAgUmVmPERhdGFPYmplY3RHdGs+IGRhdGFPYmplY3QgPSBEYXRhT2JqZWN0R3RrOjpjcmVh dGUoKTsKLSAgICAgICAgb2JqZWN0TWFwLnNldChjbGlwYm9hcmQsIGRhdGFPYmplY3QuY29weVJl ZigpKTsKLSAgICAgICAgcmV0dXJuIGRhdGFPYmplY3QucHRyKCk7CisgICAgICAgIERhdGFPYmpl Y3RHdGsqIGRhdGFPYmplY3RQdHIgPSBkYXRhT2JqZWN0LnB0cigpOworICAgICAgICBvYmplY3RN YXAuc2V0KGNsaXBib2FyZCwgV1RGTW92ZShkYXRhT2JqZWN0KSk7CisgICAgICAgIHJldHVybiBk YXRhT2JqZWN0UHRyOwogICAgIH0KIAotICAgIEhhc2hNYXA8R3RrQ2xpcGJvYXJkKiwgUmVmUHRy PERhdGFPYmplY3RHdGs+ID46Oml0ZXJhdG9yIGl0ID0gb2JqZWN0TWFwLmZpbmQoY2xpcGJvYXJk KTsKICAgICByZXR1cm4gaXQtPnZhbHVlLmdldCgpOwogfQogCg== 274090 2016-03-15 05:39:19 -0700 2016-03-15 06:28:15 -0700 Patch bug-155470-20160315213906.patch text/plain 1918 jh718.park U3VidmVyc2lvbiBSZXZpc2lvbjogMTk4MjA4CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggZTI3NjhkY2VjZDRhMzI1 YjZmNGNjNjJhNWEzZjA5OTIwNmU0NTUyZC4uMzVlYWYwYWExYzlmMzBjYmFhNWUyY2VlNDI0YmNm NWM3ZWU0YWNhNiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE1IEBACisyMDE2LTAzLTE1ICBKb29u Z2h1biBQYXJrICA8amg3MTgucGFya0BzYW1zdW5nLmNvbT4KKworICAgICAgICBbR1RLXSBSZW1v dmUgZHVwbGljYXRlIEhhc2hNYXAgdHJhdmVyc2FsIGFuZCB1bm5lZWRlZCByZWZlcmVuY2UgY291 bnQgY2h1cm4gaW4gRGF0YU9iamVjdEd0azo6Zm9yQ2xpcGJvYXJkCisgICAgICAgIGh0dHBzOi8v YnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xNTU0NzAKKworICAgICAgICBSZXZpZXdl ZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBObyBuZXcgdGVzdHMsIG5vIG5ldyBiZWhh dmlvdXJzLgorCisgICAgICAgICogcGxhdGZvcm0vZ3RrL0RhdGFPYmplY3RHdGsuY3BwOgorICAg ICAgICAoV2ViQ29yZTo6RGF0YU9iamVjdEd0azo6Zm9yQ2xpcGJvYXJkKToKKwogMjAxNi0wMy0x NSAgTWFudWVsIFJlZ28gQ2FzYXNub3ZhcyAgPHJlZ29AaWdhbGlhLmNvbT4KIAogICAgICAgICBb Y3NzLWdyaWRdIFJlbmFtZSBHcmlkUmVzb2x2ZWRQb3NpdGlvbiB0byBHcmlkUG9zaXRpb25zUmVz b2x2ZXIKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2d0ay9EYXRhT2JqZWN0 R3RrLmNwcCBiL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2d0ay9EYXRhT2JqZWN0R3RrLmNwcApp bmRleCAwNjk4YWJlMTRhNWM5MmZlOGU2MDFmM2I4ZWFiMzQxYWUzNzM2N2QzLi44NWUxZWQ5Mjdh NzIzZmYyMjA0OWQyN2YxM2IwODFhNTJhOGE5MzU4IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9wbGF0Zm9ybS9ndGsvRGF0YU9iamVjdEd0ay5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvcGxh dGZvcm0vZ3RrL0RhdGFPYmplY3RHdGsuY3BwCkBAIC0xNTAsMTQgKzE1MCwxNSBAQCB2b2lkIERh dGFPYmplY3RHdGs6OmNsZWFyQWxsKCkKIERhdGFPYmplY3RHdGsqIERhdGFPYmplY3RHdGs6OmZv ckNsaXBib2FyZChHdGtDbGlwYm9hcmQqIGNsaXBib2FyZCkKIHsKICAgICBzdGF0aWMgSGFzaE1h cDxHdGtDbGlwYm9hcmQqLCBSZWZQdHI8RGF0YU9iamVjdEd0az4gPiBvYmplY3RNYXA7CisgICAg YXV0byBpdCA9IG9iamVjdE1hcC5maW5kKGNsaXBib2FyZCk7CiAKLSAgICBpZiAoIW9iamVjdE1h cC5jb250YWlucyhjbGlwYm9hcmQpKSB7Ci0gICAgICAgIFJlZjxEYXRhT2JqZWN0R3RrPiBkYXRh T2JqZWN0ID0gRGF0YU9iamVjdEd0azo6Y3JlYXRlKCk7Ci0gICAgICAgIG9iamVjdE1hcC5zZXQo Y2xpcGJvYXJkLCBkYXRhT2JqZWN0LmNvcHlSZWYoKSk7Ci0gICAgICAgIHJldHVybiBkYXRhT2Jq ZWN0LnB0cigpOworICAgIGlmIChpdCA9PSBvYmplY3RNYXAuZW5kKCkpIHsKKyAgICAgICAgYXV0 byBkYXRhT2JqZWN0ID0gRGF0YU9iamVjdEd0azo6Y3JlYXRlKCk7CisgICAgICAgIERhdGFPYmpl Y3RHdGsqIGRhdGFPYmplY3RQdHIgPSBkYXRhT2JqZWN0LnB0cigpOworICAgICAgICBvYmplY3RN YXAuc2V0KGNsaXBib2FyZCwgV1RGTW92ZShkYXRhT2JqZWN0KSk7CisgICAgICAgIHJldHVybiBk YXRhT2JqZWN0UHRyOwogICAgIH0KIAotICAgIEhhc2hNYXA8R3RrQ2xpcGJvYXJkKiwgUmVmUHRy PERhdGFPYmplY3RHdGs+ID46Oml0ZXJhdG9yIGl0ID0gb2JqZWN0TWFwLmZpbmQoY2xpcGJvYXJk KTsKICAgICByZXR1cm4gaXQtPnZhbHVlLmdldCgpOwogfQogCg== 274091 2016-03-15 06:28:22 -0700 2016-03-15 07:21:36 -0700 Patch bug-155470-20160315222810.patch text/plain 1811 jh718.park U3VidmVyc2lvbiBSZXZpc2lvbjogMTk4MjA4CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggZTI3NjhkY2VjZDRhMzI1 YjZmNGNjNjJhNWEzZjA5OTIwNmU0NTUyZC4uMzVlYWYwYWExYzlmMzBjYmFhNWUyY2VlNDI0YmNm NWM3ZWU0YWNhNiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE1IEBACisyMDE2LTAzLTE1ICBKb29u Z2h1biBQYXJrICA8amg3MTgucGFya0BzYW1zdW5nLmNvbT4KKworICAgICAgICBbR1RLXSBSZW1v dmUgZHVwbGljYXRlIEhhc2hNYXAgdHJhdmVyc2FsIGFuZCB1bm5lZWRlZCByZWZlcmVuY2UgY291 bnQgY2h1cm4gaW4gRGF0YU9iamVjdEd0azo6Zm9yQ2xpcGJvYXJkCisgICAgICAgIGh0dHBzOi8v YnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xNTU0NzAKKworICAgICAgICBSZXZpZXdl ZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBObyBuZXcgdGVzdHMsIG5vIG5ldyBiZWhh dmlvdXJzLgorCisgICAgICAgICogcGxhdGZvcm0vZ3RrL0RhdGFPYmplY3RHdGsuY3BwOgorICAg ICAgICAoV2ViQ29yZTo6RGF0YU9iamVjdEd0azo6Zm9yQ2xpcGJvYXJkKToKKwogMjAxNi0wMy0x NSAgTWFudWVsIFJlZ28gQ2FzYXNub3ZhcyAgPHJlZ29AaWdhbGlhLmNvbT4KIAogICAgICAgICBb Y3NzLWdyaWRdIFJlbmFtZSBHcmlkUmVzb2x2ZWRQb3NpdGlvbiB0byBHcmlkUG9zaXRpb25zUmVz b2x2ZXIKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2d0ay9EYXRhT2JqZWN0 R3RrLmNwcCBiL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2d0ay9EYXRhT2JqZWN0R3RrLmNwcApp bmRleCAwNjk4YWJlMTRhNWM5MmZlOGU2MDFmM2I4ZWFiMzQxYWUzNzM2N2QzLi44NzA5MjM1NmQ1 ZDZlZTkyMTNmYjk3NGQ5NmUyMWZkOGZjM2VmNTg1IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9wbGF0Zm9ybS9ndGsvRGF0YU9iamVjdEd0ay5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvcGxh dGZvcm0vZ3RrL0RhdGFPYmplY3RHdGsuY3BwCkBAIC0xNTEsMTQgKzE1MSwxMSBAQCBEYXRhT2Jq ZWN0R3RrKiBEYXRhT2JqZWN0R3RrOjpmb3JDbGlwYm9hcmQoR3RrQ2xpcGJvYXJkKiBjbGlwYm9h cmQpCiB7CiAgICAgc3RhdGljIEhhc2hNYXA8R3RrQ2xpcGJvYXJkKiwgUmVmUHRyPERhdGFPYmpl Y3RHdGs+ID4gb2JqZWN0TWFwOwogCi0gICAgaWYgKCFvYmplY3RNYXAuY29udGFpbnMoY2xpcGJv YXJkKSkgewotICAgICAgICBSZWY8RGF0YU9iamVjdEd0az4gZGF0YU9iamVjdCA9IERhdGFPYmpl Y3RHdGs6OmNyZWF0ZSgpOwotICAgICAgICBvYmplY3RNYXAuc2V0KGNsaXBib2FyZCwgZGF0YU9i amVjdC5jb3B5UmVmKCkpOwotICAgICAgICByZXR1cm4gZGF0YU9iamVjdC5wdHIoKTsKLSAgICB9 CisgICAgYXV0byBhZGRSZXN1bHQgPSBvYmplY3RNYXAuYWRkKGNsaXBib2FyZCwgbnVsbHB0cik7 CisgICAgaWYgKGFkZFJlc3VsdC5pc05ld0VudHJ5KQorICAgICAgICBhZGRSZXN1bHQuaXRlcmF0 b3ItPnZhbHVlID0gRGF0YU9iamVjdEd0azo6Y3JlYXRlKCk7CiAKLSAgICBIYXNoTWFwPEd0a0Ns aXBib2FyZCosIFJlZlB0cjxEYXRhT2JqZWN0R3RrPiA+OjppdGVyYXRvciBpdCA9IG9iamVjdE1h cC5maW5kKGNsaXBib2FyZCk7Ci0gICAgcmV0dXJuIGl0LT52YWx1ZS5nZXQoKTsKKyAgICByZXR1 cm4gYWRkUmVzdWx0Lml0ZXJhdG9yLT52YWx1ZS5nZXQoKTsKIH0KIAogfQo=