New issue
Advanced search Search tips

Issue 906029 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 29
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocking:
issue 902678



Sign in to add a comment

Add metrics to find out how far PCs come in negotiation before failing

Project Member Reported by hbos@chromium.org, Nov 16

Issue description

This will be used to find out more about SDP negotiation failures, especially in comparison between Plan B and Unified Plan usage.

The offerer for example goes through:

createOffer()
setLocalDescription(offer)
setRemoteDescription(answer)

And the answerer goes through:

setRemoteDescription(offer)
createAnswer()
setLocalDescription(answer)

In both cases, each operation has a "pending", "resolved" or "rejected" state, where you won't be able to go to the next step if "rejected" happens.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Nov 20

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/87866bee9d6f4e9c20c6c528059ea2dfc22cc38c

commit 87866bee9d6f4e9c20c6c528059ea2dfc22cc38c
Author: Henrik Boström <hbos@chromium.org>
Date: Tue Nov 20 13:53:24 2018

CallSetupStateTracker and unittests added.

This helper class keeps track of an offerer state and an answerer state
and makes sure that the transitions between different states are valid.
By having a single state that reports whether the call setup was
successful or if (and in which step) it failed we can be sure to get
trustworthy comparisons between % success and % failures, which is
important for the Unified Plan experiments.

Follow-up CL(s) will wire up RTCPeerConnection to use this helper class
and make sure the states are reported with UMA.

Bug:  906029 
Change-Id: Iafb77533c66c7625269e3c6e33ef954d88e54cdb
Reviewed-on: https://chromium-review.googlesource.com/c/1340299
Reviewed-by: Philip Jägenstedt <foolip@chromium.org>
Reviewed-by: Harald Alvestrand <hta@chromium.org>
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Commit-Queue: Henrik Boström <hbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609685}
[modify] https://crrev.com/87866bee9d6f4e9c20c6c528059ea2dfc22cc38c/third_party/blink/renderer/modules/BUILD.gn
[modify] https://crrev.com/87866bee9d6f4e9c20c6c528059ea2dfc22cc38c/third_party/blink/renderer/modules/peerconnection/BUILD.gn
[add] https://crrev.com/87866bee9d6f4e9c20c6c528059ea2dfc22cc38c/third_party/blink/renderer/modules/peerconnection/call_setup_state_tracker.cc
[add] https://crrev.com/87866bee9d6f4e9c20c6c528059ea2dfc22cc38c/third_party/blink/renderer/modules/peerconnection/call_setup_state_tracker.h
[add] https://crrev.com/87866bee9d6f4e9c20c6c528059ea2dfc22cc38c/third_party/blink/renderer/modules/peerconnection/call_setup_state_tracker_unittest.cc

Project Member

Comment 2 by bugdroid1@chromium.org, Nov 21

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/24d90b037ea13721420fac65d82c107021898d5a

commit 24d90b037ea13721420fac65d82c107021898d5a
Author: Guido Urdaneta <guidou@chromium.org>
Date: Wed Nov 21 10:52:44 2018

Add UserMediaController::HasRequestedUserMedia()

This is intended to make it easier to detect if an RTCPeerConnection is used
together with getUserMedia() or getDisplayMedia() for real-time audio/video.

Bug:  906029 
Change-Id: I440fc93567bd9a490047c2bcf3bf5641a4ff3dfa
Reviewed-on: https://chromium-review.googlesource.com/c/1344101
Reviewed-by: Henrik Boström <hbos@chromium.org>
Commit-Queue: Guido Urdaneta <guidou@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609993}
[modify] https://crrev.com/24d90b037ea13721420fac65d82c107021898d5a/third_party/blink/renderer/modules/mediastream/user_media_controller.h

Project Member

Comment 3 by bugdroid1@chromium.org, Nov 21

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7b1a3d2d1fcc43cdcbbad07dfb93f3e205157df6

commit 7b1a3d2d1fcc43cdcbbad07dfb93f3e205157df6
Author: Henrik Boström <hbos@chromium.org>
Date: Wed Nov 21 14:57:02 2018

CallSetupStateTracker::CallSetupState() added.

CallSetupState simplifies the OffererState/AnswererState to:
- kNotStarted
- kStarted
- kFailed
- kSucceeded

This is allows an RTCPeerConnection to be given a single label,
instead of one label per role (offerer, answerer) and will make
it easier to estimate % success/failure. (This is roughly what the
Alternative proposal from the CallSetupStateTracker design doc is).

Bug:  906029 
Change-Id: I8d0c673d4fa70403e6a552e9f76f9b4b3c10e5d4
Reviewed-on: https://chromium-review.googlesource.com/c/1344051
Commit-Queue: Henrik Boström <hbos@chromium.org>
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Reviewed-by: Harald Alvestrand <hta@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610050}
[modify] https://crrev.com/7b1a3d2d1fcc43cdcbbad07dfb93f3e205157df6/third_party/blink/renderer/modules/peerconnection/call_setup_state_tracker.cc
[modify] https://crrev.com/7b1a3d2d1fcc43cdcbbad07dfb93f3e205157df6/third_party/blink/renderer/modules/peerconnection/call_setup_state_tracker.h
[modify] https://crrev.com/7b1a3d2d1fcc43cdcbbad07dfb93f3e205157df6/third_party/blink/renderer/modules/peerconnection/call_setup_state_tracker_unittest.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Nov 27

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/061e2a06f26a395a8005ddcfe17a4397c8c1978c

commit 061e2a06f26a395a8005ddcfe17a4397c8c1978c
Author: Henrik Boström <hbos@chromium.org>
Date: Tue Nov 27 13:33:10 2018

RTCPeerConnection wired up to use CallSetupStateTracker.

This makes the RTCPeerConnection aware, through the help of the tracker,
what its call setup state is. This is confirmed by unittests.

UMA metrics and trigger point for collecting this will be added in a
follow-up CL.

Bug:  906029 
Change-Id: Ib71ec03d7560c5ee9ac65fd5427b297808e8c4fa
Reviewed-on: https://chromium-review.googlesource.com/c/1349340
Reviewed-by: Harald Alvestrand <hta@chromium.org>
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Commit-Queue: Henrik Boström <hbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611088}
[modify] https://crrev.com/061e2a06f26a395a8005ddcfe17a4397c8c1978c/third_party/blink/renderer/modules/peerconnection/BUILD.gn
[modify] https://crrev.com/061e2a06f26a395a8005ddcfe17a4397c8c1978c/third_party/blink/renderer/modules/peerconnection/rtc_peer_connection.cc
[modify] https://crrev.com/061e2a06f26a395a8005ddcfe17a4397c8c1978c/third_party/blink/renderer/modules/peerconnection/rtc_peer_connection.h
[modify] https://crrev.com/061e2a06f26a395a8005ddcfe17a4397c8c1978c/third_party/blink/renderer/modules/peerconnection/rtc_peer_connection_test.cc
[add] https://crrev.com/061e2a06f26a395a8005ddcfe17a4397c8c1978c/third_party/blink/renderer/modules/peerconnection/rtc_session_description_enums.h
[modify] https://crrev.com/061e2a06f26a395a8005ddcfe17a4397c8c1978c/third_party/blink/renderer/modules/peerconnection/rtc_session_description_request_impl.cc
[modify] https://crrev.com/061e2a06f26a395a8005ddcfe17a4397c8c1978c/third_party/blink/renderer/modules/peerconnection/rtc_session_description_request_impl.h
[modify] https://crrev.com/061e2a06f26a395a8005ddcfe17a4397c8c1978c/third_party/blink/renderer/modules/peerconnection/rtc_session_description_request_promise_impl.cc
[modify] https://crrev.com/061e2a06f26a395a8005ddcfe17a4397c8c1978c/third_party/blink/renderer/modules/peerconnection/rtc_session_description_request_promise_impl.h
[modify] https://crrev.com/061e2a06f26a395a8005ddcfe17a4397c8c1978c/third_party/blink/renderer/modules/peerconnection/rtc_void_request_impl.cc
[modify] https://crrev.com/061e2a06f26a395a8005ddcfe17a4397c8c1978c/third_party/blink/renderer/modules/peerconnection/rtc_void_request_impl.h
[modify] https://crrev.com/061e2a06f26a395a8005ddcfe17a4397c8c1978c/third_party/blink/renderer/modules/peerconnection/rtc_void_request_promise_impl.cc
[modify] https://crrev.com/061e2a06f26a395a8005ddcfe17a4397c8c1978c/third_party/blink/renderer/modules/peerconnection/rtc_void_request_promise_impl.h
[modify] https://crrev.com/061e2a06f26a395a8005ddcfe17a4397c8c1978c/third_party/blink/renderer/platform/testing/testing_platform_support_with_web_rtc.h

Project Member

Comment 5 by bugdroid1@chromium.org, Nov 28

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/23592b26eab7f41c5f67fd4b46c78022a866a8cd

commit 23592b26eab7f41c5f67fd4b46c78022a866a8cd
Author: Henrik Boström <hbos@chromium.org>
Date: Wed Nov 28 17:29:11 2018

Add UMA metrics for CallSetupStateTracker enums.

The state tracker is already well-tested. What is new is reporting the
existing states. This is done at Dispose()-time because we cannot
generally speaking know when an RTCPeerConnection is done with the call
setup. We cannot trust close() is called because not all apps call it,
but even if they do this will not happen if the user closes the tab.
Dispose() will happen even if the tab is closed even if close() was
never called, though it only happens when the GC decides to clean things
up (which may happen at a much later time, and which does not happen if
all of Chrome is shut down). Making the reporting independent of any API
calls is a good thing to do regardless since this should reduce the risk
of additional sampling bias.

There are two flavors of the enums: One that always reports, and one that
only reports if the RTCPeerConnection belongs to a document that has ever
performed getUserMedia(). This is a heuristic for "is this a media use
case?" that does not depend on looking at the SDP.

Because Dispose() most likely happens after the context/document is
destroyed, the flag for if a document uses media is stored within the
CallSetupStateTracker, and is updated along side other state changes.

Bug:  906029 
Change-Id: Icf6b983119ef52f3cf9901695968bc3236073cd5
Reviewed-on: https://chromium-review.googlesource.com/c/1352257
Reviewed-by: Jesse Doherty <jwd@chromium.org>
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Commit-Queue: Henrik Boström <hbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611752}
[modify] https://crrev.com/23592b26eab7f41c5f67fd4b46c78022a866a8cd/third_party/blink/renderer/modules/peerconnection/call_setup_state_tracker.cc
[modify] https://crrev.com/23592b26eab7f41c5f67fd4b46c78022a866a8cd/third_party/blink/renderer/modules/peerconnection/call_setup_state_tracker.h
[modify] https://crrev.com/23592b26eab7f41c5f67fd4b46c78022a866a8cd/third_party/blink/renderer/modules/peerconnection/call_setup_state_tracker_unittest.cc
[modify] https://crrev.com/23592b26eab7f41c5f67fd4b46c78022a866a8cd/third_party/blink/renderer/modules/peerconnection/rtc_peer_connection.cc
[modify] https://crrev.com/23592b26eab7f41c5f67fd4b46c78022a866a8cd/third_party/blink/renderer/modules/peerconnection/rtc_peer_connection.h
[modify] https://crrev.com/23592b26eab7f41c5f67fd4b46c78022a866a8cd/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/23592b26eab7f41c5f67fd4b46c78022a866a8cd/tools/metrics/histograms/histograms.xml

Labels: M-72
Status: Fixed (was: Started)
Blocking: -857003 902678

Sign in to add a comment