[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