Bug 188758 - WebKitTestRunner: Add watchOS entitlements
Summary: WebKitTestRunner: Add watchOS entitlements
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jonathan Bedard
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-08-20 14:28 PDT by Jonathan Bedard
Modified: 2018-08-27 15:40 PDT (History)
6 users (show)

See Also:


Attachments
Patch (4.36 KB, patch)
2018-08-20 14:43 PDT, Jonathan Bedard
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Bedard 2018-08-20 14:28:34 PDT
Testing WebKit on watchOS requires some specific entitlements.
Comment 1 Jonathan Bedard 2018-08-20 14:43:42 PDT
Created attachment 347547 [details]
Patch
Comment 2 Jonathan Bedard 2018-08-20 14:44:59 PDT
Talking with Tim Horton, while service workers are not supported on watchOS, in the event that they were supported in the future, someone would need to re-discover this entitlement, so I kept it.
Comment 3 WebKit Commit Bot 2018-08-20 16:10:27 PDT
Comment on attachment 347547 [details]
Patch

Clearing flags on attachment: 347547

Committed r235099: <https://trac.webkit.org/changeset/235099>
Comment 4 WebKit Commit Bot 2018-08-20 16:10:28 PDT
All reviewed patches have been landed.  Closing bug.
Comment 5 Radar WebKit Bug Importer 2018-08-20 16:48:21 PDT
<rdar://problem/43534326>
Comment 6 Daniel Bates 2018-08-27 13:40:22 PDT
(In reply to Jonathan Bedard from comment #2)
> Talking with Tim Horton, while service workers are not supported on watchOS,
> in the event that they were supported in the future, someone would need to
> re-discover this entitlement, so I kept it.

We (the WebKit OpenSource Project and Apple) practice <https://en.wikipedia.org/wiki/You_aren%27t_gonna_need_it>: do not add functionality until deemed necessary. And this bug does just that. Adding "dead code" like what is being done in this patch hurts hackability and I suggest we revert <https://trac.webkit.org/changeset/235099>. The entitlement can be found by looking at <https://trac.webkit.org/browser/webkit/trunk/Tools/WebKitTestRunner/Configurations/WebKitTestRunnerApp-iOS.entitlements?rev=228589> or grepping ChangeLogs and blaming code to find the code changes associated with the bug #182865 , which added the entitlement.
Comment 7 Daniel Bates 2018-08-27 13:42:50 PDT
(In reply to Daniel Bates from comment #6)
> (In reply to Jonathan Bedard from comment #2)
> > Talking with Tim Horton, while service workers are not supported on watchOS,
> > in the event that they were supported in the future, someone would need to
> > re-discover this entitlement, so I kept it.
> 
> We (the WebKit OpenSource Project and Apple) practice
> <https://en.wikipedia.org/wiki/You_aren%27t_gonna_need_it>: do not add
> functionality until deemed necessary. And this bug does just that.

*And this bug does does the opposite: It adds functionality that is not necessary.
Comment 8 Jonathan Bedard 2018-08-27 15:40:08 PDT
(In reply to Daniel Bates from comment #6)
> (In reply to Jonathan Bedard from comment #2)
> > Talking with Tim Horton, while service workers are not supported on watchOS,
> > in the event that they were supported in the future, someone would need to
> > re-discover this entitlement, so I kept it.
> 
> We (the WebKit OpenSource Project and Apple) practice
> <https://en.wikipedia.org/wiki/You_aren%27t_gonna_need_it>: do not add
> functionality until deemed necessary. And this bug does just that. Adding
> "dead code" like what is being done in this patch hurts hackability and I
> suggest we revert <https://trac.webkit.org/changeset/235099>. The
> entitlement can be found by looking at
> <https://trac.webkit.org/browser/webkit/trunk/Tools/WebKitTestRunner/
> Configurations/WebKitTestRunnerApp-iOS.entitlements?rev=228589> or grepping
> ChangeLogs and blaming code to find the code changes associated with the bug
> #182865 , which added the entitlement.

Do not revert <https://trac.webkit.org/changeset/235099>.

There are two entitlements in that change, one which is most certainly being used. If you would like to remove the Service Workers entitlement, then upload a patch to do so.