[Webkit-unassigned] [Bug 171100] Implement the Web Share API

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Aug 27 20:04:04 PDT 2018


--- Comment #53 from Wenson Hsieh <wenson_hsieh at apple.com> ---
Comment on attachment 348241
  --> https://bugs.webkit.org/attachment.cgi?id=348241

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

I think this is looking pretty good. Just some minor comments below:

> Source/WebCore/page/Navigator.cpp:135
> +    return;

Nit - you don't need the return down here.

> Source/WebKit/UIProcess/API/C/WKPreferencesRefPrivate.h:500
> +// Defaults to true

It seems like this only defaults to true on iOS?

> Source/WebKit/UIProcess/PageClient.h:205
> +    virtual bool showShareSheet(WebPageProxy*, const WebCore::ShareDataWithParsedURL&, WTF::CompletionHandler<void (bool)>&&) { return false; }

It doesn't seem like the first argument here (WebPageProxy*) is needed.

> Source/WebKit/UIProcess/WebPageProxy.cpp:4593
> +    CompletionHandler<void(bool)> completionHandler = [this, protectedThis = makeRef(*this), callbackID] (bool access) {

I don't think you need to capture this as well as protectedThis; you can just capture protectedThis and then use protectedThis->m_process and protectedThis->m_pageID.

> Source/WebKit/UIProcess/WebPageProxy.h:1479
> +    void runShareSheet(const WebCore::ShareDataWithParsedURL&, uint64_t callbackID);


> Source/WebKit/WebProcess/WebPage/WebPage.h:1078
> +    void runShareSheetResponse(bool wasCompleted, uint64_t contextId);


> LayoutTests/resources/ui-helper.js:248
> +	const resolveShareSheet = `(() => uiController.invokeShareSheetWithResolution(${resolved}))()`;

Nit - still missing one level of indentation here.

You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20180828/a01c5937/attachment.html>

More information about the webkit-unassigned mailing list