Bug 233714 - Add an InstallCoordination webpushd backend
Summary: Add an InstallCoordination webpushd backend
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Brady Eidson
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-12-01 11:07 PST by Brady Eidson
Modified: 2022-02-18 11:02 PST (History)
8 users (show)

See Also:


Attachments
EWS v1 (41.88 KB, patch)
2021-12-01 14:00 PST, Brady Eidson
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
EWS v2 (42.14 KB, patch)
2021-12-01 16:15 PST, Brady Eidson
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
EWS v3 (43.13 KB, patch)
2021-12-01 17:05 PST, Brady Eidson
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch v1 (47.83 KB, patch)
2021-12-01 17:51 PST, Brady Eidson
no flags Details | Formatted Diff | Diff
Patch v2 (47.85 KB, patch)
2021-12-01 18:02 PST, Brady Eidson
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch v3 (49.34 KB, patch)
2021-12-01 20:11 PST, Brady Eidson
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch v4 (47.62 KB, patch)
2021-12-01 20:43 PST, Brady Eidson
achristensen: review+
Details | Formatted Diff | Diff
PFL (47.67 KB, patch)
2021-12-02 12:21 PST, Brady Eidson
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Revived for EWS + landing (51.79 KB, patch)
2022-02-17 19:33 PST, Brady Eidson
no flags Details | Formatted Diff | Diff
Revived for EWS + landing v2 (51.79 KB, patch)
2022-02-17 19:34 PST, Brady Eidson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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].