Bug 180795 - Use BlockPtrs and lambdas instead of new/delete to pass parameters to blocks in WebViewImpl::performDragOperation
Summary: Use BlockPtrs and lambdas instead of new/delete to pass parameters to blocks ...
Status: REOPENED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alex Christensen
URL:
Keywords: InRadar
Depends on: 183152
Blocks:
  Show dependency treegraph
 
Reported: 2017-12-13 19:07 PST by Alex Christensen
Modified: 2018-02-26 15:18 PST (History)
4 users (show)

See Also:


Attachments
Patch (6.38 KB, patch)
2017-12-13 19:08 PST, Alex Christensen
bfulgham: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Christensen 2017-12-13 19:07:41 PST
Use BlockPtrs and lambdas instead of new/delete to pass parameters to blocks in WebViewImpl::performDragOperation
Comment 1 Alex Christensen 2017-12-13 19:08:01 PST
Created attachment 329311 [details]
Patch
Comment 2 Brent Fulgham 2017-12-28 14:05:34 PST
Comment on attachment 329311 [details]
Patch

Nice! r=me.
Comment 3 Alex Christensen 2018-01-02 12:03:02 PST
http://trac.webkit.org/r226330
Comment 4 Radar WebKit Bug Importer 2018-01-02 12:57:55 PST
<rdar://problem/36260853>
Comment 5 Andy Estes 2018-02-24 20:10:30 PST
Comment on attachment 329311 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=329311&action=review

> Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm:3723
> +            [item receivePromisedFilesAtDestination:dropLocation.get() options:options operationQueue:queue.get() reader:BlockPtr<void(NSURL *, NSError *)>::fromCallable([this, fileNames = WTFMove(fileNames), fileCount, dragData = WTFMove(dragData), pasteboardName](NSURL * _Nonnull fileURL, NSError * _Nullable errorOrNil) mutable {

You can't move fileNames here. The outer lambda that captured fileNames is called multiple times by -enumerateDraggingItems..., and you'll end up moving from the same variable multiple times, which is illegal.

> Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm:3729
> +                    if (fileNames.size() == fileCount) {

Because of how fileNames is being moved, this condition will never be true if fileCount is greater than 1.
Comment 6 WebKit Commit Bot 2018-02-26 15:18:55 PST
Re-opened since this is blocked by bug 183152