Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1979553002:
WebRTC: Launch ECDSA as default certificate (Closed)
|
Created:
4 years, 7 months ago by hbos_chromium Modified:
4 years, 7 months ago CC:
chromium-reviews, mlamouri+watch-content_chromium.org, posciak+watch_chromium.org, jam, feature-media-reviews_chromium.org, darin-cc_chromium.org, mkwst+moarreviews-renderer_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionWebRTC: Launch ECDSA as default certificate.
Launch bug: https://bugs.chromium.org/p/chromium/issues/detail?id=601850
content_features.cc:
Flip the default value of WebRTC-EnableWebRtcEcdsa to enabled.
rtc_peer_connection_handler.cc:
Move "if flag" code into a helper function. DCHECK that KT_DEFAULT == KT_RSA to make the experiment's assumption about this explicit.
BUG=601850, 611698
Committed: https://crrev.com/cf97b45f5d1af1f0aa3dd6e76150c5c778c691d1
Cr-Commit-Position: refs/heads/master@{#394128}
Patch Set 1 #
Total comments: 2
Patch Set 2 : static_assert #Patch Set 3 : Removed static_assert #Patch Set 4 : static_assert again, rebase with master #
Messages
Total messages: 33 (14 generated)
Description was changed from ========== EC default and DCHECK KT_DEFAULT=KT_RSA BUG= ========== to ========== WebRTC: Launch ECDSA as default certificate. Launch bug: https://bugs.chromium.org/p/chromium/issues/detail?id=601850 content_features.cc: Flip the default value of WebRTC-EnableWebRtcEcdsa to enabled. rtc_peer_connection_handler.cc: Move "if flag" code into a helper function. DCHECK that KT_DEFAULT == KT_RSA to make the experiment's assumption about this explicit. BUG=601850, 611698 ==========
hbos@chromium.org changed reviewers: + nick@chromium.org, tommi@chromium.org
Please take a look tommi and nick.
On 2016/05/13 11:28:53, hbos_chromium wrote: > Please take a look tommi and nick. Note: As I'm writing this all bits except Launch-Legal have been flipped. Will hold of commit button until it has been flipped.
Description was changed from ========== WebRTC: Launch ECDSA as default certificate. Launch bug: https://bugs.chromium.org/p/chromium/issues/detail?id=601850 content_features.cc: Flip the default value of WebRTC-EnableWebRtcEcdsa to enabled. rtc_peer_connection_handler.cc: Move "if flag" code into a helper function. DCHECK that KT_DEFAULT == KT_RSA to make the experiment's assumption about this explicit. BUG=601850, 611698 ========== to ========== WebRTC: Launch ECDSA as default certificate. Launch bug: https://bugs.chromium.org/p/chromium/issues/detail?id=601850 content_features.cc: Flip the default value of WebRTC-EnableWebRtcEcdsa to enabled. rtc_peer_connection_handler.cc: Move "if flag" code into a helper function. DCHECK that KT_DEFAULT == KT_RSA to make the experiment's assumption about this explicit. BUG=601850, 611698 NOCOMMIT=True ==========
just checking - is this cl still up to date? https://codereview.chromium.org/1979553002/diff/1/content/renderer/media/rtc_... File content/renderer/media/rtc_peer_connection_handler.cc (right): https://codereview.chromium.org/1979553002/diff/1/content/renderer/media/rtc_... content/renderer/media/rtc_peer_connection_handler.cc:736: DCHECK(rtc::KT_DEFAULT == rtc::KT_RSA); Did you mean to do a static (compile time) assert here? If so, use static_assert().
On 2016/05/13 13:27:06, tommi-chrömium wrote: > just checking - is this cl still up to date? Yes, this is the one I want to launch with.
PTAL tommi, ncarter https://codereview.chromium.org/1979553002/diff/1/content/renderer/media/rtc_... File content/renderer/media/rtc_peer_connection_handler.cc (right): https://codereview.chromium.org/1979553002/diff/1/content/renderer/media/rtc_... content/renderer/media/rtc_peer_connection_handler.cc:736: DCHECK(rtc::KT_DEFAULT == rtc::KT_RSA); On 2016/05/13 13:27:06, tommi-chrömium wrote: > Did you mean to do a static (compile time) assert here? If so, use > static_assert(). Done.
lgtm
lgtm
The CQ bit was checked by hbos@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1979553002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1979553002/20001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: chromeos_amd64-generic_chromium_compile_only_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...) chromeos_x86-generic_chromium_compile_only_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...)
The CQ bit was checked by hbos@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1979553002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1979553002/40001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by hbos@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1979553002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1979553002/60001
FYI about all the CQ dry runs: I discovered a #if defined macro typo in the webrtc repo that made it fail. It has been addressed and rolled in, PS4 is the same as the patch set that was LGTM'd.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by tommi@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from nick@chromium.org, tommi@chromium.org Link to the patchset: https://codereview.chromium.org/1979553002/#ps60001 (title: "static_assert again, rebase with master")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1979553002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1979553002/60001
On 2016/05/17 13:26:25, commit-bot: I haz the power wrote: > Dry run: This issue passed the CQ dry run. Landing now so that we'll have some margin of time before the next cut to do some testing.
Message was sent while issue was closed.
Description was changed from ========== WebRTC: Launch ECDSA as default certificate. Launch bug: https://bugs.chromium.org/p/chromium/issues/detail?id=601850 content_features.cc: Flip the default value of WebRTC-EnableWebRtcEcdsa to enabled. rtc_peer_connection_handler.cc: Move "if flag" code into a helper function. DCHECK that KT_DEFAULT == KT_RSA to make the experiment's assumption about this explicit. BUG=601850, 611698 NOCOMMIT=True ========== to ========== WebRTC: Launch ECDSA as default certificate. Launch bug: https://bugs.chromium.org/p/chromium/issues/detail?id=601850 content_features.cc: Flip the default value of WebRTC-EnableWebRtcEcdsa to enabled. rtc_peer_connection_handler.cc: Move "if flag" code into a helper function. DCHECK that KT_DEFAULT == KT_RSA to make the experiment's assumption about this explicit. BUG=601850, 611698 NOCOMMIT=True ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== WebRTC: Launch ECDSA as default certificate. Launch bug: https://bugs.chromium.org/p/chromium/issues/detail?id=601850 content_features.cc: Flip the default value of WebRTC-EnableWebRtcEcdsa to enabled. rtc_peer_connection_handler.cc: Move "if flag" code into a helper function. DCHECK that KT_DEFAULT == KT_RSA to make the experiment's assumption about this explicit. BUG=601850, 611698 NOCOMMIT=True ========== to ========== WebRTC: Launch ECDSA as default certificate. Launch bug: https://bugs.chromium.org/p/chromium/issues/detail?id=601850 content_features.cc: Flip the default value of WebRTC-EnableWebRtcEcdsa to enabled. rtc_peer_connection_handler.cc: Move "if flag" code into a helper function. DCHECK that KT_DEFAULT == KT_RSA to make the experiment's assumption about this explicit. BUG=601850, 611698 NOCOMMIT=True Committed: https://crrev.com/cf97b45f5d1af1f0aa3dd6e76150c5c778c691d1 Cr-Commit-Position: refs/heads/master@{#394128} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/cf97b45f5d1af1f0aa3dd6e76150c5c778c691d1 Cr-Commit-Position: refs/heads/master@{#394128}
Message was sent while issue was closed.
On 2016/05/17 14:59:17, tommi-chrömium wrote: > On 2016/05/17 13:26:25, commit-bot: I haz the power wrote: > > Dry run: This issue passed the CQ dry run. > > Landing now so that we'll have some margin of time before the next cut to do > some testing. All bits have been flipped.
Message was sent while issue was closed.
Description was changed from ========== WebRTC: Launch ECDSA as default certificate. Launch bug: https://bugs.chromium.org/p/chromium/issues/detail?id=601850 content_features.cc: Flip the default value of WebRTC-EnableWebRtcEcdsa to enabled. rtc_peer_connection_handler.cc: Move "if flag" code into a helper function. DCHECK that KT_DEFAULT == KT_RSA to make the experiment's assumption about this explicit. BUG=601850, 611698 NOCOMMIT=True Committed: https://crrev.com/cf97b45f5d1af1f0aa3dd6e76150c5c778c691d1 Cr-Commit-Position: refs/heads/master@{#394128} ========== to ========== WebRTC: Launch ECDSA as default certificate. Launch bug: https://bugs.chromium.org/p/chromium/issues/detail?id=601850 content_features.cc: Flip the default value of WebRTC-EnableWebRtcEcdsa to enabled. rtc_peer_connection_handler.cc: Move "if flag" code into a helper function. DCHECK that KT_DEFAULT == KT_RSA to make the experiment's assumption about this explicit. BUG=601850, 611698 Committed: https://crrev.com/cf97b45f5d1af1f0aa3dd6e76150c5c778c691d1 Cr-Commit-Position: refs/heads/master@{#394128} ========== |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
