|
|
Chromium Code Reviews|
Created:
3 years, 6 months ago by raymes Modified:
3 years, 5 months ago CC:
asvitkine+watch_chromium.org, blink-reviews, blink-reviews-api_chromium.org, chrome-apps-syd-reviews_chromium.org, chromium-reviews, dglazkov+blink, eric.carlson_apple.com, feature-media-reviews_chromium.org, haraken, lunalu1, mlamouri+watch-blink_chromium.org, Srirama Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionAdd use counters for navigator.requestMediaKeySystemAccess
Adds counters for valid usage and usage from cross-origin iframes
BUG=689802
Review-Url: https://codereview.chromium.org/2941003002
Cr-Commit-Position: refs/heads/master@{#483931}
Committed: https://chromium.googlesource.com/chromium/src/+/e23beab8620190c576a22b39f1469ae73705f30f
Patch Set 1 #
Total comments: 1
Patch Set 2 : Add use counters for navigator.requestMediaKeySystemAccess #
Total comments: 2
Patch Set 3 : . #Patch Set 4 : . #Patch Set 5 : . #
Messages
Total messages: 32 (17 generated)
raymes@chromium.org changed reviewers: + ddorwin@chromium.org
ddorwin@chromium.org changed reviewers: + xhwang@chromium.org
https://codereview.chromium.org/2941003002/diff/1/third_party/WebKit/public/p... File third_party/WebKit/public/platform/UseCounterFeature.def (right): https://codereview.chromium.org/2941003002/diff/1/third_party/WebKit/public/p... third_party/WebKit/public/platform/UseCounterFeature.def:1557: kRequestMediaKeySystemAccess = 2024, We used to have values for secure and insecure origins (https://www.chromestatus.com/metrics/feature/timeline/popularity/770, https://www.chromestatus.com/metrics/feature/timeline/popularity/771), but apparently these were removed. I wonder if we shouldn't revive the secure origin one so we have a single history, albeit with some missing data. I also wonder about naming. This is the method, but "EncryptedMedia" might be more meaningful to people. +xhwang.
Thanks! I changed the names as suggested. PTAL :)
looking good. only one naming suggestion. Thanks! https://codereview.chromium.org/2941003002/diff/20001/third_party/WebKit/publ... File third_party/WebKit/public/platform/WebFeature.h (right): https://codereview.chromium.org/2941003002/diff/20001/third_party/WebKit/publ... third_party/WebKit/public/platform/WebFeature.h:1565: kEncryptedMediaIframe = 2027, naming nit: "Iframe" is ambiguous. It seems we already have: AudioContextCrossOriginIframe WebAudioAutoplayCrossOriginIframe Should we use "EncryptedMediaCrossOriginIframe" instead?
ptal, thanks! https://codereview.chromium.org/2941003002/diff/20001/third_party/WebKit/publ... File third_party/WebKit/public/platform/WebFeature.h (right): https://codereview.chromium.org/2941003002/diff/20001/third_party/WebKit/publ... third_party/WebKit/public/platform/WebFeature.h:1565: kEncryptedMediaIframe = 2027, On 2017/06/21 17:03:18, xhwang wrote: > naming nit: "Iframe" is ambiguous. It seems we already have: > > AudioContextCrossOriginIframe > WebAudioAutoplayCrossOriginIframe > > Should we use "EncryptedMediaCrossOriginIframe" instead? Done.
The CQ bit was checked by raymes@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios-device on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...) ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...) mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by raymes@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
lgtm
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
raymes@chromium.org changed reviewers: + dstockwell@chromium.org, isherman@chromium.org
ddorwin, does this lg? +dstockwell for WebFeature.h OWNERS +isherman for enums.xml OWNERS
lgtm
On 2017/06/29 01:04:53, raymes wrote: > +isherman for enums.xml OWNERS You don't need my review for a simple addition to enums.xml =)
On 2017/06/29 04:37:41, Ilya Sherman wrote: > On 2017/06/29 01:04:53, raymes wrote: > > +isherman for enums.xml OWNERS > > You don't need my review for a simple addition to enums.xml =) Oh, thanks for the heads up and sorry for the noise :)
lgtm
The CQ bit was checked by raymes@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...) mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
The CQ bit was checked by raymes@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from dstockwell@chromium.org, xhwang@chromium.org, ddorwin@chromium.org Link to the patchset: https://codereview.chromium.org/2941003002/#ps80001 (title: ".")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 80001, "attempt_start_ts": 1499045050783440,
"parent_rev": "c5330b4aab314775157dafe8adadde0f600be8b7", "commit_rev":
"e23beab8620190c576a22b39f1469ae73705f30f"}
Message was sent while issue was closed.
Description was changed from ========== Add use counters for navigator.requestMediaKeySystemAccess Adds counters for valid usage and usage from cross-origin iframes BUG=689802 ========== to ========== Add use counters for navigator.requestMediaKeySystemAccess Adds counters for valid usage and usage from cross-origin iframes BUG=689802 Review-Url: https://codereview.chromium.org/2941003002 Cr-Commit-Position: refs/heads/master@{#483931} Committed: https://chromium.googlesource.com/chromium/src/+/e23beab8620190c576a22b39f146... ==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001) as https://chromium.googlesource.com/chromium/src/+/e23beab8620190c576a22b39f146... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
