Bug 173197 - [WebCrypto] Remove experimental feature flag of SubtleCrypto
Summary: [WebCrypto] Remove experimental feature flag of SubtleCrypto
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jiewen Tan
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-06-09 18:03 PDT by Jiewen Tan
Modified: 2017-06-13 13:05 PDT (History)
13 users (show)

See Also:

Attachments
Patch (15.87 KB, patch)
2017-06-09 18:10 PDT, Jiewen Tan 		no flags Details | Formatted Diff | Diff
Patch (15.89 KB, patch)
2017-06-09 18:20 PDT, Jiewen Tan 		bfulgham: review+
bfulgham: commit-queue-
Details | Formatted Diff | Diff
Patch for landing (16.03 KB, patch)
2017-06-12 12:45 PDT, Jiewen Tan 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Jiewen Tan 2017-06-09 18:03:31 PDT 
Remove experimental feature flag.
Comment 1 Radar WebKit Bug Importer 2017-06-09 18:04:32 PDT 
<rdar://problem/32688148>
Comment 2 Jiewen Tan 2017-06-09 18:10:29 PDT 
Created attachment 312523 [details]
Patch
Comment 3 Jiewen Tan 2017-06-09 18:20:37 PDT 
Created attachment 312524 [details]
Patch
Comment 4 Sam Weinig 2017-06-09 20:36:13 PDT 
This patch does not contain a reason.
Comment 5 Brent Fulgham 2017-06-12 09:45:11 PDT 
Comment on attachment 312524 [details]
Patch

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

> Source/WebCore/ChangeLog:7
> +        Reviewed by NOBODY (OOPS!).

Please add a statement like "The SubtleCrypto implementation is no longer experimental and is ready for production use. We are therefore removing the runtime flag."
Comment 6 Jiewen Tan 2017-06-12 12:40:16 PDT 
Comment on attachment 312524 [details]
Patch

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

Thanks Brent for r+ my patch.

>> Source/WebCore/ChangeLog:7
>> +        Reviewed by NOBODY (OOPS!).
> 
> Please add a statement like "The SubtleCrypto implementation is no longer experimental and is ready for production use. We are therefore removing the runtime flag."

Fixed.
Comment 7 Jiewen Tan 2017-06-12 12:45:01 PDT 
Created attachment 312686 [details]
Patch for landing
Comment 8 WebKit Commit Bot 2017-06-12 13:26:29 PDT 
The commit-queue encountered the following flaky tests while processing attachment 312686 [details]:

imported/w3c/web-platform-tests/media-source/mediasource-config-change-mp4-av-audio-bitrate.html bug 173270 (author: jer.noble@apple.com)
The commit-queue is continuing to process your patch.
Comment 9 WebKit Commit Bot 2017-06-12 13:27:04 PDT 
Comment on attachment 312686 [details]
Patch for landing

Clearing flags on attachment: 312686

Committed r218129: <http://trac.webkit.org/changeset/218129>
Comment 10 Michael Catanzaro 2017-06-12 18:41:47 PDT 
Congrats Jiewen!
Comment 11 Daniel Bates 2017-06-13 10:14:39 PDT 
Comment on attachment 312524 [details]
Patch

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

> Source/WebCore/page/Crypto.idl:34
> +    [Conditional=SUBTLE_CRYPTO] readonly attribute SubtleCrypto subtle;

Would it be reasonable to mark this as SecureContext as per the spec.? Ideally, we would mark all WebCrypto API as SecureContext though I'm unclear if we can mark API associated with the deprecated webkitSubtle without breaking web compatibility.
Comment 12 Michael Catanzaro 2017-06-13 11:06:32 PDT 
Good catch.

Was webkitSubtle ever web exposed? Are we not able to just remove it?
Comment 13 Jiewen Tan 2017-06-13 13:04:14 PDT 
(In reply to Daniel Bates from comment #11)
> Comment on attachment 312524 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=312524&action=review
> 
> > Source/WebCore/page/Crypto.idl:34
> > +    [Conditional=SUBTLE_CRYPTO] readonly attribute SubtleCrypto subtle;
> 
> Would it be reasonable to mark this as SecureContext as per the spec.?
> Ideally, we would mark all WebCrypto API as SecureContext though I'm unclear
> if we can mark API associated with the deprecated webkitSubtle without
> breaking web compatibility.

We should mark this as SecureContext. See Bug 166959. For the webkitSubtle, I think we shouldn't.
Comment 14 Jiewen Tan 2017-06-13 13:05:23 PDT 
(In reply to Michael Catanzaro from comment #12)
> Good catch.
> 
> Was webkitSubtle ever web exposed? Are we not able to just remove it?

It is web exposed therefore I think we should gradually deprecate it.