Bug 233714 – Add an InstallCoordination webpushd backend

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Brady Eidson]

Add an InstallCoordination webpushd backend

[Brady Eidson]

Created attachment 445613 [details]
EWS v1

[Tim Horton]

Comment on attachment 445613 [details]
EWS v1

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

> Source/WebKit/webpushd/AppBundleRequest.mm:56
> +#if PLATFORM(IOS)

Please give yourself an ENABLE for all of these

> Source/WebKit/webpushd/ICAppBundle.mm:190
> +    LSApplicationRecord *appRecord = [[LSApplicationRecord alloc] initWithBundleIdentifier:(NSString *)getBundleIdentifier() allowPlaceholder:YES error:&error];

This leaks

[Brady Eidson]

Created attachment 445631 [details]
EWS v2

[Brady Eidson]

Created attachment 445639 [details]
EWS v3

[Brady Eidson]

Created attachment 445648 [details]
Patch v1

[Brady Eidson]

Created attachment 445649 [details]
Patch v2

[Brady Eidson]

Created attachment 445661 [details]
Patch v3

[Brady Eidson]

Created attachment 445666 [details]
Patch v4

[Alex Christensen]

Comment on attachment 445666 [details]
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)

[Brady Eidson]

(In reply to Alex Christensen from comment #9)
> Comment on attachment 445666 [details]
> 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?

Salt discussion happening out of band, can be added later if needed.

> 
> > Source/WebKit/webpushd/ICAppBundle.mm:295
> > +    dataPromise.get().percentComplete = 1.0;
> 
> Should this be 100?

Misnamed SPI - It's actually 0.0 to 1.0 despite the "percent" name

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

TBD

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

Changed.

[Brady Eidson]

Created attachment 445759 [details]
PFL

[EWS]

ChangeLog entry in Source/WebCore/PAL/ChangeLog contains OOPS!.

[Radar WebKit Bug Importer]

<rdar://problem/86220489>

[Brady Eidson]

Created attachment 452470 [details]
Revived for EWS + landing

[Brady Eidson]

Created attachment 452471 [details]
Revived for EWS + landing v2

[EWS]

Committed r290146 (247485@main): <https://commits.webkit.org/247485@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 452471 [details].