[webkit-reviews] review granted: [Bug 180294] [Web App Manifest] Add SPI for fetching the manifest : [Attachment 328369] Patch v1
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Dec 4 15:37:39 PST 2017
Geoffrey Garen <ggaren at apple.com> has granted David Quesada
<david_quesada at apple.com>'s request for review:
Bug 180294: [Web App Manifest] Add SPI for fetching the manifest
https://bugs.webkit.org/show_bug.cgi?id=180294
Attachment 328369: Patch v1
https://bugs.webkit.org/attachment.cgi?id=328369&action=review
--- Comment #3 from Geoffrey Garen <ggaren at apple.com> ---
Comment on attachment 328369
--> https://bugs.webkit.org/attachment.cgi?id=328369
Patch v1
View in context: https://bugs.webkit.org/attachment.cgi?id=328369&action=review
r=me with comments
> Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:4354
> + _page->getApplicationManifest([completion =
makeBlockPtr(completionHandler)](const
std::optional<WebCore::ApplicationManifest>& manifest,
WebKit::CallbackBase::Error error) {
You can just call this "completionHandler" as we do elsewhere, since the lambda
creates a new lexical scope.
> Source/WebKit/UIProcess/WebPageProxy.cpp:5323
> + // FIXME: Log error or assert.
> + // this can validly happen if a load invalidated the callback,
though
This comment contradicts itself, so you should remove it.
More information about the webkit-reviews
mailing list