Bug 238003 - Allow push preference to be set programatically
Summary: Allow push preference to be set programatically
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ben Nham
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-03-16 23:51 PDT by Ben Nham
Modified: 2022-03-17 09:48 PDT (History)
2 users (show)

See Also:


Attachments
Patch (4.42 KB, patch)
2022-03-16 23:56 PDT, Ben Nham
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ben Nham 2022-03-16 23:51:52 PDT
Allow push preference to be set programatically
Comment 1 Ben Nham 2022-03-16 23:56:40 PDT
Created attachment 454935 [details]
Patch
Comment 2 Geoffrey Garen 2022-03-17 09:29:52 PDT
Comment on attachment 454935 [details]
Patch

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

r=me

> Source/WebKit/UIProcess/API/Cocoa/WKPreferencesPrivate.h:182
>  @property (nonatomic, setter=_setStorageAPIEnabled:) BOOL _storageAPIEnabled WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA));
>  @property (nonatomic, setter=_setAccessHandleEnabled:) BOOL _accessHandleEnabled WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA));
>  @property (nonatomic, setter=_setNotificationsEnabled:) BOOL _notificationsEnabled WK_API_AVAILABLE(macos(10.13.4), ios(WK_IOS_TBA));
> +@property (nonatomic, setter=_setPushAPIEnabled:) BOOL _pushAPIEnabled WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA));
>  @property (nonatomic, setter=_setModelDocumentEnabled:) BOOL _modelDocumentEnabled WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA));

Kind of surprising to me that we have these toggles on WKPreferences, even though they're things that can't change after a webpage has loaded -- but I guess yours is not the first.

Do we not have a generic API for turning on an experimental feature? I believe we do, and it is used in the Develop->Experimental Features menu. What happens if the Experimental Feature is set to No and _setPushAPIEnabled: is set to YES?

I'll say r+ for now because this is fine; but please follow up on whether we can use the experimental features API instead.
Comment 3 Ben Nham 2022-03-17 09:39:09 PDT
We do have that API but it's really meant to be used by the menu, which iterates over all experimental features and provides some logic to toggle each of the features on and off via the GUI. If you wanted to do it outside of that context then you would write something like:

```
for (_WKExperimentalFeature *feature in [WKPreferences _experimentalFeatures])
    if ([feature.key isEqualToString:@"PushAPIEnabled"])
        [preferences _setEnabled:YES forFeature:feature];
```

There are plenty of other properties in WKPreferences that manipulate experimental preferences like in this patch so you don't have to write the loop above.

(In reply to Geoffrey Garen from comment #2)
> Comment on attachment 454935 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=454935&action=review
> 
> r=me
> 
> > Source/WebKit/UIProcess/API/Cocoa/WKPreferencesPrivate.h:182
> >  @property (nonatomic, setter=_setStorageAPIEnabled:) BOOL _storageAPIEnabled WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA));
> >  @property (nonatomic, setter=_setAccessHandleEnabled:) BOOL _accessHandleEnabled WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA));
> >  @property (nonatomic, setter=_setNotificationsEnabled:) BOOL _notificationsEnabled WK_API_AVAILABLE(macos(10.13.4), ios(WK_IOS_TBA));
> > +@property (nonatomic, setter=_setPushAPIEnabled:) BOOL _pushAPIEnabled WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA));
> >  @property (nonatomic, setter=_setModelDocumentEnabled:) BOOL _modelDocumentEnabled WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA));
> 
> Kind of surprising to me that we have these toggles on WKPreferences, even
> though they're things that can't change after a webpage has loaded -- but I
> guess yours is not the first.
> 
> Do we not have a generic API for turning on an experimental feature? I
> believe we do, and it is used in the Develop->Experimental Features menu.
> What happens if the Experimental Feature is set to No and
> _setPushAPIEnabled: is set to YES?
> 
> I'll say r+ for now because this is fine; but please follow up on whether we
> can use the experimental features API instead.
Comment 4 EWS 2022-03-17 09:47:01 PDT
Committed r291413 (248541@main): <https://commits.webkit.org/248541@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 454935 [details].
Comment 5 Radar WebKit Bug Importer 2022-03-17 09:48:18 PDT
<rdar://problem/90436709>