Bug 181705 - Make WebContextMenuListenerProxy a wrapper around a CompletionHandler
Summary: Make WebContextMenuListenerProxy a wrapper around a CompletionHandler
Status: RESOLVED WONTFIX
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:
Depends on:
Blocks:
 
Reported: 2018-01-16 14:18 PST by Alex Christensen
Modified: 2018-01-16 16:43 PST (History)
1 user (show)

See Also:

Attachments
Patch (4.57 KB, patch)
2018-01-16 14:19 PST, Alex Christensen 		no flags Details | Formatted Diff | Diff
Patch (5.77 KB, patch)
2018-01-16 14:26 PST, Alex Christensen 		achristensen: review-
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Christensen 2018-01-16 14:18:55 PST 
Make WebContextMenuListenerProxy a wrapper around a CompletionHandler
Comment 1 Alex Christensen 2018-01-16 14:19:43 PST 
Created attachment 331428 [details]
Patch
Comment 2 Alex Christensen 2018-01-16 14:26:50 PST 
Created attachment 331431 [details]
Patch
Comment 3 Sam Weinig 2018-01-16 16:15:25 PST 
Comment on attachment 331431 [details]
Patch

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

> Source/WebKit/UIProcess/WebContextMenuListenerProxy.cpp:59
> -    m_contextMenuMac = nullptr;
> +    m_completionHandler({ });

This seems odd to me.  Do you really want to now call the completion handler on invalidation?
Comment 4 Alex Christensen 2018-01-16 16:43:08 PST 
Comment on attachment 331431 [details]
Patch

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

>> Source/WebKit/UIProcess/WebContextMenuListenerProxy.cpp:59
>> +    m_completionHandler({ });
> 
> This seems odd to me.  Do you really want to now call the completion handler on invalidation?

Hmmm.  I guess not, which means it's not a completion handler.  I think we'll just not land this patch, because an ObjC completion handler can just capture the listener like we do with authentication challenges.