| Summary: | Content filter should use WTF::Function instead of std::function | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Alex Christensen <achristensen> | ||||
| Component: | New Bugs | Assignee: | Alex Christensen <achristensen> | ||||
| Status: | NEW --- | ||||||
| Severity: | Normal | CC: | aestes, youennf, ysuzuki | ||||
| Priority: | P2 | ||||||
| Version: | WebKit Nightly Build | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Attachments: |
|
||||||
|
Description
Alex Christensen
2020-01-24 12:25:29 PST
Created attachment 388720 [details]
Patch
Andy, any idea what I did wrong in this first patch? 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 on attachment 388720 [details]
Patch
Putting r- since it is already reviewed and need some changes.
|