Bug 236846

Summary: onpush/onpushsubscriptionchange exposed in Service Workers when Push API is disabled
Product: WebKit Reporter: Sam Sneddon [:gsnedders] <gsnedders>
Component: Service WorkersAssignee: Sam Sneddon [:gsnedders] <gsnedders>
Status: RESOLVED FIXED    
Severity: Normal CC: beidson, cdumez, esprehn+autocc, ews-watchlist, ggaren, kondapallykalyan, nham, ntim, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=236874
Attachments:
Description Flags
Patch
none
[fast-cq] Patch none

Description Sam Sneddon [:gsnedders] 2022-02-18 10:42:12 PST
This will be because:

[
    EnabledAtRuntime=ServiceWorkerEnabled
] partial interface ServiceWorkerGlobalScope {
    attribute EventHandler onpush;
    attribute EventHandler onpushsubscriptionchange;
};

This isn't gated on Push API being enabled.
Comment 1 Radar WebKit Bug Importer 2022-02-18 10:42:46 PST
<rdar://problem/89154452>
Comment 2 Sam Sneddon [:gsnedders] 2022-02-18 11:46:32 PST
Created attachment 452554 [details]
Patch
Comment 3 Tim Nguyen (:ntim) 2022-02-18 11:56:59 PST
Comment on attachment 452554 [details]
Patch

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

> LayoutTests/http/wpt/push-api/onpush-disabled-expected.txt:3
> +PASS onpush
> +PASS onpushsubscriptionchange

Can you use more descriptive PASS messages like "onpush is not exposed when pref is disabled"?

> LayoutTests/http/wpt/push-api/onpush-enabled-expected.txt:3
> +PASS onpush
> +PASS onpushsubscriptionchange

ditto
Comment 4 Chris Dumez 2022-02-18 11:59:30 PST
Comment on attachment 452554 [details]
Patch

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

> Source/WebCore/Modules/push-api/ServiceWorkerGlobalScope+PushAPI.idl:28
> +    EnabledAtRuntime=PushAPIEnabled

Shouldn't this be `EnabledAtRuntime=ServiceWorkerEnabled&PushAPIEnabled` ?
Comment 5 Chris Dumez 2022-02-18 12:05:17 PST
Created attachment 452556 [details]
[fast-cq] Patch
Comment 6 Chris Dumez 2022-02-18 12:58:59 PST
Comment on attachment 452556 [details]
[fast-cq] Patch

Clearing flags on attachment: 452556

Committed r290156 (247495@trunk): <https://commits.webkit.org/247495@trunk>
Comment 7 Chris Dumez 2022-02-18 12:59:02 PST
All reviewed patches have been landed.  Closing bug.