[webkit-reviews] review granted: [Bug 233714] Add an InstallCoordination webpushd backend : [Attachment 445666] Patch v4

[bugzilla-daemon at webkit.org]

Alex Christensen <achristensen at apple.com> has granted Brady Eidson
<beidson at apple.com>'s request for review:
Bug 233714: Add an InstallCoordination webpushd backend
https://bugs.webkit.org/show_bug.cgi?id=233714

Attachment 445666: Patch v4

https://bugs.webkit.org/attachment.cgi?id=445666&action=review




--- Comment #9 from Alex Christensen <achristensen at apple.com> ---
Comment on attachment 445666
  --> https://bugs.webkit.org/attachment.cgi?id=445666
Patch v4

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

> Source/WebKit/webpushd/ICAppBundle.mm:137
> +    appDigest->addBytes(utf8Identifier.data(), utf8Identifier.length());

Do we want to add a salt?

> Source/WebKit/webpushd/ICAppBundle.mm:295
> +    dataPromise.get().percentComplete = 1.0;

Should this be 100?

> Source/WebKit/webpushd/ICAppBundle.mm:313
> +void ICAppBundle::bundleCreationFailed(NSError *error)

Why error parameter?

> Tools/TestWebKitAPI/Tests/WebKitCocoa/WebPushDaemon.mm:333
> +#if PLATFORM(IOS)

ENABLE(INSTALL_COORDINATION_BUNDLES)