Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(1)

Issue 1176383004: DtlsIdentityStore[Interface/Impl] updated, DtlsIdentityService to be removed (Closed)

Created:
4 years ago by hbos
Modified:
3 years, 10 months ago
CC:
Andrew MacDonald, interface-changes_webrtc.org, niklas.enbom, qiang.lu, rwolff_gocast.it, tterriberry_mozilla.com, webrtc-reviews_webrtc.org, yujie_mao (webrtc)
Base URL:
https://chromium.googlesource.com/external/webrtc.git@master
Target Ref:
refs/pending/heads/master
Project:
webrtc
Visibility:
Public.

Description

DtlsIdentityStoreInterface added and the implementation is called DtlsIdentityStoreImpl (previously named without the -Impl bit and without an interface). DtlsIdentityStoreImpl is updated to take KeyType into account, something which will be relevant after this CL lands: https://codereview.webrtc.org/1189583002 The DtlsIdentityService[Interface] classes are about to be removed (to be removed when Chromium no longer implements and uses the interface). This was an unnecessary layer of complexity. The FakeIdentityService is now instead a FakeDtlsIdentityStore. Where a service was previously passed around, a store is now passed around. Identity generation is now commonly performed using DtlsIdentityStoreInterface. Previously, if a service was not specified, WebRtcSessionDescriptionFactory could fall back on its own generation code. Now, a store has to be provided for generation to occur. For more information about the steps being taken to land this without breaking Chromium, see referenced bug. BUG=webrtc:4899 R=magjed@webrtc.org, tommi@webrtc.org Committed: https://chromium.googlesource.com/external/webrtc/+/5e56c5927e097f095aef2e9f7be49fd3d59221e1

Patch Set 1 : Remove store service, replace with store interface/impl (store impl significantly changed again in PS4) #

Patch Set 2 : Del old requestobserver & service interface (forgot), made global common_name_ const char* #

Total comments: 8

Patch Set 3 : Taking ownership of store and ownership explicit with use of scoped_ptr #

Patch Set 4 : DtlsIdentityStoreImpl made simpler and better and not using any locks. #

Total comments: 1

Patch Set 5 : Making Win compile #

Patch Set 6 : Fixed dtlsidentitystore_unittest and made RequestIdentity etc take scoped_refptr instead of ptr #

Total comments: 35

Patch Set 7 : Addressing most of tommi's comments #

Patch Set 8 : Addressed rest of tommi's comments: Removed need for lock #

Total comments: 47

Patch Set 9 : Fixed message data leak in OnMessage introduced in PS8 #

Patch Set 10 : Addressed tommi's comments #

Total comments: 2

Patch Set 11 : tommi's nits #

Patch Set 12 : Merge with master #

Total comments: 2

Patch Set 13 : queue instead of list & merge with master #

Total comments: 1

Patch Set 14 : Merge w master AFTER the landing of 1268363002. "CreatePC(service,store)" using store instead of service. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+434 lines, -585 lines) Patch
D talk/app/webrtc/dtlsidentityservice.h View 1 chunk +0 lines, -59 lines 0 comments Download
D talk/app/webrtc/dtlsidentityservice.cc View 1 chunk +0 lines, -50 lines 0 comments Download
M talk/app/webrtc/dtlsidentitystore.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +53 lines, -32 lines 0 comments Download
M talk/app/webrtc/dtlsidentitystore.cc View 1 2 3 4 5 6 7 8 9 10 11 12 3 chunks +130 lines, -114 lines 0 comments Download
M talk/app/webrtc/dtlsidentitystore_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 4 chunks +40 lines, -17 lines 0 comments Download
M talk/app/webrtc/peerconnection.h View 1 2 3 4 5 6 7 8 9 10 11 3 chunks +2 lines, -9 lines 0 comments Download
M talk/app/webrtc/peerconnection.cc View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +2 lines, -2 lines 0 comments Download
M talk/app/webrtc/peerconnection_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +9 lines, -8 lines 0 comments Download
M talk/app/webrtc/peerconnectionfactory.h View 1 2 3 4 5 6 7 8 9 3 chunks +9 lines, -5 lines 0 comments Download
M talk/app/webrtc/peerconnectionfactory.cc View 1 2 3 4 5 6 7 8 9 6 chunks +37 lines, -12 lines 0 comments Download
M talk/app/webrtc/peerconnectionfactory_unittest.cc View 1 2 10 chunks +35 lines, -18 lines 0 comments Download
M talk/app/webrtc/peerconnectionfactoryproxy.h View 1 2 3 4 5 6 7 8 9 10 2 chunks +24 lines, -7 lines 0 comments Download
M talk/app/webrtc/peerconnectioninterface.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 5 chunks +12 lines, -7 lines 0 comments Download
M talk/app/webrtc/peerconnectioninterface_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +5 lines, -5 lines 0 comments Download
D talk/app/webrtc/test/fakedtlsidentityservice.h View 1 chunk +0 lines, -136 lines 0 comments Download
A + talk/app/webrtc/test/fakedtlsidentitystore.h View 1 2 3 4 5 6 7 8 9 4 chunks +26 lines, -40 lines 0 comments Download
M talk/app/webrtc/test/peerconnectiontestwrapper.h View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M talk/app/webrtc/test/peerconnectiontestwrapper.cc View 1 2 2 chunks +5 lines, -4 lines 0 comments Download
M talk/app/webrtc/webrtcsession.h View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +2 lines, -1 line 0 comments Download
M talk/app/webrtc/webrtcsession.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 3 chunks +4 lines, -4 lines 0 comments Download
M talk/app/webrtc/webrtcsession_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 5 chunks +15 lines, -13 lines 0 comments Download
M talk/app/webrtc/webrtcsessiondescriptionfactory.h View 1 2 3 4 5 6 7 8 9 10 11 4 chunks +9 lines, -9 lines 0 comments Download
M talk/app/webrtc/webrtcsessiondescriptionfactory.cc View 1 2 3 4 5 6 7 8 9 10 11 8 chunks +11 lines, -28 lines 0 comments Download
M talk/libjingle.gyp View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +0 lines, -2 lines 0 comments Download
M talk/libjingle_tests.gyp View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +1 line, -1 line 0 comments Download
M webrtc/base/sslidentity.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 2 chunks +2 lines, -2 lines 0 comments Download

Messages

Total messages: 21 (4 generated)
hbos
The DtlsIdentityStore-related changes of https://codereview.webrtc.org/1151943005/. https://codereview.webrtc.org/1176383004/diff/2/talk/app/webrtc/dtlsidentitystore.cc File talk/app/webrtc/dtlsidentitystore.cc (right): https://codereview.webrtc.org/1176383004/diff/2/talk/app/webrtc/dtlsidentitystore.cc#newcode76 talk/app/webrtc/dtlsidentitystore.cc:76: // to generate one. ...
4 years ago (2015-06-15 10:10:34 UTC) #2
hbos
https://codereview.webrtc.org/1176383004/diff/2/talk/app/webrtc/peerconnectioninterface.h File talk/app/webrtc/peerconnectioninterface.h (right): https://codereview.webrtc.org/1176383004/diff/2/talk/app/webrtc/peerconnectioninterface.h#newcode497 talk/app/webrtc/peerconnectioninterface.h:497: // This method takes the ownership of |dtls_identity_service|. Should ...
4 years ago (2015-06-16 07:26:25 UTC) #3
hbos
PTAL tommi and rock on. https://codereview.webrtc.org/1176383004/diff/2/talk/app/webrtc/dtlsidentitystore.cc File talk/app/webrtc/dtlsidentitystore.cc (right): https://codereview.webrtc.org/1176383004/diff/2/talk/app/webrtc/dtlsidentitystore.cc#newcode76 talk/app/webrtc/dtlsidentitystore.cc:76: // to generate one. ...
4 years ago (2015-06-18 08:43:13 UTC) #5
hbos
PTAL magjed, maybe you can review and then help me land this? I'm OOO for ...
4 years ago (2015-06-18 09:52:13 UTC) #7
tommi
https://codereview.webrtc.org/1176383004/diff/110001/talk/app/webrtc/dtlsidentitystore.cc File talk/app/webrtc/dtlsidentitystore.cc (right): https://codereview.webrtc.org/1176383004/diff/110001/talk/app/webrtc/dtlsidentitystore.cc#newcode77 talk/app/webrtc/dtlsidentitystore.cc:77: new IdentityResult(key_type_, identity.release())); would be better to do identity.Pass() ...
4 years ago (2015-06-18 13:55:36 UTC) #8
hbos
I addressed almost all of the comments. Vacation mode activate! https://codereview.webrtc.org/1176383004/diff/110001/talk/app/webrtc/dtlsidentitystore.cc File talk/app/webrtc/dtlsidentitystore.cc (right): https://codereview.webrtc.org/1176383004/diff/110001/talk/app/webrtc/dtlsidentitystore.cc#newcode77 ...
4 years ago (2015-06-18 15:45:48 UTC) #9
hbos
PTAL tommi https://codereview.webrtc.org/1176383004/diff/110001/talk/app/webrtc/dtlsidentitystore.cc File talk/app/webrtc/dtlsidentitystore.cc (right): https://codereview.webrtc.org/1176383004/diff/110001/talk/app/webrtc/dtlsidentitystore.cc#newcode78 talk/app/webrtc/dtlsidentitystore.cc:78: signaling_thread_->Post(store_, MSG_GENERATE_IDENTITY_RESULT, msg); On 2015/06/18 15:45:48, hbos ...
3 years, 11 months ago (2015-07-02 09:32:23 UTC) #10
tommi
https://codereview.webrtc.org/1176383004/diff/170001/talk/app/webrtc/dtlsidentitystore.cc File talk/app/webrtc/dtlsidentitystore.cc (right): https://codereview.webrtc.org/1176383004/diff/170001/talk/app/webrtc/dtlsidentitystore.cc#newcode68 talk/app/webrtc/dtlsidentitystore.cc:68: void GenerateIdentity_w() { can we dcheck thread correctness? https://codereview.webrtc.org/1176383004/diff/170001/talk/app/webrtc/dtlsidentitystore.cc#newcode70 ...
3 years, 11 months ago (2015-07-02 11:23:00 UTC) #12
hbos
PTAL again, tommi https://codereview.webrtc.org/1176383004/diff/170001/talk/app/webrtc/dtlsidentitystore.cc File talk/app/webrtc/dtlsidentitystore.cc (right): https://codereview.webrtc.org/1176383004/diff/170001/talk/app/webrtc/dtlsidentitystore.cc#newcode68 talk/app/webrtc/dtlsidentitystore.cc:68: void GenerateIdentity_w() { On 2015/07/02 11:22:59, ...
3 years, 11 months ago (2015-07-02 12:28:28 UTC) #13
tommi
looks good (nice work!). minor request for ThreadChecker. https://codereview.webrtc.org/1176383004/diff/170001/talk/app/webrtc/dtlsidentitystore.cc File talk/app/webrtc/dtlsidentitystore.cc (right): https://codereview.webrtc.org/1176383004/diff/170001/talk/app/webrtc/dtlsidentitystore.cc#newcode68 talk/app/webrtc/dtlsidentitystore.cc:68: void ...
3 years, 11 months ago (2015-07-09 16:13:57 UTC) #14
hbos
PTAL magjed, tommi https://codereview.webrtc.org/1176383004/diff/170001/talk/app/webrtc/dtlsidentitystore.cc File talk/app/webrtc/dtlsidentitystore.cc (right): https://codereview.webrtc.org/1176383004/diff/170001/talk/app/webrtc/dtlsidentitystore.cc#newcode68 talk/app/webrtc/dtlsidentitystore.cc:68: void GenerateIdentity_w() { On 2015/07/09 16:13:56, ...
3 years, 10 months ago (2015-07-27 13:29:00 UTC) #15
magjed_webrtc
I only have a shallow understanding of this, but it lgtm. https://codereview.webrtc.org/1176383004/diff/250001/talk/app/webrtc/dtlsidentitystore.h File talk/app/webrtc/dtlsidentitystore.h (right): ...
3 years, 10 months ago (2015-07-29 09:31:57 UTC) #16
tommi
https://codereview.webrtc.org/1176383004/diff/170001/talk/app/webrtc/dtlsidentitystore.cc File talk/app/webrtc/dtlsidentitystore.cc (right): https://codereview.webrtc.org/1176383004/diff/170001/talk/app/webrtc/dtlsidentitystore.cc#newcode68 talk/app/webrtc/dtlsidentitystore.cc:68: void GenerateIdentity_w() { On 2015/07/27 13:29:00, hbos wrote: > ...
3 years, 10 months ago (2015-08-04 10:47:20 UTC) #17
hbos
https://codereview.webrtc.org/1176383004/diff/170001/talk/app/webrtc/dtlsidentitystore.cc File talk/app/webrtc/dtlsidentitystore.cc (right): https://codereview.webrtc.org/1176383004/diff/170001/talk/app/webrtc/dtlsidentitystore.cc#newcode68 talk/app/webrtc/dtlsidentitystore.cc:68: void GenerateIdentity_w() { On 2015/08/04 10:47:20, tommi (webrtc) wrote: ...
3 years, 10 months ago (2015-08-04 11:38:49 UTC) #18
hbos
A question for ya, tommi. https://codereview.webrtc.org/1176383004/diff/170001/talk/app/webrtc/dtlsidentitystore.cc File talk/app/webrtc/dtlsidentitystore.cc (right): https://codereview.webrtc.org/1176383004/diff/170001/talk/app/webrtc/dtlsidentitystore.cc#newcode68 talk/app/webrtc/dtlsidentitystore.cc:68: void GenerateIdentity_w() { On ...
3 years, 10 months ago (2015-08-04 12:16:19 UTC) #19
tommi
On 2015/08/04 12:16:19, hbos wrote: > A question for ya, tommi. > > https://codereview.webrtc.org/1176383004/diff/170001/talk/app/webrtc/dtlsidentitystore.cc > ...
3 years, 10 months ago (2015-08-04 12:28:26 UTC) #20
hbos
3 years, 10 months ago (2015-08-11 08:33:33 UTC) #21
Message was sent while issue was closed.
Committed patchset #14 (id:290001) manually as
5e56c5927e097f095aef2e9f7be49fd3d59221e1 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698