Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(567)

Issue 2295493002: Add a switch to redetermine role when ICE restarts. (Closed)

Created:
4 years, 3 months ago by honghaiz3
Modified:
4 years, 3 months ago
Reviewers:
pthatcher1
CC:
webrtc-reviews_webrtc.org, tterriberry_mozilla.com
Target Ref:
refs/pending/heads/master
Project:
webrtc
Visibility:
Public.

Description

Patch Set 1 : . #

Total comments: 8

Patch Set 2 : Address comments #

Total comments: 4

Patch Set 3 : Make redetermine_role_on_ice_restart default to false #

Patch Set 4 : Add NativeConfiguration method in RTCConfiguration #

Total comments: 4

Patch Set 5 : Address Peter's comments #

Total comments: 2

Patch Set 6 : Changed ExperimentalConfiguration to AggressiveConfiguration #

Patch Set 7 : Remove the objective-c change to submit #

Unified diffs Side-by-side diffs Delta from patch set Stats (+32 lines, -8 lines) Patch
M webrtc/api/android/jni/peerconnection_jni.cc View 1 2 3 4 5 2 chunks +4 lines, -2 lines 0 comments Download
M webrtc/api/peerconnection.cc View 1 chunk +3 lines, -1 line 0 comments Download
M webrtc/api/peerconnectionfactory.h View 1 chunk +2 lines, -1 line 0 comments Download
M webrtc/api/peerconnectionfactory.cc View 1 chunk +4 lines, -2 lines 0 comments Download
M webrtc/api/peerconnectioninterface.h View 1 2 3 4 5 2 chunks +15 lines, -0 lines 0 comments Download
M webrtc/api/peerconnectioninterface_unittest.cc View 1 chunk +4 lines, -2 lines 0 comments Download

Messages

Total messages: 26 (10 generated)
honghaiz3
4 years, 3 months ago (2016-08-29 21:07:21 UTC) #4
pthatcher1
https://codereview.webrtc.org/2295493002/diff/40001/webrtc/api/peerconnectionfactory.cc File webrtc/api/peerconnectionfactory.cc (right): https://codereview.webrtc.org/2295493002/diff/40001/webrtc/api/peerconnectionfactory.cc#newcode317 webrtc/api/peerconnectionfactory.cc:317: redetermine_role_on_ice_restart); Don't you need to change transportcontroller.h also? I ...
4 years, 3 months ago (2016-08-29 22:18:07 UTC) #5
honghaiz3
https://codereview.webrtc.org/2295493002/diff/40001/webrtc/api/peerconnectionfactory.cc File webrtc/api/peerconnectionfactory.cc (right): https://codereview.webrtc.org/2295493002/diff/40001/webrtc/api/peerconnectionfactory.cc#newcode317 webrtc/api/peerconnectionfactory.cc:317: redetermine_role_on_ice_restart); On 2016/08/29 22:18:07, pthatcher1 wrote: > Don't you ...
4 years, 3 months ago (2016-08-29 23:16:59 UTC) #6
pthatcher1
https://codereview.webrtc.org/2295493002/diff/40001/webrtc/api/peerconnectionfactory.cc File webrtc/api/peerconnectionfactory.cc (right): https://codereview.webrtc.org/2295493002/diff/40001/webrtc/api/peerconnectionfactory.cc#newcode317 webrtc/api/peerconnectionfactory.cc:317: redetermine_role_on_ice_restart); On 2016/08/29 23:16:59, honghaiz3 wrote: > On 2016/08/29 ...
4 years, 3 months ago (2016-08-29 23:27:42 UTC) #7
honghaiz3
PTAL. https://codereview.webrtc.org/2295493002/diff/60001/webrtc/api/android/jni/peerconnection_jni.cc File webrtc/api/android/jni/peerconnection_jni.cc (right): https://codereview.webrtc.org/2295493002/diff/60001/webrtc/api/android/jni/peerconnection_jni.cc#newcode1634 webrtc/api/android/jni/peerconnection_jni.cc:1634: jfieldID j_redetermine_role_on_ice_restart_id = On 2016/08/29 23:27:42, pthatcher1 wrote: ...
4 years, 3 months ago (2016-08-29 23:32:27 UTC) #8
honghaiz3
I think we can also add a new method like RtcConfiguration::NativeRtcConfiguration to return default configs ...
4 years, 3 months ago (2016-08-30 01:33:43 UTC) #9
honghaiz3
Patch 4 is the one based on my last suggestion. It will make it easier ...
4 years, 3 months ago (2016-08-30 19:59:51 UTC) #10
pthatcher1
https://codereview.webrtc.org/2295493002/diff/100001/webrtc/api/peerconnectioninterface.h File webrtc/api/peerconnectioninterface.h (right): https://codereview.webrtc.org/2295493002/diff/100001/webrtc/api/peerconnectioninterface.h#newcode243 webrtc/api/peerconnectioninterface.h:243: static RTCConfiguration NativeConfiguration() { Can we call this OptimalConfiguration()? ...
4 years, 3 months ago (2016-08-30 20:25:36 UTC) #11
honghaiz3
https://codereview.webrtc.org/2295493002/diff/100001/webrtc/api/peerconnectioninterface.h File webrtc/api/peerconnectioninterface.h (right): https://codereview.webrtc.org/2295493002/diff/100001/webrtc/api/peerconnectioninterface.h#newcode243 webrtc/api/peerconnectioninterface.h:243: static RTCConfiguration NativeConfiguration() { On 2016/08/30 20:25:36, pthatcher1 wrote: ...
4 years, 3 months ago (2016-08-30 21:32:59 UTC) #12
pthatcher1
lgtm, assuming you change the name https://codereview.webrtc.org/2295493002/diff/120001/webrtc/api/peerconnectioninterface.h File webrtc/api/peerconnectioninterface.h (right): https://codereview.webrtc.org/2295493002/diff/120001/webrtc/api/peerconnectioninterface.h#newcode248 webrtc/api/peerconnectioninterface.h:248: // may be ...
4 years, 3 months ago (2016-08-30 23:47:01 UTC) #13
honghaiz3
https://codereview.webrtc.org/2295493002/diff/120001/webrtc/api/peerconnectioninterface.h File webrtc/api/peerconnectioninterface.h (right): https://codereview.webrtc.org/2295493002/diff/120001/webrtc/api/peerconnectioninterface.h#newcode248 webrtc/api/peerconnectioninterface.h:248: // may be riskier and may need extra support ...
4 years, 3 months ago (2016-08-31 00:02:58 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/2295493002/140001
4 years, 3 months ago (2016-08-31 00:03:16 UTC) #17
commit-bot: I haz the power
Try jobs failed on following builders: presubmit on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/presubmit/builds/7860)
4 years, 3 months ago (2016-08-31 00:23:41 UTC) #19
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/2295493002/160001
4 years, 3 months ago (2016-08-31 04:49:19 UTC) #22
commit-bot: I haz the power
Patchset 7 (id:??) landed as https://crrev.com/bfd398ccda27550629ec2440888f4083e4510069 Cr-Commit-Position: refs/heads/master@{#13982}
4 years, 3 months ago (2016-08-31 05:08:03 UTC) #25
honghaiz3
4 years, 3 months ago (2016-08-31 05:08:04 UTC) #26
Message was sent while issue was closed.
Committed patchset #7 (id:160001) manually as
bfd398ccda27550629ec2440888f4083e4510069 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698