Bug 233714

Summary: Add an InstallCoordination webpushd backend
Product: WebKit Reporter: Brady Eidson <beidson>
Component: WebKit Misc.Assignee: Brady Eidson <beidson>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, benjamin, cdumez, cmarcelo, ews-watchlist, jiewen_tan, thorton, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
EWS v1
ews-feeder: commit-queue-
EWS v2
ews-feeder: commit-queue-
EWS v3
ews-feeder: commit-queue-
Patch v1
none
Patch v2
ews-feeder: commit-queue-
Patch v3
ews-feeder: commit-queue-
Patch v4
achristensen: review+
PFL
ews-feeder: commit-queue-
Revived for EWS + landing
none
Revived for EWS + landing v2 none

Description Brady Eidson 2021-12-01 11:07:24 PST 
Add an InstallCoordination webpushd backend
Comment 1 Brady Eidson 2021-12-01 14:00:25 PST 
Created attachment 445613 [details]
EWS v1
Comment 2 Tim Horton 2021-12-01 14:13:15 PST 
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
Comment 3 Brady Eidson 2021-12-01 16:15:05 PST 
Created attachment 445631 [details]
EWS v2
Comment 4 Brady Eidson 2021-12-01 17:05:01 PST 
Created attachment 445639 [details]
EWS v3
Comment 5 Brady Eidson 2021-12-01 17:51:15 PST 
Created attachment 445648 [details]
Patch v1
Comment 6 Brady Eidson 2021-12-01 18:02:06 PST 
Created attachment 445649 [details]
Patch v2
Comment 7 Brady Eidson 2021-12-01 20:11:34 PST 
Created attachment 445661 [details]
Patch v3
Comment 8 Brady Eidson 2021-12-01 20:43:18 PST 
Created attachment 445666 [details]
Patch v4
Comment 9 Alex Christensen 2021-12-02 11:37:00 PST 
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)
Comment 10 Brady Eidson 2021-12-02 12:21:38 PST 
(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.
Comment 11 Brady Eidson 2021-12-02 12:21:52 PST 
Created attachment 445759 [details]
PFL
Comment 12 EWS 2021-12-02 14:23:38 PST 
ChangeLog entry in Source/WebCore/PAL/ChangeLog contains OOPS!.
Comment 13 Radar WebKit Bug Importer 2021-12-08 11:08:23 PST 
<rdar://problem/86220489>
Comment 14 Brady Eidson 2022-02-17 19:33:04 PST 
Created attachment 452470 [details]
Revived for EWS + landing
Comment 15 Brady Eidson 2022-02-17 19:34:35 PST 
Created attachment 452471 [details]
Revived for EWS + landing v2
Comment 16 EWS 2022-02-18 11:02:39 PST 
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].