Bug 206764

Summary: Content filter should use WTF::Function instead of std::function
Product: WebKit Reporter: Alex Christensen <achristensen>
Component: New BugsAssignee: Alex Christensen <achristensen>
Status: NEW ---    
Severity: Normal CC: aestes, youennf, ysuzuki
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch ysuzuki: review-

Description Alex Christensen 2020-01-24 12:25:29 PST
Content filter should use async completion handler instead of ignoring then reloading
Comment 1 Alex Christensen 2020-01-24 13:19:00 PST
Created attachment 388720 [details]
Patch
Comment 2 Alex Christensen 2020-01-24 14:11:43 PST
Andy, any idea what I did wrong in this first patch?
Comment 3 youenn fablet 2020-01-30 00:20:36 PST
Comment on attachment 388720 [details]
Patch

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

> Source/WebCore/platform/cocoa/ContentFilterUnblockHandlerCocoa.mm:78
> +        wrapped.requestUnblockAsync([wrappedDecisionHandler = WTFMove(wrappedDecisionHandler), decisionHandler = WTFMove(decisionHandler)](bool unblocked) mutable {

Can m_unblockRequester be called multiple times?
If so, we are moving decisionHandler several times.
Should we instead have decisionHandler be part of ContentFilterUnblockHandler?

> Source/WebKit/UIProcess/ProvisionalPageProxy.cpp:377
> +void ProvisionalPageProxy::contentFilterDidBlockLoadForFrame(WebCore::ContentFilterUnblockHandler&& unblockHandler, FrameIdentifier frameID)

s/WebCore::/

> Source/WebKit/UIProcess/Cocoa/WebPageProxyCocoa.mm:104
> +void WebPageProxy::contentFilterDidBlockLoadForFrame(WebCore::ContentFilterUnblockHandler&& unblockHandler, FrameIdentifier frameID)

s/WebCore::/

> Source/WebKit/UIProcess/Cocoa/WebPageProxyCocoa.mm:109
> +void WebPageProxy::contentFilterDidBlockLoadForFrameShared(Ref<WebProcessProxy>&& process, WebCore::ContentFilterUnblockHandler&& unblockHandler, FrameIdentifier frameID)

s/WebCore::/
Also, why are we passing a Ref<WebProcessProxy>&&? Can we use a WebProcessProxy& instead?

> Source/WebKit/UIProcess/Cocoa/WebPageProxyCocoa.mm:111
>      if (WebFrameProxy* frame = process->webFrame(frameID))

auto*
Comment 4 Yusuke Suzuki 2020-06-12 20:38:25 PDT
Comment on attachment 388720 [details]
Patch

Putting r- since it is already reviewed and need some changes.