Bug 122946 – Main thread tasks in ThreadableBlobRegistry should use std::unique_ptr

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Zan Dobersek]

Main thread tasks in ThreadableBlobRegistry should use std::unique_ptr

[Zan Dobersek]

Created attachment 214429 [details]
Patch

[Darin Adler]

Comment on attachment 214429 [details]
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*.

[Zan Dobersek]

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))));

[Anders Carlsson]

(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.

[Zan Dobersek]

OK, I'll land the original patch.

[Zan Dobersek]

Comment on attachment 214429 [details]
Patch

Clearing flags on attachment: 214429

Committed r158661: <http://trac.webkit.org/changeset/158661>

[Zan Dobersek]

All reviewed patches have been landed.  Closing bug.