Media Remoting: UMAs to track session events and measurements.

Media Remoting: UMAs to track session events and measurements.

Adds metrics that track when sessions start and stop, and why; and how
long they lasted; and coarsely-grained stats about codecs/profiles that
were being used. This will help us track that the system is performing
as expected in upcoming trials.

This change also added a lot of missing error-reporting structure, to
properly account for all possible start and end triggers throughout the
implementation. In some places, small tweaks to shutdown-on-error logic
were made.

Furthermore, a minor change to the fullscreen exit control logic was
made (since the histograms revealed a subtle flaw that would prevent
exiting remoting in some cases where the user's action requested it).

BUG=643964
TEST=Manual testing of several different media remoting scenarios, and checking chrome://histograms to confirm correct measurement values and event tracking.

Review-Url: https://codereview.chromium.org/2631993002
Cr-Commit-Position: refs/heads/master@{#444230}
Committed: https://chromium.googlesource.com/chromium/src/+/5949e051331c74dc1bc140b609a785e491858dd4

[miu, 2017-01-15 02:09:36]

The CQ bit was checked by miu@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-15 02:09:40]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[miu, 2017-01-15 02:11:08]

miu@chromium.org changed reviewers:
+ holte@chromium.org, xjz@chromium.org

[miu, 2017-01-15 02:11:09]

holte: histograms.xml RS please.

xjz: PTAL.

[commit-bot: I haz the power, 2017-01-15 02:22:12]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-15 02:22:13]

Dry run: Try jobs failed on following builders:
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)

[miu, 2017-01-16 21:54:22]

Description was changed from

==========
Media Remoting: UMAs to track session events and measurements.

Adds metrics that track when sessions start and stop, and why; and how
long they lasted; and coarsely-grained stats about codecs/profiles that
were being used. This will help us track that the system is performing
as expected in upcoming trials.

This change also added a lot of missing error-reporting structure, to
properly account for all possible start and end triggers throughout the
implementation. In some places, small tweaks to shutdown-on-error logic
were made.

BUG=643964
==========

to

==========
Media Remoting: UMAs to track session events and measurements.

Adds metrics that track when sessions start and stop, and why; and how
long they lasted; and coarsely-grained stats about codecs/profiles that
were being used. This will help us track that the system is performing
as expected in upcoming trials.

This change also added a lot of missing error-reporting structure, to
properly account for all possible start and end triggers throughout the
implementation. In some places, small tweaks to shutdown-on-error logic
were made.

BUG=643964
TEST=Manual testing of several different media remoting scenarios, and checking
chrome://histograms to confirm correct measurement values and event tracking.
==========

[miu, 2017-01-16 23:08:44]

Patchset #1 (id:1) has been deleted

[miu, 2017-01-16 23:11:45]

Description was changed from

==========
Media Remoting: UMAs to track session events and measurements.

Adds metrics that track when sessions start and stop, and why; and how
long they lasted; and coarsely-grained stats about codecs/profiles that
were being used. This will help us track that the system is performing
as expected in upcoming trials.

This change also added a lot of missing error-reporting structure, to
properly account for all possible start and end triggers throughout the
implementation. In some places, small tweaks to shutdown-on-error logic
were made.

BUG=643964
TEST=Manual testing of several different media remoting scenarios, and checking
chrome://histograms to confirm correct measurement values and event tracking.
==========

to

==========
Media Remoting: UMAs to track session events and measurements.

Adds metrics that track when sessions start and stop, and why; and how
long they lasted; and coarsely-grained stats about codecs/profiles that
were being used. This will help us track that the system is performing
as expected in upcoming trials.

This change also added a lot of missing error-reporting structure, to
properly account for all possible start and end triggers throughout the
implementation. In some places, small tweaks to shutdown-on-error logic
were made.

Furthermore, a minor change to the fullscreen exit control logic was
made (since the histograms revealed a subtle flaw that would prevent
exiting remoting in some cases where the user's action requested it).

BUG=643964
TEST=Manual testing of several different media remoting scenarios, and checking
chrome://histograms to confirm correct measurement values and event tracking.
==========

[miu, 2017-01-16 23:20:08]

The CQ bit was checked by miu@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-16 23:20:21]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-17 02:42:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-17 02:42:55]

Dry run: This issue passed the CQ dry run.

[xjz, 2017-01-17 05:36:26]

Looks great! Some nits.

https://codereview.chromium.org/2631993002/diff/20001/media/remoting/metrics.cc
File media/remoting/metrics.cc (right):

https://codereview.chromium.org/2631993002/diff/20001/media/remoting/metrics....
media/remoting/metrics.cc:93: // Record the session duration.
nit: DCHECK(!start_time_.is_null());

https://codereview.chromium.org/2631993002/diff/20001/media/remoting/metrics....
media/remoting/metrics.cc:130: config.natural_size() != last_natural_size_);
Do we want to record the change of natural size? Currently
audio/video_decoder_config is only reported once, but the change on natural size
is reported in metadata.natural_size. So maybe we can check:
metadata.natural_size != last_natural_size here.

https://codereview.chromium.org/2631993002/diff/20001/media/remoting/metrics....
media/remoting/metrics.cc:133: last_natural_size_ = config.natural_size();
As commented above, maybe s/config.natural_size()/metadata.natural_size?

https://codereview.chromium.org/2631993002/diff/20001/media/remoting/remoting...
File media/remoting/remoting_renderer_controller.cc (right):

https://codereview.chromium.org/2631993002/diff/20001/media/remoting/remoting...
media/remoting/remoting_renderer_controller.cc:54:
UpdateAndMaybeSwitch(remoting::SINK_AVAILABLE, remoting::ROUTE_TERMINATED);
For EME case, this call will start the remoting session when status is STARTED
(please see the comment in OnSetCdm() below). So maybe:
UpdateAndMaybeSwitch(is_encrypted_ ? remoting::CDM_READY :
remoting::SINK_AVAILABLE,
                                          is_encrypted_ ? 
remoting::DECRYPTION_ERROR : remoting::ROUTE_TERMINATED);

https://codereview.chromium.org/2631993002/diff/20001/media/remoting/remoting...
media/remoting/remoting_renderer_controller.cc:129:
UpdateAndMaybeSwitch(remoting::CDM_READY, remoting::DECRYPTION_ERROR);
I just noticed that this UpdateAndMaybeSwith() call is duplicate and should be
removed here, as it will be called in OnSessionStateChanged() when AddClient()
is called above.

https://codereview.chromium.org/2631993002/diff/20001/media/remoting/remoting...
media/remoting/remoting_renderer_controller.cc:162:
remoting::UNKNOWN_STOP_TRIGGER);
As you explained here, this will never cause a switch. Maybe just remove this
UpdateAndMaybeSwitch() call?

https://codereview.chromium.org/2631993002/diff/20001/media/remoting/remoting...
media/remoting/remoting_renderer_controller.cc:216: start_trigger =
remoting::SUPPORTED_VIDEO_CODEC;
Does the SUPPORTED/UNSUPPORTED_VIDEO_CODEC trigger means to start/stop remoting
when either video codec or both audio and video codec become
supported/unsupported?

https://codereview.chromium.org/2631993002/diff/20001/media/remoting/remoting...
File media/remoting/remoting_renderer_controller.h (right):

https://codereview.chromium.org/2631993002/diff/20001/media/remoting/remoting...
media/remoting/remoting_renderer_controller.h:91: // Called by
RemotingRendererImpl when it encountered a fatal error. This will
s/RemotingRendererImpl/RemoteRendererImpl

https://codereview.chromium.org/2631993002/diff/20001/media/remoting/triggers.h
File media/remoting/triggers.h (right):

https://codereview.chromium.org/2631993002/diff/20001/media/remoting/triggers...
media/remoting/triggers.h:49: ROUTE_TERMINATED = 1,  // The route to the sink
was terminated (user action?).
remove ?

https://codereview.chromium.org/2631993002/diff/20001/media/remoting/triggers...
media/remoting/triggers.h:57: UNSUPPORTED_AUDIO_CODEC = 7,  // Stream began
using an unsupported codec.
nit: s/codec/audio codec

https://codereview.chromium.org/2631993002/diff/20001/media/remoting/triggers...
media/remoting/triggers.h:58: UNSUPPORTED_VIDEO_CODEC = 8,  // Stream began
using an unsupported codec.
nit: s/code/video codec

https://codereview.chromium.org/2631993002/diff/20001/media/remoting/triggers...
media/remoting/triggers.h:70: DATA_PIPE_CREATE_ERROR = 16,  // Mojo data pipe
creation failed (OOM?).
ooc: Is OOM the only reason for this error?

https://codereview.chromium.org/2631993002/diff/20001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/2631993002/diff/20001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:25782: +    Measured audio content
transfer bitrate while remoting content.
nit: Measured/Estimated or Measures?

https://codereview.chromium.org/2631993002/diff/20001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:25852: +    received from the the remote
device indicating initialization succeeded.
s/the the/the

https://codereview.chromium.org/2631993002/diff/20001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:25870: +    Measured video content
transfer bitrate while remoting content.
nit: Measured/Estimated or Measures?

[Steven Holte, 2017-01-17 19:40:12]

https://codereview.chromium.org/2631993002/diff/20001/media/remoting/metrics.cc
File media/remoting/metrics.cc (right):

https://codereview.chromium.org/2631993002/diff/20001/media/remoting/metrics....
media/remoting/metrics.cc:34: name,
This macro uses a static pointer, so you can't use it with a variable name.  You
can either use base::CustomHistogram::FactoryGet directly or convert this into
another macro.

https://codereview.chromium.org/2631993002/diff/20001/media/remoting/metrics....
media/remoting/metrics.cc:51: UMA_HISTOGRAM_CUSTOM_ENUMERATION(
Ditto.

https://codereview.chromium.org/2631993002/diff/20001/media/remoting/metrics....
media/remoting/metrics.cc:160: UMA_HISTOGRAM_CUSTOM_TIMES(name,
download_duration,
Ditto.  For this one you can use UmaHistogramCustomTimes from
histogram_functions.h instead.

[miu, 2017-01-17 21:09:26]

xjz and holte: Thanks for the quick turnaround. Addressed all comments. PTAL.

https://codereview.chromium.org/2631993002/diff/20001/media/remoting/metrics.cc
File media/remoting/metrics.cc (right):

https://codereview.chromium.org/2631993002/diff/20001/media/remoting/metrics....
media/remoting/metrics.cc:34: name,
On 2017/01/17 19:40:11, Steven Holte wrote:
> This macro uses a static pointer, so you can't use it with a variable name. 
You
> can either use base::CustomHistogram::FactoryGet directly or convert this into
> another macro.

Ah! Good catch. Fixed. I realized I was just using these in one place, so I
inlined the code.

(Looks like the code in the file I borrowed this from is okay, since it's only
used in one place. I'll fix that in the follow-up CL.)

https://codereview.chromium.org/2631993002/diff/20001/media/remoting/metrics....
media/remoting/metrics.cc:51: UMA_HISTOGRAM_CUSTOM_ENUMERATION(
On 2017/01/17 19:40:11, Steven Holte wrote:
> Ditto.

Done.

https://codereview.chromium.org/2631993002/diff/20001/media/remoting/metrics....
media/remoting/metrics.cc:93: // Record the session duration.
On 2017/01/17 05:36:25, xjz wrote:
> nit: DCHECK(!start_time_.is_null());

I left it out because I think it's pretty obvious it should be set
(start_trigger_ is being checked a few lines above). I usually use DCHECK() for
logic assumptions that may not be clear.

https://codereview.chromium.org/2631993002/diff/20001/media/remoting/metrics....
media/remoting/metrics.cc:130: config.natural_size() != last_natural_size_);
On 2017/01/17 05:36:25, xjz wrote:
> Do we want to record the change of natural size? Currently
> audio/video_decoder_config is only reported once, but the change on natural
size
> is reported in metadata.natural_size. So maybe we can check:
> metadata.natural_size != last_natural_size here. 

Done. Yes, that's the better one to use.

https://codereview.chromium.org/2631993002/diff/20001/media/remoting/metrics....
media/remoting/metrics.cc:133: last_natural_size_ = config.natural_size();
On 2017/01/17 05:36:25, xjz wrote:
> As commented above, maybe s/config.natural_size()/metadata.natural_size?

Done.

https://codereview.chromium.org/2631993002/diff/20001/media/remoting/metrics....
media/remoting/metrics.cc:160: UMA_HISTOGRAM_CUSTOM_TIMES(name,
download_duration,
On 2017/01/17 19:40:11, Steven Holte wrote:
> Ditto.  For this one you can use UmaHistogramCustomTimes from
> histogram_functions.h instead.

Done.

https://codereview.chromium.org/2631993002/diff/20001/media/remoting/remoting...
File media/remoting/remoting_renderer_controller.cc (right):

https://codereview.chromium.org/2631993002/diff/20001/media/remoting/remoting...
media/remoting/remoting_renderer_controller.cc:54:
UpdateAndMaybeSwitch(remoting::SINK_AVAILABLE, remoting::ROUTE_TERMINATED);
On 2017/01/17 05:36:25, xjz wrote:
> For EME case, this call will start the remoting session when status is STARTED
> (please see the comment in OnSetCdm() below). So maybe:
> UpdateAndMaybeSwitch(is_encrypted_ ? remoting::CDM_READY :
> remoting::SINK_AVAILABLE,
>                                           is_encrypted_ ? 
> remoting::DECRYPTION_ERROR : remoting::ROUTE_TERMINATED);

Hmm...But, there are cases where the route was terminated for a remoting session
with encrypted media. So, I came up with an alternate solution, where the
Start/StopTrigger values depend on how this LOC was reached.

https://codereview.chromium.org/2631993002/diff/20001/media/remoting/remoting...
media/remoting/remoting_renderer_controller.cc:129:
UpdateAndMaybeSwitch(remoting::CDM_READY, remoting::DECRYPTION_ERROR);
On 2017/01/17 05:36:25, xjz wrote:
> I just noticed that this UpdateAndMaybeSwith() call is duplicate and should be
> removed here, as it will be called in OnSessionStateChanged() when AddClient()
> is called above.

Changed as discussed in prior comment. Here, notice that I took out the
automatic call to OnSessionStateChanged() from RemotingSourceImpl; so I make an
"update" call anyway.

https://codereview.chromium.org/2631993002/diff/20001/media/remoting/remoting...
media/remoting/remoting_renderer_controller.cc:162:
remoting::UNKNOWN_STOP_TRIGGER);
On 2017/01/17 05:36:25, xjz wrote:
> As you explained here, this will never cause a switch. Maybe just remove this
> UpdateAndMaybeSwitch() call?

Done.

https://codereview.chromium.org/2631993002/diff/20001/media/remoting/remoting...
media/remoting/remoting_renderer_controller.cc:216: start_trigger =
remoting::SUPPORTED_VIDEO_CODEC;
On 2017/01/17 05:36:25, xjz wrote:
> Does the SUPPORTED/UNSUPPORTED_VIDEO_CODEC trigger means to start/stop
remoting
> when either video codec or both audio and video codec become
> supported/unsupported?

I was doing this, but I suppose it's easy to have more granularity here. Fixed.

https://codereview.chromium.org/2631993002/diff/20001/media/remoting/remoting...
File media/remoting/remoting_renderer_controller.h (right):

https://codereview.chromium.org/2631993002/diff/20001/media/remoting/remoting...
media/remoting/remoting_renderer_controller.h:91: // Called by
RemotingRendererImpl when it encountered a fatal error. This will
On 2017/01/17 05:36:25, xjz wrote:
> s/RemotingRendererImpl/RemoteRendererImpl

Done.

https://codereview.chromium.org/2631993002/diff/20001/media/remoting/triggers.h
File media/remoting/triggers.h (right):

https://codereview.chromium.org/2631993002/diff/20001/media/remoting/triggers...
media/remoting/triggers.h:49: ROUTE_TERMINATED = 1,  // The route to the sink
was terminated (user action?).
On 2017/01/17 05:36:25, xjz wrote:
> remove ?

I left it in there because commonly it's the user shutting down the route.
Sometimes, however, it could be due to an error (network cable unplugged). We
have no way of knowing here.

https://codereview.chromium.org/2631993002/diff/20001/media/remoting/triggers...
media/remoting/triggers.h:57: UNSUPPORTED_AUDIO_CODEC = 7,  // Stream began
using an unsupported codec.
On 2017/01/17 05:36:25, xjz wrote:
> nit: s/codec/audio codec

Done (80 chars is hard).

https://codereview.chromium.org/2631993002/diff/20001/media/remoting/triggers...
media/remoting/triggers.h:58: UNSUPPORTED_VIDEO_CODEC = 8,  // Stream began
using an unsupported codec.
On 2017/01/17 05:36:25, xjz wrote:
> nit: s/code/video codec

Done.

https://codereview.chromium.org/2631993002/diff/20001/media/remoting/triggers...
media/remoting/triggers.h:70: DATA_PIPE_CREATE_ERROR = 16,  // Mojo data pipe
creation failed (OOM?).
On 2017/01/17 05:36:25, xjz wrote:
> ooc: Is OOM the only reason for this error?

No. It could be due to any weird OS issue (or impl bug). But, most commonly, I
would assume the create fails because shared memory cannot be allocated.

https://codereview.chromium.org/2631993002/diff/20001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/2631993002/diff/20001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:25782: +    Measured audio content
transfer bitrate while remoting content.
On 2017/01/17 05:36:25, xjz wrote:
> nit: Measured/Estimated or Measures?

Well, it *is* the measured rate. :) Sure, it's also estimated, but that's
implied by "measurement error."

https://codereview.chromium.org/2631993002/diff/20001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:25852: +    received from the the remote
device indicating initialization succeeded.
On 2017/01/17 05:36:25, xjz wrote:
> s/the the/the

Done.

https://codereview.chromium.org/2631993002/diff/20001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:25870: +    Measured video content
transfer bitrate while remoting content.
On 2017/01/17 05:36:25, xjz wrote:
> nit: Measured/Estimated or Measures?

Acknowledged.

[Steven Holte, 2017-01-17 21:36:22]

histograms lgtm

[xjz, 2017-01-17 21:52:04]

lgtm % nits

https://codereview.chromium.org/2631993002/diff/20001/media/remoting/remoting...
File media/remoting/remoting_renderer_controller.cc (right):

https://codereview.chromium.org/2631993002/diff/20001/media/remoting/remoting...
media/remoting/remoting_renderer_controller.cc:54:
UpdateAndMaybeSwitch(remoting::SINK_AVAILABLE, remoting::ROUTE_TERMINATED);
On 2017/01/17 21:09:18, miu wrote:
> On 2017/01/17 05:36:25, xjz wrote:
> > For EME case, this call will start the remoting session when status is
STARTED
> > (please see the comment in OnSetCdm() below). So maybe:
> > UpdateAndMaybeSwitch(is_encrypted_ ? remoting::CDM_READY :
> > remoting::SINK_AVAILABLE,
> >                                           is_encrypted_ ? 
> > remoting::DECRYPTION_ERROR : remoting::ROUTE_TERMINATED);
> 
> Hmm...But, there are cases where the route was terminated for a remoting
session
> with encrypted media. So, I came up with an alternate solution, where the
> Start/StopTrigger values depend on how this LOC was reached.

This lgtm, though for encrypted media, ROUTE_TERMINATED will never be a trigger
to stop remoting. In that case, we'll just show error interstitial and wait for
user to reload the page.

https://codereview.chromium.org/2631993002/diff/40001/media/remoting/remoting...
File media/remoting/remoting_renderer_controller.cc (right):

https://codereview.chromium.org/2631993002/diff/40001/media/remoting/remoting...
media/remoting/remoting_renderer_controller.cc:166: // trigger a switch.
nit: Now this comment can be deleted.

https://codereview.chromium.org/2631993002/diff/40001/media/remoting/triggers.h
File media/remoting/triggers.h (right):

https://codereview.chromium.org/2631993002/diff/40001/media/remoting/triggers...
media/remoting/triggers.h:36: START_TRIGGER_MAX = 10,
s/10/9

[miu, 2017-01-17 22:46:31]

https://codereview.chromium.org/2631993002/diff/20001/media/remoting/remoting...
File media/remoting/remoting_renderer_controller.cc (right):

https://codereview.chromium.org/2631993002/diff/20001/media/remoting/remoting...
media/remoting/remoting_renderer_controller.cc:54:
UpdateAndMaybeSwitch(remoting::SINK_AVAILABLE, remoting::ROUTE_TERMINATED);
On 2017/01/17 21:52:04, xjz wrote:
> On 2017/01/17 21:09:18, miu wrote:
> > On 2017/01/17 05:36:25, xjz wrote:
> > > For EME case, this call will start the remoting session when status is
> STARTED
> > > (please see the comment in OnSetCdm() below). So maybe:
> > > UpdateAndMaybeSwitch(is_encrypted_ ? remoting::CDM_READY :
> > > remoting::SINK_AVAILABLE,
> > >                                           is_encrypted_ ? 
> > > remoting::DECRYPTION_ERROR : remoting::ROUTE_TERMINATED);
> > 
> > Hmm...But, there are cases where the route was terminated for a remoting
> session
> > with encrypted media. So, I came up with an alternate solution, where the
> > Start/StopTrigger values depend on how this LOC was reached.
> 
> This lgtm, though for encrypted media, ROUTE_TERMINATED will never be a
trigger
> to stop remoting. In that case, we'll just show error interstitial and wait
for
> user to reload the page.

Added a TODO comment to revisit the logic in the future, once more of the
encrypted-remoting impl is in-place.

https://codereview.chromium.org/2631993002/diff/40001/media/remoting/remoting...
File media/remoting/remoting_renderer_controller.cc (right):

https://codereview.chromium.org/2631993002/diff/40001/media/remoting/remoting...
media/remoting/remoting_renderer_controller.cc:166: // trigger a switch.
On 2017/01/17 21:52:04, xjz wrote:
> nit: Now this comment can be deleted.

Notice that I re-worded the comment since the last patch set to explain why we
*do not* call UpdateAndMaybeSwitch(). ;-) The comment is now just documentative,
so we don't forget why we purposely don't call UpdateAndMaybeSwitch() here (even
though setting |switch_renderer_cb_| might seem like a good reason to try
switching).

https://codereview.chromium.org/2631993002/diff/40001/media/remoting/triggers.h
File media/remoting/triggers.h (right):

https://codereview.chromium.org/2631993002/diff/40001/media/remoting/triggers...
media/remoting/triggers.h:36: START_TRIGGER_MAX = 10,
On 2017/01/17 21:52:04, xjz wrote:
> s/10/9

Done.

[miu, 2017-01-17 22:48:16]

The CQ bit was checked by miu@chromium.org

[miu, 2017-01-17 22:48:17]

The patchset sent to the CQ was uploaded after l-g-t-m from xjz@chromium.org,
holte@chromium.org
Link to the patchset: https://codereview.chromium.org/2631993002/#ps80001
(title: "REBASE before commit")

[commit-bot: I haz the power, 2017-01-17 22:48:49]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-18 01:31:44]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1484693296758220,
"parent_rev": "3472bffad91d597a52c92e7e9f4812608d7a9130", "commit_rev":
"5949e051331c74dc1bc140b609a785e491858dd4"}

[commit-bot: I haz the power, 2017-01-18 01:34:39]

Description was changed from

==========
Media Remoting: UMAs to track session events and measurements.

Adds metrics that track when sessions start and stop, and why; and how
long they lasted; and coarsely-grained stats about codecs/profiles that
were being used. This will help us track that the system is performing
as expected in upcoming trials.

This change also added a lot of missing error-reporting structure, to
properly account for all possible start and end triggers throughout the
implementation. In some places, small tweaks to shutdown-on-error logic
were made.

Furthermore, a minor change to the fullscreen exit control logic was
made (since the histograms revealed a subtle flaw that would prevent
exiting remoting in some cases where the user's action requested it).

BUG=643964
TEST=Manual testing of several different media remoting scenarios, and checking
chrome://histograms to confirm correct measurement values and event tracking.
==========

to

==========
Media Remoting: UMAs to track session events and measurements.

Adds metrics that track when sessions start and stop, and why; and how
long they lasted; and coarsely-grained stats about codecs/profiles that
were being used. This will help us track that the system is performing
as expected in upcoming trials.

This change also added a lot of missing error-reporting structure, to
properly account for all possible start and end triggers throughout the
implementation. In some places, small tweaks to shutdown-on-error logic
were made.

Furthermore, a minor change to the fullscreen exit control logic was
made (since the histograms revealed a subtle flaw that would prevent
exiting remoting in some cases where the user's action requested it).

BUG=643964
TEST=Manual testing of several different media remoting scenarios, and checking
chrome://histograms to confirm correct measurement values and event tracking.

Review-Url: https://codereview.chromium.org/2631993002
Cr-Commit-Position: refs/heads/master@{#444230}
Committed:
https://chromium.googlesource.com/chromium/src/+/5949e051331c74dc1bc140b609a7...
==========

[commit-bot: I haz the power, 2017-01-18 01:34:40]

Committed patchset #4 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/5949e051331c74dc1bc140b609a7...