Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1972383002:
Make ECDSA the default certificate in WebRTC standalone (Closed)
DescriptionMakes ECDSA the default certificate to use (generated if no other certificates
are specified when constructing a peer connection, at:
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/webrtc/api/webrtcsessiondescriptionfactory.cc&q=webrtcsessiondescriptionfactory&sq=package:chromium&type=cs&l=191).
This does not affect WEBRTC_BUILD_CHROMIUM builds whose ECDSA launch is handled
separately: https://crbug.com/601850 (req. @chromium acc).
BUG=chromium:611698, webrtc:5795
Committed: https://crrev.com/8ae8ab4e2364821eb9b4c7cf4d9718e78f79432f
Cr-Commit-Position: refs/heads/master@{#12756}
Patch Set 1 : #
Total comments: 2
Patch Set 2 : Reordered #if #Messages
Total messages: 19 (12 generated)
Description was changed from ========== EC default if not chromium build BUG= ========== to ========== EC default if not chromium build Hold of landing until https://codereview.webrtc.org/1965313002/ has landed. BUG=chromium:611698 ==========
Description was changed from ========== EC default if not chromium build Hold of landing until https://codereview.webrtc.org/1965313002/ has landed. BUG=chromium:611698 ========== to ========== EC default if not chromium build Hold of landing until https://codereview.webrtc.org/1965313002/ has landed. BUG=chromium:611698, webrtc:5795 ==========
Patchset #1 (id:1) has been deleted
Description was changed from ========== EC default if not chromium build Hold of landing until https://codereview.webrtc.org/1965313002/ has landed. BUG=chromium:611698, webrtc:5795 ========== to ========== Makes ECDSA the default certificate to use (generated if no other certificates are specified when constructing a peer connection, at: https://code.google.com/p/chromium/codesearch#chromium/src/third_party/webrtc...). This does not affect WEBRTC_BUILD_CHROMIUM whose ECDSA launch is handled separately: https://crbug.com/601850. BUG=chromium:611698, webrtc:5795 ==========
hbos@webrtc.org changed reviewers: + hta@webrtc.org, tommi@webrtc.org
Please take a look tommi and hta.
Description was changed from ========== Makes ECDSA the default certificate to use (generated if no other certificates are specified when constructing a peer connection, at: https://code.google.com/p/chromium/codesearch#chromium/src/third_party/webrtc...). This does not affect WEBRTC_BUILD_CHROMIUM whose ECDSA launch is handled separately: https://crbug.com/601850. BUG=chromium:611698, webrtc:5795 ========== to ========== Makes ECDSA the default certificate to use (generated if no other certificates are specified when constructing a peer connection, at: https://code.google.com/p/chromium/codesearch#chromium/src/third_party/webrtc...). This does not affect WEBRTC_BUILD_CHROMIUM builds whose ECDSA launch is handled separately: https://crbug.com/601850. BUG=chromium:611698, webrtc:5795 ==========
Description was changed from ========== Makes ECDSA the default certificate to use (generated if no other certificates are specified when constructing a peer connection, at: https://code.google.com/p/chromium/codesearch#chromium/src/third_party/webrtc...). This does not affect WEBRTC_BUILD_CHROMIUM builds whose ECDSA launch is handled separately: https://crbug.com/601850. BUG=chromium:611698, webrtc:5795 ========== to ========== Makes ECDSA the default certificate to use (generated if no other certificates are specified when constructing a peer connection, at: https://code.google.com/p/chromium/codesearch#chromium/src/third_party/webrtc...). This does not affect WEBRTC_BUILD_CHROMIUM builds whose ECDSA launch is handled separately: https://crbug.com/601850 (req. @chromium acc). BUG=chromium:611698, webrtc:5795 ==========
tommi@webrtc.org changed reviewers: + jansson@webrtc.org
lgtm with one nit. +jansson for testing perspective. https://codereview.webrtc.org/1972383002/diff/20001/webrtc/base/sslidentity.h File webrtc/base/sslidentity.h (right): https://codereview.webrtc.org/1972383002/diff/20001/webrtc/base/sslidentity.h... webrtc/base/sslidentity.h:122: #if !defined(WEBRTC_BUILD_CHROMIUM) nit: do you mind switching this around so that we have the positive test first: #if defined(WEBRTC_BUILD_CHROMIUM) // TODO(hbos): ... ... #else KT_DEFAULT = KT_ECDSA #endif
Please take a look jansson. https://codereview.webrtc.org/1972383002/diff/20001/webrtc/base/sslidentity.h File webrtc/base/sslidentity.h (right): https://codereview.webrtc.org/1972383002/diff/20001/webrtc/base/sslidentity.h... webrtc/base/sslidentity.h:122: #if !defined(WEBRTC_BUILD_CHROMIUM) On 2016/05/13 13:20:49, tommi-webrtc wrote: > nit: do you mind switching this around so that we have the positive test first: > > #if defined(WEBRTC_BUILD_CHROMIUM) > // TODO(hbos): ... > ... > #else > KT_DEFAULT = KT_ECDSA > #endif Done.
On 2016/05/13 14:38:45, hbos wrote: > Please take a look jansson. > > https://codereview.webrtc.org/1972383002/diff/20001/webrtc/base/sslidentity.h > File webrtc/base/sslidentity.h (right): > > https://codereview.webrtc.org/1972383002/diff/20001/webrtc/base/sslidentity.h... > webrtc/base/sslidentity.h:122: #if !defined(WEBRTC_BUILD_CHROMIUM) > On 2016/05/13 13:20:49, tommi-webrtc wrote: > > nit: do you mind switching this around so that we have the positive test > first: > > > > #if defined(WEBRTC_BUILD_CHROMIUM) > > // TODO(hbos): ... > > ... > > #else > > KT_DEFAULT = KT_ECDSA > > #endif > > Done. LGTM from a testing perspective.
The CQ bit was checked by hbos@webrtc.org
The patchset sent to the CQ was uploaded after l-g-t-m from tommi@webrtc.org Link to the patchset: https://codereview.webrtc.org/1972383002/#ps40001 (title: "Reordered #if")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1972383002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1972383002/40001
Message was sent while issue was closed.
Description was changed from ========== Makes ECDSA the default certificate to use (generated if no other certificates are specified when constructing a peer connection, at: https://code.google.com/p/chromium/codesearch#chromium/src/third_party/webrtc...). This does not affect WEBRTC_BUILD_CHROMIUM builds whose ECDSA launch is handled separately: https://crbug.com/601850 (req. @chromium acc). BUG=chromium:611698, webrtc:5795 ========== to ========== Makes ECDSA the default certificate to use (generated if no other certificates are specified when constructing a peer connection, at: https://code.google.com/p/chromium/codesearch#chromium/src/third_party/webrtc...). This does not affect WEBRTC_BUILD_CHROMIUM builds whose ECDSA launch is handled separately: https://crbug.com/601850 (req. @chromium acc). BUG=chromium:611698, webrtc:5795 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Makes ECDSA the default certificate to use (generated if no other certificates are specified when constructing a peer connection, at: https://code.google.com/p/chromium/codesearch#chromium/src/third_party/webrtc...). This does not affect WEBRTC_BUILD_CHROMIUM builds whose ECDSA launch is handled separately: https://crbug.com/601850 (req. @chromium acc). BUG=chromium:611698, webrtc:5795 ========== to ========== Makes ECDSA the default certificate to use (generated if no other certificates are specified when constructing a peer connection, at: https://code.google.com/p/chromium/codesearch#chromium/src/third_party/webrtc...). This does not affect WEBRTC_BUILD_CHROMIUM builds whose ECDSA launch is handled separately: https://crbug.com/601850 (req. @chromium acc). BUG=chromium:611698, webrtc:5795 Committed: https://crrev.com/8ae8ab4e2364821eb9b4c7cf4d9718e78f79432f Cr-Commit-Position: refs/heads/master@{#12756} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/8ae8ab4e2364821eb9b4c7cf4d9718e78f79432f Cr-Commit-Position: refs/heads/master@{#12756} |
