Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2856783002:
Autoplay time metric (Closed)
|
Created:
3 years, 7 months ago by hubbe Modified:
3 years, 7 months ago CC:
mlamouri (slow - plz ping), blink-reviews, blink-reviews-html_chromium.org, chromium-reviews, DaleCurtis, dglazkov+blink, feature-media-reviews_chromium.org Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionAutoplay time metric
We're going to run an experiment to make the "canplaythrough" event fire according
to spec instead of at the same time as "canplay" this will sometimes delay autoplay,
and we want to know how much, so let's add a metric for that.
BUG=73609
Review-Url: https://codereview.chromium.org/2856783002
Cr-Commit-Position: refs/heads/master@{#472331}
Committed: https://chromium.googlesource.com/chromium/src/+/7272b31c2cd71e1ef2f1b0161920d2e040eb5424
Patch Set 1 #
Total comments: 5
Patch Set 2 : comments addressed #
Total comments: 3
Patch Set 3 : added OnLoadStarted() #
Total comments: 9
Patch Set 4 : plumbed load type through #
Total comments: 2
Patch Set 5 : remove MS postfix #
Total comments: 2
Patch Set 6 : histogram description update #
Total comments: 8
Patch Set 7 : comments addressed #
Total comments: 2
Patch Set 8 : use loadstart event #
Messages
Total messages: 75 (44 generated)
The CQ bit was checked by hubbe@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
hubbe@chromium.org changed reviewers: + zqzhang@chromium.org
https://codereview.chromium.org/2856783002/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/html/media/AutoplayUmaHelper.cpp (right): https://codereview.chromium.org/2856783002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/html/media/AutoplayUmaHelper.cpp:51: int32_t autoplay_wait_time_ms = static_cast<int32_t>( Do we need to distinguish the autoplay sources? https://codereview.chromium.org/2856783002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/html/media/AutoplayUmaHelper.cpp:69: ("Media.Video.Autoplay.Video.WaitTimeMS", 1, You probably forgot to add the new histograms to histograms.xml :)
The CQ bit was checked by hubbe@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2856783002/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/html/media/AutoplayUmaHelper.cpp (right): https://codereview.chromium.org/2856783002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/html/media/AutoplayUmaHelper.cpp:51: int32_t autoplay_wait_time_ms = static_cast<int32_t>( On 2017/05/01 23:44:55, Zhiqiang Zhang (Inactive) wrote: > Do we need to distinguish the autoplay sources? That's probably a good idea, done. https://codereview.chromium.org/2856783002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/html/media/AutoplayUmaHelper.cpp:69: ("Media.Video.Autoplay.Video.WaitTimeMS", 1, On 2017/05/01 23:44:55, Zhiqiang Zhang (Inactive) wrote: > You probably forgot to add the new histograms to histograms.xml :) OOps, yes added. (Should I add you as an additional <owner> ?)
Put some comments on "autoplay by attribute". Otherwise, lgtm https://codereview.chromium.org/2856783002/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/html/media/AutoplayUmaHelper.cpp (right): https://codereview.chromium.org/2856783002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/html/media/AutoplayUmaHelper.cpp:69: ("Media.Video.Autoplay.Video.WaitTimeMS", 1, On 2017/05/02 at 00:05:31, hubbe wrote: > On 2017/05/01 23:44:55, Zhiqiang Zhang (Inactive) wrote: > > You probably forgot to add the new histograms to histograms.xml :) > > OOps, yes added. > (Should I add you as an additional <owner> ?) I left chromium recently so no need to add me. https://codereview.chromium.org/2856783002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/html/media/AutoplayUmaHelper.cpp (right): https://codereview.chromium.org/2856783002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/html/media/AutoplayUmaHelper.cpp:92: if (source == AutoplaySource::kAttribute) { Not sure autoplay by attribute will be affected by the experiment. Currently it is independent from canplaythrough event. https://codereview.chromium.org/2856783002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/html/media/AutoplayUmaHelper.cpp:99: if (source == AutoplaySource::kAttribute) { Ditto. https://codereview.chromium.org/2856783002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/html/media/AutoplayUmaHelper.h (right): https://codereview.chromium.org/2856783002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/html/media/AutoplayUmaHelper.h:148: double created_time_ms_; nit: maybe "creation_time_ms_" is better?
dalecurtis@chromium.org changed reviewers: + dalecurtis@chromium.org
I don't think this is right, we create the autoplay helper at HTMLME creation time, but a src may not be set until much later. We instead want to log time from load() -> playback starts.
On 2017/05/02 at 00:21:21, dalecurtis wrote: > I don't think this is right, we create the autoplay helper at HTMLME creation time, but a src may not be set until much later. We instead want to log time from load() -> playback starts. I agree. You probably want to plumb the load event to AutoplayUmaHelper via a method called "onLoadStarted()".
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by hubbe@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
PTAL
On 2017/05/02 at 20:57:49, hubbe wrote: > PTAL lgtm
https://codereview.chromium.org/2856783002/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/html/media/AutoplayUmaHelper.cpp (right): https://codereview.chromium.org/2856783002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/html/media/AutoplayUmaHelper.cpp:50: void AutoplayUmaHelper::OnLoadStarted() { Probably should take the type. We only care about src= and MSE/data/blob/MediaStream will slant values towards zero. https://codereview.chromium.org/2856783002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/html/media/AutoplayUmaHelper.cpp:58: std::min<int64_t>(MonotonicallyIncreasingTimeMS() - load_start_time_ms_, Clamp to kMaxWaitTimeUmaMS? https://codereview.chromium.org/2856783002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/html/media/AutoplayUmaHelper.cpp:74: DEFINE_STATIC_LOCAL(CustomCountHistogram, wait_time_video_attrib_histogram, I don't think we really care about the difference between attribute or play method, do we?
https://codereview.chromium.org/2856783002/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/html/media/AutoplayUmaHelper.cpp (right): https://codereview.chromium.org/2856783002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/html/media/AutoplayUmaHelper.cpp:74: DEFINE_STATIC_LOCAL(CustomCountHistogram, wait_time_video_attrib_histogram, On 2017/05/02 at 21:05:28, DaleCurtis wrote: > I don't think we really care about the difference between attribute or play method, do we? I think the experiment would only affect autoplay by play() method. For attribute, autoplay starts as long as the readyState changes to HAVE_ENOUGH_DATA, which seems independent with canplaythrough. If you think the different doesn't matter, I'm OK with it.
If you try some sample sites out what kind of values do you see? https://codereview.chromium.org/2856783002/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/html/media/AutoplayUmaHelper.cpp (right): https://codereview.chromium.org/2856783002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/html/media/AutoplayUmaHelper.cpp:74: DEFINE_STATIC_LOCAL(CustomCountHistogram, wait_time_video_attrib_histogram, On 2017/05/02 at 21:21:22, Zhiqiang Zhang (Inactive) wrote: > On 2017/05/02 at 21:05:28, DaleCurtis wrote: > > I don't think we really care about the difference between attribute or play method, do we? > > I think the experiment would only affect autoplay by play() method. For attribute, autoplay starts as long as the readyState changes to HAVE_ENOUGH_DATA, which seems independent with canplaythrough. > > If you think the different doesn't matter, I'm OK with it. Ah, I didn't realize the behavior was different. Having both values seems useful in that case then, so we can compare what autoplay does vs what play() does.
The CQ bit was checked by hubbe@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2856783002/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/html/media/AutoplayUmaHelper.cpp (right): https://codereview.chromium.org/2856783002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/html/media/AutoplayUmaHelper.cpp:50: void AutoplayUmaHelper::OnLoadStarted() { On 2017/05/02 21:05:28, DaleCurtis wrote: > Probably should take the type. We only care about src= and > MSE/data/blob/MediaStream will slant values towards zero. Done. https://codereview.chromium.org/2856783002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/html/media/AutoplayUmaHelper.cpp:58: std::min<int64_t>(MonotonicallyIncreasingTimeMS() - load_start_time_ms_, On 2017/05/02 21:05:28, DaleCurtis wrote: > Clamp to kMaxWaitTimeUmaMS? Doesn't the histogram already do that? https://codereview.chromium.org/2856783002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/html/media/AutoplayUmaHelper.cpp:74: DEFINE_STATIC_LOCAL(CustomCountHistogram, wait_time_video_attrib_histogram, On 2017/05/02 21:36:38, DaleCurtis wrote: > On 2017/05/02 at 21:21:22, Zhiqiang Zhang (Inactive) wrote: > > On 2017/05/02 at 21:05:28, DaleCurtis wrote: > > > I don't think we really care about the difference between attribute or play > method, do we? > > > > I think the experiment would only affect autoplay by play() method. For > attribute, autoplay starts as long as the readyState changes to > HAVE_ENOUGH_DATA, which seems independent with canplaythrough. > > > > If you think the different doesn't matter, I'm OK with it. > > Ah, I didn't realize the behavior was different. Having both values seems useful > in that case then, so we can compare what autoplay does vs what play() does. The thing we're changing actually changes the ready state to HAVE_FUTURE_DATA until we think we can play through or until we stop loading because our buffers are full. At that time we change to HAVE_ENOUGH_DATA and fire canplaythrough, so autoplay will be affected. I still want to break out play() from canplaythrough, because the javascript code may or may not wait for canplaythrough, while autoplay is guaranteed to do it. I suspect that the play() data will be swamped by other random javascript behaviors while the autoplay data will cleanly show what my change is doing.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2856783002/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/html/media/AutoplayUmaHelper.cpp (right): https://codereview.chromium.org/2856783002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/html/media/AutoplayUmaHelper.cpp:58: std::min<int64_t>(MonotonicallyIncreasingTimeMS() - load_start_time_ms_, On 2017/05/02 22:33:26, hubbe wrote: > On 2017/05/02 21:05:28, DaleCurtis wrote: > > Clamp to kMaxWaitTimeUmaMS? > > Doesn't the histogram already do that? Checked, it does, here: https://cs.chromium.org/chromium/src/base/metrics/histogram.cc?rcl=8b47351be5...
lgtm then, needs histogram reviewer. https://codereview.chromium.org/2856783002/diff/60001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/html/media/AutoplayUmaHelper.cpp (right): https://codereview.chromium.org/2856783002/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/core/html/media/AutoplayUmaHelper.cpp:77: ("Media.Video.Autoplay.Video.Attribute.WaitTimeMS", 1, I think we typically don't include MS suffix, but instead set units in the metrics.
The CQ bit was checked by hubbe@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
hubbe@chromium.org changed reviewers: + mpearson@chromium.org
+mpearson for histograms.xml https://codereview.chromium.org/2856783002/diff/60001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/html/media/AutoplayUmaHelper.cpp (right): https://codereview.chromium.org/2856783002/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/core/html/media/AutoplayUmaHelper.cpp:77: ("Media.Video.Autoplay.Video.Attribute.WaitTimeMS", 1, On 2017/05/04 21:22:07, DaleCurtis_OOO_May_5_To_May23 wrote: > I think we typically don't include MS suffix, but instead set units in the > metrics. Done.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
hubbe@chromium.org changed reviewers: + rkaplow@chromium.org - mpearson@chromium.org
-mpearson, +rkaplow for histograms.xml
lgtm
The CQ bit was checked by hubbe@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from zqzhang@chromium.org, dalecurtis@chromium.org Link to the patchset: https://codereview.chromium.org/2856783002/#ps80001 (title: "remove MS postfix")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
mpearson@chromium.org changed reviewers: + mpearson@chromium.org
drive-by comment; sorry I somehow didn't see the original code review request. --mark https://codereview.chromium.org/2856783002/diff/80001/tools/metrics/histogram... File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2856783002/diff/80001/tools/metrics/histogram... tools/metrics/histograms/histograms.xml:28497: + Records time until audio starts based on autoplay attribute. Records time from *when* until audio starts? ditto everywhere below
The CQ bit was checked by hubbe@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
https://codereview.chromium.org/2856783002/diff/80001/tools/metrics/histogram... File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2856783002/diff/80001/tools/metrics/histogram... tools/metrics/histograms/histograms.xml:28497: + Records time until audio starts based on autoplay attribute. On 2017/05/09 22:09:00, Mark P wrote: > Records time from *when* until audio starts? > ditto everywhere below Updated metric descriptions.
The CQ bit was checked by hubbe@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from rkaplow@chromium.org, dalecurtis@chromium.org, zqzhang@chromium.org Link to the patchset: https://codereview.chromium.org/2856783002/#ps100001 (title: "histogram description update")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
hubbe@chromium.org changed reviewers: + mlamouri@chromium.org
+mlamouri@ for HTMLMediaElement.*
histograms.xml lgtm for the record; I know robert already approved it.
The change itself sgtm but I have one high level comment regarding one metric (in addition of other nits). Maybe you could give more information in the CL title so it's clearer it's about canplaythrough and impact on autoplay? https://codereview.chromium.org/2856783002/diff/100001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/html/media/AutoplayUmaHelper.cpp (right): https://codereview.chromium.org/2856783002/diff/100001/third_party/WebKit/Sou... third_party/WebKit/Source/core/html/media/AutoplayUmaHelper.cpp:53: } style: should drop the { } https://codereview.chromium.org/2856783002/diff/100001/third_party/WebKit/Sou... third_party/WebKit/Source/core/html/media/AutoplayUmaHelper.cpp:57: int32_t autoplay_wait_time_ms = -1; You could use optional here instead of setting to -1 and having to handle 64 bits and 32 bits int in the condition body below. I think it would be more readable but I have no strong opinion. https://codereview.chromium.org/2856783002/diff/100001/third_party/WebKit/Sou... third_party/WebKit/Source/core/html/media/AutoplayUmaHelper.cpp:87: kMaxWaitTimeUmaMS, kWaitTimeBucketCount)); Shouldn't this be Media.Audio.Autoplay instead of Media.Video.Autoplay.Audio? https://codereview.chromium.org/2856783002/diff/100001/third_party/WebKit/Sou... third_party/WebKit/Source/core/html/media/AutoplayUmaHelper.cpp:104: wait_time_video_play_histogram.Count(autoplay_wait_time_ms); Will your change have an effect on `play()`? Autoplaying with `play()` doesn't follow the same rules as with the attribute. With the attribute you set it and the UA will decide when to start so it only start on canplaythrough. However, for `play()`, the usual rules of playback are applied which means that `play()` will trigger when the ready state is `HAVE_FUTURE_DATA` which is one step before `HAVE_ENOUGH_DATA`. Will your change affect this? If it does, it will affect all playback, not only autoplay'd.
The CQ bit was checked by hubbe@chromium.org to run a CQ dry run
PTAL https://codereview.chromium.org/2856783002/diff/100001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/html/media/AutoplayUmaHelper.cpp (right): https://codereview.chromium.org/2856783002/diff/100001/third_party/WebKit/Sou... third_party/WebKit/Source/core/html/media/AutoplayUmaHelper.cpp:53: } On 2017/05/15 14:31:21, mlamouri wrote: > style: should drop the { } Done. https://codereview.chromium.org/2856783002/diff/100001/third_party/WebKit/Sou... third_party/WebKit/Source/core/html/media/AutoplayUmaHelper.cpp:57: int32_t autoplay_wait_time_ms = -1; On 2017/05/15 14:31:20, mlamouri wrote: > You could use optional here instead of setting to -1 and having to handle 64 > bits and 32 bits int in the condition body below. I think it would be more > readable but I have no strong opinion. Acknowledged. https://codereview.chromium.org/2856783002/diff/100001/third_party/WebKit/Sou... third_party/WebKit/Source/core/html/media/AutoplayUmaHelper.cpp:87: kMaxWaitTimeUmaMS, kWaitTimeBucketCount)); On 2017/05/15 14:31:21, mlamouri wrote: > Shouldn't this be Media.Audio.Autoplay instead of Media.Video.Autoplay.Audio? Done. https://codereview.chromium.org/2856783002/diff/100001/third_party/WebKit/Sou... third_party/WebKit/Source/core/html/media/AutoplayUmaHelper.cpp:104: wait_time_video_play_histogram.Count(autoplay_wait_time_ms); On 2017/05/15 14:31:20, mlamouri wrote: > Will your change have an effect on `play()`? Autoplaying with `play()` doesn't > follow the same rules as with the attribute. With the attribute you set it and > the UA will decide when to start so it only start on canplaythrough. However, > for `play()`, the usual rules of playback are applied which means that `play()` > will trigger when the ready state is `HAVE_FUTURE_DATA` which is one step before > `HAVE_ENOUGH_DATA`. Will your change affect this? If it does, it will affect all > playback, not only autoplay'd. It shouldn't affect code that calls play() immediately, as I'm not making any changes to HAVE_FUTURE_DATA. It should however effect code that waits for the state to change to canplaythrough. This is part of the reason I split it out into several metrics. One to see if it affects the web as a whole (expected answer: no, or within margin of error.) and one that shows how it affects autoplay (expected answer: yes, but not very much.)
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
lgtm with comment applied. https://codereview.chromium.org/2856783002/diff/120001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/html/media/AutoplayUmaHelper.cpp (right): https://codereview.chromium.org/2856783002/diff/120001/third_party/WebKit/Sou... third_party/WebKit/Source/core/html/media/AutoplayUmaHelper.cpp:53: } Instead of adding a "OnLoadStarted", I think it would be more efficient to do: - in the ctor, add `element->addEventListener(EventTypeNames::loadstart, this, false); - in handleEvent, forward the call to a private "OnLoadStarted" - in OnLoadStarted, check `element->GetLoadType()` instead of using `load_type`. This will reduce the hard depedency between HTMLMediaElement and AutoplayUmaHelper.
The CQ bit was checked by hubbe@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2856783002/diff/120001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/html/media/AutoplayUmaHelper.cpp (right): https://codereview.chromium.org/2856783002/diff/120001/third_party/WebKit/Sou... third_party/WebKit/Source/core/html/media/AutoplayUmaHelper.cpp:53: } On 2017/05/16 12:59:40, mlamouri wrote: > Instead of adding a "OnLoadStarted", I think it would be more efficient to do: > - in the ctor, add `element->addEventListener(EventTypeNames::loadstart, this, > false); > - in handleEvent, forward the call to a private "OnLoadStarted" > - in OnLoadStarted, check `element->GetLoadType()` instead of using `load_type`. > > This will reduce the hard depedency between HTMLMediaElement and > AutoplayUmaHelper. Much better, done. (I just had to make GetLoadType() public)
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
The CQ bit was checked by hubbe@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from rkaplow@chromium.org, mpearson@chromium.org, zqzhang@chromium.org, dalecurtis@chromium.org, mlamouri@chromium.org Link to the patchset: https://codereview.chromium.org/2856783002/#ps140001 (title: "use loadstart event")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 140001, "attempt_start_ts": 1494978859490200,
"parent_rev": "bb5dc299a8a07453c5accc73a35da63440261dae", "commit_rev":
"7272b31c2cd71e1ef2f1b0161920d2e040eb5424"}
Message was sent while issue was closed.
Description was changed from ========== Autoplay time metric We're going to run an experiment to make the "canplaythrough" event fire according to spec instead of at the same time as "canplay" this will sometimes delay autoplay, and we want to know how much, so let's add a metric for that. BUG=73609 ========== to ========== Autoplay time metric We're going to run an experiment to make the "canplaythrough" event fire according to spec instead of at the same time as "canplay" this will sometimes delay autoplay, and we want to know how much, so let's add a metric for that. BUG=73609 Review-Url: https://codereview.chromium.org/2856783002 Cr-Commit-Position: refs/heads/master@{#472331} Committed: https://chromium.googlesource.com/chromium/src/+/7272b31c2cd71e1ef2f1b0161920... ==========
Message was sent while issue was closed.
Committed patchset #8 (id:140001) as https://chromium.googlesource.com/chromium/src/+/7272b31c2cd71e1ef2f1b0161920... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
