122946 2013-10-17 00:45:45 -0700 Main thread tasks in ThreadableBlobRegistry should use std::unique_ptr 2013-11-05 02:47:06 -0800 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 zan zan andersca darin oldest_to_newest 940682 0 zan 2013-10-17 00:45:45 -0700 Main thread tasks in ThreadableBlobRegistry should use std::unique_ptr 940686 1 214429 zan 2013-10-17 00:56:37 -0700 Created attachment 214429 Patch 946305 2 214429 darin 2013-11-02 14:17:35 -0700 Comment on attachment 214429 Patch Every time we have a new rather than a make_unique, it makes it hard to be sure we got the object lifetime right. It would be nicer to use the lambda form of callOnMainThread so we could pass the unique_ptr through rather than having to pass it through as a void*. 946701 3 zan 2013-11-04 09:37:42 -0800 Lambda could be used here if wrapped inside std::bind() with a naked pointer to the BlobRegistryContext object bound as the first argument to the lambda function, where the pointer would then be put under a std::unique_ptr. Binding std::unique_ptr as the first argument to the lambda function doesn't work since storing the std::bind() product inside a std::function requires that product to be copy-constructible, but this falters since std::unique_ptr is not copyable. So, this works, and I have a patch ready if it's preferable: callOnMainThread(std::bind([](BlobRegistryContext* contextPtr) { std::unique_ptr<BlobRegistryContext> context(contextPtr); blobRegistry().registerBlobURL(context->url, std::move(context->blobData)); }, new BlobRegistryContext(url, std::move(blobData)))); 946734 4 andersca 2013-11-04 10:58:20 -0800 (In reply to comment #3) > Lambda could be used here if wrapped inside std::bind() with a naked pointer to the BlobRegistryContext object bound as the first argument to the lambda function, where the pointer would then be put under a std::unique_ptr. > > Binding std::unique_ptr as the first argument to the lambda function doesn't work since storing the std::bind() product inside a std::function requires that product to be copy-constructible, but this falters since std::unique_ptr is not copyable. > > So, this works, and I have a patch ready if it's preferable: > > callOnMainThread(std::bind([](BlobRegistryContext* contextPtr) { > std::unique_ptr<BlobRegistryContext> context(contextPtr); > blobRegistry().registerBlobURL(context->url, std::move(context->blobData)); > }, new BlobRegistryContext(url, std::move(blobData)))); Let’s hold off on making such a change. I’m working on an experiment that will let you create a lambda and do a “cross-thread copy” of all the required parameters. 947046 5 zan 2013-11-05 00:06:46 -0800 OK, I'll land the original patch. 947097 6 214429 zan 2013-11-05 02:47:00 -0800 Comment on attachment 214429 Patch Clearing flags on attachment: 214429 Committed r158661: <http://trac.webkit.org/changeset/158661> 947098 7 zan 2013-11-05 02:47:06 -0800 All reviewed patches have been landed. Closing bug. 214429 2013-10-17 00:56:37 -0700 2013-11-05 02:47:00 -0800 Patch bug-122946-20131017005636.patch text/plain 4364 zan U3VidmVyc2lvbiBSZXZpc2lvbjogMTU3NTIyCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggMGM2MDM1NDVjYzVmYjUz NjcyMGRlZWE5YzNhZjhlYTg5ODZhNTBhOC4uMTRmMmVkYjlmNzg0NDgyMjNhNjYyNmRlN2M5NjY2 N2NlNjY1NjMzZSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIxIEBACisyMDEzLTEwLTE3ICBaYW4g RG9iZXJzZWsgIDx6ZG9iZXJzZWtAaWdhbGlhLmNvbT4KKworICAgICAgICBNYWluIHRocmVhZCB0 YXNrcyBpbiBUaHJlYWRhYmxlQmxvYlJlZ2lzdHJ5IHNob3VsZCB1c2Ugc3RkOjp1bmlxdWVfcHRy CisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xMjI5NDYK KworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBUaGUgbmV3 IEJsb2JSZWdpc3RyeUNvbnRleHQgb2JqZWN0cyBkb24ndCBoYXZlIHRvIGJlIGFkb3B0ZWQgaW50 byBPd25QdHIgYW5kIHRoZW4gaGF2ZSBPd25QdHIncyBsZWFrZWQgcG9pbnRlcgorICAgICAgICBw YXNzZWQgaW50byB0aGUgV1RGOjpjYWxsT25NYWluVGhyZWFkIGNhbGwgLSB0aGUgcG9pbnRlciB0 byB0aGUgbmV3IGhlYXAtYWxsb2NhdGVkIG9iamVjdCBpcyBwYXNzZWQgaW4gZGlyZWN0bHksCisg ICAgICAgIHdpdGggdGhlIG9iamVjdCBlbmRpbmcgdXAgYmVpbmcgbWFuYWdlZCBieSBzdGQ6OnVu aXF1ZV9wdHIgaW4gdGhlIGRlc2lnbmF0ZWQgbWFpbiB0aHJlYWQgdGFzay4KKworICAgICAgICAq IGZpbGVhcGkvVGhyZWFkYWJsZUJsb2JSZWdpc3RyeS5jcHA6CisgICAgICAgIChXZWJDb3JlOjpy ZWdpc3RlckJsb2JVUkxUYXNrKToKKyAgICAgICAgKFdlYkNvcmU6OlRocmVhZGFibGVCbG9iUmVn aXN0cnk6OnJlZ2lzdGVyQmxvYlVSTCk6CisgICAgICAgIChXZWJDb3JlOjpyZWdpc3RlckJsb2JV UkxGcm9tVGFzayk6CisgICAgICAgIChXZWJDb3JlOjp1bnJlZ2lzdGVyQmxvYlVSTFRhc2spOgor ICAgICAgICAoV2ViQ29yZTo6VGhyZWFkYWJsZUJsb2JSZWdpc3RyeTo6dW5yZWdpc3RlckJsb2JV UkwpOgorCiAyMDEzLTEwLTE2ICBLeXVuZ1RhZSBLaW0gIDxrdGYua2ltQHNhbXN1bmcuY29tPgog CiAgICAgICAgIER1cmluZyBlZGl0aW5nLCBtZXJnZSBpbmxpbmUgc3R5bGUgd2l0aCBvdmVycmlk aW5nIG90aGVyIGF1dGhvciBzdHlsZXMKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL2ZpbGVh cGkvVGhyZWFkYWJsZUJsb2JSZWdpc3RyeS5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9maWxlYXBpL1Ro cmVhZGFibGVCbG9iUmVnaXN0cnkuY3BwCmluZGV4IDNjZDUzY2I0MjdlZDljNTRlNDk1OWFkZTNi ZDBjN2VhMmQwN2FhZWMuLmJiYzM4OWJhMGEyOWEyZTc5MjQ4NzEzMGViNDVkMWMyZjc5ZmQyMzkg MTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL2ZpbGVhcGkvVGhyZWFkYWJsZUJsb2JSZWdpc3Ry eS5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvZmlsZWFwaS9UaHJlYWRhYmxlQmxvYlJlZ2lzdHJ5 LmNwcApAQCAtMjksNyArMjksNiBAQAogICovCiAKICNpbmNsdWRlICJjb25maWcuaCIKLQogI2lu Y2x1ZGUgIlRocmVhZGFibGVCbG9iUmVnaXN0cnkuaCIKIAogI2luY2x1ZGUgIkJsb2JEYXRhLmgi CkBAIC04Myw3ICs4Miw3IEBAIHN0YXRpYyBUaHJlYWRTcGVjaWZpYzxCbG9iVXJsT3JpZ2luTWFw PiYgb3JpZ2luTWFwKCkKIAogc3RhdGljIHZvaWQgcmVnaXN0ZXJCbG9iVVJMVGFzayh2b2lkKiBj b250ZXh0KQogewotICAgIE93blB0cjxCbG9iUmVnaXN0cnlDb250ZXh0PiBibG9iUmVnaXN0cnlD b250ZXh0ID0gYWRvcHRQdHIoc3RhdGljX2Nhc3Q8QmxvYlJlZ2lzdHJ5Q29udGV4dCo+KGNvbnRl eHQpKTsKKyAgICBzdGQ6OnVuaXF1ZV9wdHI8QmxvYlJlZ2lzdHJ5Q29udGV4dD4gYmxvYlJlZ2lz dHJ5Q29udGV4dChzdGF0aWNfY2FzdDxCbG9iUmVnaXN0cnlDb250ZXh0Kj4oY29udGV4dCkpOwog ICAgIGJsb2JSZWdpc3RyeSgpLnJlZ2lzdGVyQmxvYlVSTChibG9iUmVnaXN0cnlDb250ZXh0LT51 cmwsIHN0ZDo6bW92ZShibG9iUmVnaXN0cnlDb250ZXh0LT5ibG9iRGF0YSkpOwogfQogCkBAIC05 MSwxNSArOTAsMTMgQEAgdm9pZCBUaHJlYWRhYmxlQmxvYlJlZ2lzdHJ5OjpyZWdpc3RlckJsb2JV UkwoY29uc3QgVVJMJiB1cmwsIHN0ZDo6dW5pcXVlX3B0cjxCbG8KIHsKICAgICBpZiAoaXNNYWlu VGhyZWFkKCkpCiAgICAgICAgIGJsb2JSZWdpc3RyeSgpLnJlZ2lzdGVyQmxvYlVSTCh1cmwsIHN0 ZDo6bW92ZShibG9iRGF0YSkpOwotICAgIGVsc2UgewotICAgICAgICBPd25QdHI8QmxvYlJlZ2lz dHJ5Q29udGV4dD4gY29udGV4dCA9IGFkb3B0UHRyKG5ldyBCbG9iUmVnaXN0cnlDb250ZXh0KHVy bCwgc3RkOjptb3ZlKGJsb2JEYXRhKSkpOwotICAgICAgICBjYWxsT25NYWluVGhyZWFkKCZyZWdp c3RlckJsb2JVUkxUYXNrLCBjb250ZXh0LmxlYWtQdHIoKSk7Ci0gICAgfQorICAgIGVsc2UKKyAg ICAgICAgY2FsbE9uTWFpblRocmVhZCgmcmVnaXN0ZXJCbG9iVVJMVGFzaywgbmV3IEJsb2JSZWdp c3RyeUNvbnRleHQodXJsLCBzdGQ6Om1vdmUoYmxvYkRhdGEpKSk7CiB9CiAKIHN0YXRpYyB2b2lk IHJlZ2lzdGVyQmxvYlVSTEZyb21UYXNrKHZvaWQqIGNvbnRleHQpCiB7Ci0gICAgT3duUHRyPEJs b2JSZWdpc3RyeUNvbnRleHQ+IGJsb2JSZWdpc3RyeUNvbnRleHQgPSBhZG9wdFB0cihzdGF0aWNf Y2FzdDxCbG9iUmVnaXN0cnlDb250ZXh0Kj4oY29udGV4dCkpOworICAgIHN0ZDo6dW5pcXVlX3B0 cjxCbG9iUmVnaXN0cnlDb250ZXh0PiBibG9iUmVnaXN0cnlDb250ZXh0KHN0YXRpY19jYXN0PEJs b2JSZWdpc3RyeUNvbnRleHQqPihjb250ZXh0KSk7CiAgICAgYmxvYlJlZ2lzdHJ5KCkucmVnaXN0 ZXJCbG9iVVJMKGJsb2JSZWdpc3RyeUNvbnRleHQtPnVybCwgYmxvYlJlZ2lzdHJ5Q29udGV4dC0+ c3JjVVJMKTsKIH0KIApAQCAtMTExLDE1ICsxMDgsMTMgQEAgdm9pZCBUaHJlYWRhYmxlQmxvYlJl Z2lzdHJ5OjpyZWdpc3RlckJsb2JVUkwoU2VjdXJpdHlPcmlnaW4qIG9yaWdpbiwgY29uc3QgVVJM JgogCiAgICAgaWYgKGlzTWFpblRocmVhZCgpKQogICAgICAgICBibG9iUmVnaXN0cnkoKS5yZWdp c3RlckJsb2JVUkwodXJsLCBzcmNVUkwpOwotICAgIGVsc2UgewotICAgICAgICBPd25QdHI8Qmxv YlJlZ2lzdHJ5Q29udGV4dD4gY29udGV4dCA9IGFkb3B0UHRyKG5ldyBCbG9iUmVnaXN0cnlDb250 ZXh0KHVybCwgc3JjVVJMKSk7Ci0gICAgICAgIGNhbGxPbk1haW5UaHJlYWQoJnJlZ2lzdGVyQmxv YlVSTEZyb21UYXNrLCBjb250ZXh0LmxlYWtQdHIoKSk7Ci0gICAgfQorICAgIGVsc2UKKyAgICAg ICAgY2FsbE9uTWFpblRocmVhZCgmcmVnaXN0ZXJCbG9iVVJMRnJvbVRhc2ssIG5ldyBCbG9iUmVn aXN0cnlDb250ZXh0KHVybCwgc3JjVVJMKSk7CiB9CiAKIHN0YXRpYyB2b2lkIHVucmVnaXN0ZXJC bG9iVVJMVGFzayh2b2lkKiBjb250ZXh0KQogewotICAgIE93blB0cjxCbG9iUmVnaXN0cnlDb250 ZXh0PiBibG9iUmVnaXN0cnlDb250ZXh0ID0gYWRvcHRQdHIoc3RhdGljX2Nhc3Q8QmxvYlJlZ2lz dHJ5Q29udGV4dCo+KGNvbnRleHQpKTsKKyAgICBzdGQ6OnVuaXF1ZV9wdHI8QmxvYlJlZ2lzdHJ5 Q29udGV4dD4gYmxvYlJlZ2lzdHJ5Q29udGV4dChzdGF0aWNfY2FzdDxCbG9iUmVnaXN0cnlDb250 ZXh0Kj4oY29udGV4dCkpOwogICAgIGJsb2JSZWdpc3RyeSgpLnVucmVnaXN0ZXJCbG9iVVJMKGJs b2JSZWdpc3RyeUNvbnRleHQtPnVybCk7CiB9CiAKQEAgLTEzMCwxMCArMTI1LDggQEAgdm9pZCBU aHJlYWRhYmxlQmxvYlJlZ2lzdHJ5Ojp1bnJlZ2lzdGVyQmxvYlVSTChjb25zdCBVUkwmIHVybCkK IAogICAgIGlmIChpc01haW5UaHJlYWQoKSkKICAgICAgICAgYmxvYlJlZ2lzdHJ5KCkudW5yZWdp c3RlckJsb2JVUkwodXJsKTsKLSAgICBlbHNlIHsKLSAgICAgICAgT3duUHRyPEJsb2JSZWdpc3Ry eUNvbnRleHQ+IGNvbnRleHQgPSBhZG9wdFB0cihuZXcgQmxvYlJlZ2lzdHJ5Q29udGV4dCh1cmwp KTsKLSAgICAgICAgY2FsbE9uTWFpblRocmVhZCgmdW5yZWdpc3RlckJsb2JVUkxUYXNrLCBjb250 ZXh0LmxlYWtQdHIoKSk7Ci0gICAgfQorICAgIGVsc2UKKyAgICAgICAgY2FsbE9uTWFpblRocmVh ZCgmdW5yZWdpc3RlckJsb2JVUkxUYXNrLCBuZXcgQmxvYlJlZ2lzdHJ5Q29udGV4dCh1cmwpKTsK IH0KIAogUGFzc1JlZlB0cjxTZWN1cml0eU9yaWdpbj4gVGhyZWFkYWJsZUJsb2JSZWdpc3RyeTo6 Z2V0Q2FjaGVkT3JpZ2luKGNvbnN0IFVSTCYgdXJsKQo=