|
|
DescriptionAdds histograms for casting feature of Video Player
BUG=415386
TEST=browser_test passes
Committed: https://crrev.com/6717449ee689fdfaa59d54118f9e8ace29031217
Cr-Commit-Position: refs/heads/master@{#307859}
Patch Set 1 #
Total comments: 8
Patch Set 2 : Modify the description of the histogram #
Total comments: 13
Patch Set 3 : Addressed comment #Patch Set 4 : Addressed the comments #
Total comments: 4
Patch Set 5 : Addressed the comments #Patch Set 6 : Move back the place of the recordCastVideoLength (per offline discussion with fukino@) #
Total comments: 6
Patch Set 7 : Addressed the comments by isherman #
Messages
Total messages: 37 (13 generated)
Patchset #1 (id:1) has been deleted
yoshiki@chromium.org changed reviewers: + fukino@chromium.org
@fukino-san, PTAL at video player part? Thanks.
On 2014/12/01 02:42:06, yoshiki wrote: > @fukino-san, PTAL at video player part? Thanks. Some tests are failing. Do they look like flakiness?
On 2014/12/01 03:14:03, fukino wrote: > On 2014/12/01 02:42:06, yoshiki wrote: > > @fukino-san, PTAL at video player part? Thanks. > > Some tests are failing. Do they look like flakiness? The failed tests should be flakiness, since they are galley related tests.
video_player lgtm.
yoshiki@chromium.org changed reviewers: + isherman@chromium.org
@isherman: could you take a look and approve this change? Thanks.
https://codereview.chromium.org/760853003/diff/20001/tools/metrics/histograms... File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/760853003/diff/20001/tools/metrics/histograms... tools/metrics/histograms/histograms.xml:39232: +<histogram name="VideoPlayer.CastedVideoLength"> nit: Please add a units attribute. https://codereview.chromium.org/760853003/diff/20001/tools/metrics/histograms... tools/metrics/histograms/histograms.xml:39262: + <summary>Chrome OS Video Player: the number of cast devices.</summary> When is this recorded? https://codereview.chromium.org/760853003/diff/20001/ui/file_manager/video_pl... File ui/file_manager/video_player/js/cast/caster.js (right): https://codereview.chromium.org/760853003/diff/20001/ui/file_manager/video_pl... ui/file_manager/video_player/js/cast/caster.js:36: metrics.CAST_EXTENSION.MAX_VALUE); I strongly recommend creating a wrapper function for this, to which you just pass metrics.CAST_EXTENSION.EXTENSION_UNAVAILABLE, so that you're not repeating the rest of the code each time you want to log this metric. Less code repetition = less chance of bugs creeping in. https://codereview.chromium.org/760853003/diff/20001/ui/file_manager/video_pl... File ui/file_manager/video_player/js/video_player.js (right): https://codereview.chromium.org/760853003/diff/20001/ui/file_manager/video_pl... ui/file_manager/video_player/js/video_player.js:321: metrics.recordUserAction('CastVideo'); This looks like you're recording a user action rather than a histogram. I'd recommend recording your set of user actions within one or more enumerated histograms; but if you prefer user actions, then you'll need to add these to the actions.xml file rather than the histograms.xml one.
Patchset #2 (id:40001) has been deleted
Patchset #2 (id:60001) has been deleted
Patchset #2 (id:80001) has been deleted
@isherman: PTAL again? Thanks. https://codereview.chromium.org/760853003/diff/20001/tools/metrics/histograms... File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/760853003/diff/20001/tools/metrics/histograms... tools/metrics/histograms/histograms.xml:39232: +<histogram name="VideoPlayer.CastedVideoLength"> On 2014/12/02 23:45:10, Ilya Sherman wrote: > nit: Please add a units attribute. Done. https://codereview.chromium.org/760853003/diff/20001/tools/metrics/histograms... tools/metrics/histograms/histograms.xml:39262: + <summary>Chrome OS Video Player: the number of cast devices.</summary> On 2014/12/02 23:45:10, Ilya Sherman wrote: > When is this recorded? Added a comment in description. https://codereview.chromium.org/760853003/diff/20001/ui/file_manager/video_pl... File ui/file_manager/video_player/js/cast/caster.js (right): https://codereview.chromium.org/760853003/diff/20001/ui/file_manager/video_pl... ui/file_manager/video_player/js/cast/caster.js:36: metrics.CAST_EXTENSION.MAX_VALUE); On 2014/12/02 23:45:10, Ilya Sherman wrote: > I strongly recommend creating a wrapper function for this, to which you just > pass metrics.CAST_EXTENSION.EXTENSION_UNAVAILABLE, so that you're not repeating > the rest of the code each time you want to log this metric. Less code > repetition = less chance of bugs creeping in. Done. https://codereview.chromium.org/760853003/diff/20001/ui/file_manager/video_pl... File ui/file_manager/video_player/js/video_player.js (right): https://codereview.chromium.org/760853003/diff/20001/ui/file_manager/video_pl... ui/file_manager/video_player/js/video_player.js:321: metrics.recordUserAction('CastVideo'); On 2014/12/02 23:45:10, Ilya Sherman wrote: > This looks like you're recording a user action rather than a histogram. I'd > recommend recording your set of user actions within one or more enumerated > histograms; but if you prefer user actions, then you'll need to add these to the > actions.xml file rather than the histograms.xml one. Thanks for guidance. I've changed it to recording the type of play (local play or cast). I think it is metrics rather than user action.
Thanks. I'm not very familiar with the JavaScript code, so please ask a team member to review the entire CL prior to my next review. https://codereview.chromium.org/760853003/diff/100001/tools/metrics/actions/a... File tools/metrics/actions/actions.xml (right): https://codereview.chromium.org/760853003/diff/100001/tools/metrics/actions/a... tools/metrics/actions/actions.xml:11381: </action> This sounds like it should be a histogram rather than an action. What does "the number of cast devices" mean as aan action? https://codereview.chromium.org/760853003/diff/100001/tools/metrics/actions/a... tools/metrics/actions/actions.xml:11386: Chrome OS Video Player: this is recorded when user opens video player. Optional nit: I'd rephrase slightly as: "The user opened the Chrome OS Video Player". https://codereview.chromium.org/760853003/diff/100001/tools/metrics/histogram... File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/760853003/diff/100001/tools/metrics/histogram... tools/metrics/histograms/histograms.xml:39461: + extension. Please document when this is recorded. https://codereview.chromium.org/760853003/diff/100001/tools/metrics/histogram... tools/metrics/histograms/histograms.xml:39465: +<histogram name="VideoPlayer.NumberOfCastDevice"> nit: typo: "Device" -> "Devices" https://codereview.chromium.org/760853003/diff/100001/tools/metrics/histogram... tools/metrics/histograms/histograms.xml:39480: +<histogram name="VideoPlayer.PlayType"> Please associate an enum with this histogram (in this histograms.xml file). https://codereview.chromium.org/760853003/diff/100001/tools/metrics/histogram... tools/metrics/histograms/histograms.xml:57061: + label="Cast API extension is available and loaded successfully."/> Hmm, what is the difference between installed and available? https://codereview.chromium.org/760853003/diff/100001/tools/metrics/histogram... tools/metrics/histograms/histograms.xml:57061: + label="Cast API extension is available and loaded successfully."/> Optional: Long enum labels tend to be a little difficult to read, as they are shown as labels in a table column. I'd recommend shorter labels that omit redundant information, e.g. "Unavailable", "Installation failed", "Load failed", "Installed, loaded successfully", "Available, loaded successfully". https://codereview.chromium.org/760853003/diff/100001/ui/file_manager/video_p... File ui/file_manager/video_player/js/video_player_metrics.js (right): https://codereview.chromium.org/760853003/diff/100001/ui/file_manager/video_p... ui/file_manager/video_player/js/video_player_metrics.js:36: MAX_VALUE: 1, Unless the JS metrics API is different from the C++ metrics API -- and to the best of my knowledge it isn't -- you should set this constant to one-past-the-end, rather than equal to the max value. Otherwise, if you ever extend this histogram to include any additional play types, the metrics will get confused. (If you're curious, the final value passed to recordEnum defines an overflow bucket, and it's best not to emit any data to the overflow bucket -- this both enables error-checking, and means that the size of the last bucket that you emit to doesn't change when you append more buckets.) https://codereview.chromium.org/760853003/diff/100001/ui/file_manager/video_p... ui/file_manager/video_player/js/video_player_metrics.js:95: metrics.recordUserAction('CastVideoError'); I don't see this defined in actions.xml
Patchset #3 (id:120001) has been deleted
Patchset #3 (id:140001) has been deleted
Patchset #3 (id:160001) has been deleted
Patchset #3 (id:180001) has been deleted
@fukino-san, I changed the code a lot. Could you take a look again? Thanks. https://codereview.chromium.org/760853003/diff/100001/tools/metrics/actions/a... File tools/metrics/actions/actions.xml (right): https://codereview.chromium.org/760853003/diff/100001/tools/metrics/actions/a... tools/metrics/actions/actions.xml:11381: </action> On 2014/12/09 01:57:48, Ilya Sherman wrote: > This sounds like it should be a histogram rather than an action. What does "the > number of cast devices" mean as aan action? Sorry, that was my mistake. I added NumberOfCastDevice instead of CastVideoError by mistake. https://codereview.chromium.org/760853003/diff/100001/tools/metrics/actions/a... tools/metrics/actions/actions.xml:11386: Chrome OS Video Player: this is recorded when user opens video player. On 2014/12/09 01:57:48, Ilya Sherman wrote: > Optional nit: I'd rephrase slightly as: "The user opened the Chrome OS Video > Player". Thanks for guidance! Done. https://codereview.chromium.org/760853003/diff/100001/ui/file_manager/video_p... File ui/file_manager/video_player/js/video_player_metrics.js (right): https://codereview.chromium.org/760853003/diff/100001/ui/file_manager/video_p... ui/file_manager/video_player/js/video_player_metrics.js:36: MAX_VALUE: 1, On 2014/12/09 01:57:49, Ilya Sherman wrote: > Unless the JS metrics API is different from the C++ metrics API -- and to the > best of my knowledge it isn't -- you should set this constant to > one-past-the-end, rather than equal to the max value. Otherwise, if you ever > extend this histogram to include any additional play types, the metrics will get > confused. > > (If you're curious, the final value passed to recordEnum defines an overflow > bucket, and it's best not to emit any data to the overflow bucket -- this both > enables error-checking, and means that the size of the last bucket that you emit > to doesn't change when you append more buckets.) In the base class, the final value is passed with adding 1 so the issue is not happened. https://code.google.com/p/chromium/codesearch#chromium/src/ui/file_manager/fi... But I think we should match it with the C++ metrics API. I'll do this work in separate patch as filed http://crbug.com/440645. https://codereview.chromium.org/760853003/diff/100001/ui/file_manager/video_p... ui/file_manager/video_player/js/video_player_metrics.js:95: metrics.recordUserAction('CastVideoError'); On 2014/12/09 01:57:49, Ilya Sherman wrote: > I don't see this defined in actions.xml I accidentally removed. Added.
https://codereview.chromium.org/760853003/diff/200009/ui/file_manager/video_p... File ui/file_manager/video_player/js/cast/cast_video_element.js (right): https://codereview.chromium.org/760853003/diff/200009/ui/file_manager/video_p... ui/file_manager/video_player/js/cast/cast_video_element.js:489: metrics.recordCastedVideoLength(media.media.duration); Is it possible to record casted video's length when a new video is casted? When a user plays 15-seconds videos repeatedly, the duration will be recorded only once. https://codereview.chromium.org/760853003/diff/200009/ui/file_manager/video_p... File ui/file_manager/video_player/js/cast/caster.js (right): https://codereview.chromium.org/760853003/diff/200009/ui/file_manager/video_p... ui/file_manager/video_player/js/cast/caster.js:45: * @param {boolean=} opt_secondTry Spericy try if it's second call after nit outside this CL: s/Spericy try/Specify true/? https://codereview.chromium.org/760853003/diff/200009/ui/file_manager/video_p... ui/file_manager/video_player/js/cast/caster.js:127: metrics.CAST_API_EXTENSION.LOAD_FAILED); nit: incorrect indentation https://codereview.chromium.org/760853003/diff/200009/ui/file_manager/video_p... File ui/file_manager/video_player/js/video_player_metrics.js (right): https://codereview.chromium.org/760853003/diff/200009/ui/file_manager/video_p... ui/file_manager/video_player/js/video_player_metrics.js:20: metrics.CAST_API_EXTENSION = { nit: I'd use CAST_API_EXTENSION_STATUS to match the metrics name. https://codereview.chromium.org/760853003/diff/200009/ui/file_manager/video_p... ui/file_manager/video_player/js/video_player_metrics.js:21: SKIPPED: 0, optional: I like "UNAVAILABLE" more than "SKIPPED" because the meaning of "Cast API extension is skipped" sounds unclear to me.
@fukino-san, Thanks for reviewing. PTAL again? https://codereview.chromium.org/760853003/diff/200009/ui/file_manager/video_p... File ui/file_manager/video_player/js/cast/cast_video_element.js (right): https://codereview.chromium.org/760853003/diff/200009/ui/file_manager/video_p... ui/file_manager/video_player/js/cast/cast_video_element.js:489: metrics.recordCastedVideoLength(media.media.duration); On 2014/12/10 06:49:50, fukino wrote: > Is it possible to record casted video's length when a new video is casted? When > a user plays 15-seconds videos repeatedly, the duration will be recorded only > once. Done. https://codereview.chromium.org/760853003/diff/200009/ui/file_manager/video_p... File ui/file_manager/video_player/js/cast/caster.js (right): https://codereview.chromium.org/760853003/diff/200009/ui/file_manager/video_p... ui/file_manager/video_player/js/cast/caster.js:45: * @param {boolean=} opt_secondTry Spericy try if it's second call after On 2014/12/10 06:49:50, fukino wrote: > nit outside this CL: s/Spericy try/Specify true/? Done. https://codereview.chromium.org/760853003/diff/200009/ui/file_manager/video_p... ui/file_manager/video_player/js/cast/caster.js:127: metrics.CAST_API_EXTENSION.LOAD_FAILED); On 2014/12/10 06:49:50, fukino wrote: > nit: incorrect indentation Done. https://codereview.chromium.org/760853003/diff/200009/ui/file_manager/video_p... File ui/file_manager/video_player/js/video_player_metrics.js (right): https://codereview.chromium.org/760853003/diff/200009/ui/file_manager/video_p... ui/file_manager/video_player/js/video_player_metrics.js:20: metrics.CAST_API_EXTENSION = { On 2014/12/10 06:49:50, fukino wrote: > nit: I'd use CAST_API_EXTENSION_STATUS to match the metrics name. Done. https://codereview.chromium.org/760853003/diff/200009/ui/file_manager/video_p... ui/file_manager/video_player/js/video_player_metrics.js:21: SKIPPED: 0, On 2014/12/10 06:49:51, fukino wrote: > optional: I like "UNAVAILABLE" more than "SKIPPED" because the meaning of "Cast > API extension is skipped" sounds unclear to me. I thinks UNAVAILABLE is ambiguous in this case. I added comment to make it clear, WDYT?
https://codereview.chromium.org/760853003/diff/200009/ui/file_manager/video_p... File ui/file_manager/video_player/js/video_player_metrics.js (right): https://codereview.chromium.org/760853003/diff/200009/ui/file_manager/video_p... ui/file_manager/video_player/js/video_player_metrics.js:21: SKIPPED: 0, On 2014/12/10 07:20:26, yoshiki wrote: > On 2014/12/10 06:49:51, fukino wrote: > > optional: I like "UNAVAILABLE" more than "SKIPPED" because the meaning of > "Cast > > API extension is skipped" sounds unclear to me. > > I thinks UNAVAILABLE is ambiguous in this case. I added comment to make it > clear, WDYT? OK, got it! But the 'unavailable' in the comment also sounds ambiguous. IIUC, there are two cases for 'SKIPPED'. - Any Google Cast extension is not installed. - Loading Google Cast extension failed. If so, ', is not installed or load failed' might be more specific? https://codereview.chromium.org/760853003/diff/220001/ui/file_manager/video_p... File ui/file_manager/video_player/js/cast/cast_video_element.js (right): https://codereview.chromium.org/760853003/diff/220001/ui/file_manager/video_p... ui/file_manager/video_player/js/cast/cast_video_element.js:230: metrics.recordCastedVideoLength(this.currentMediaDuration_); It seems that the code here is for avoiding sending redundant play request. I couldn't understand why this point can record the length of each casted video correctly...
Patchset #4 (id:200009) has been deleted
Patchset #5 (id:240001) has been deleted
@fukino-san, PTAL. Thanks. https://codereview.chromium.org/760853003/diff/200009/ui/file_manager/video_p... ui/file_manager/video_player/js/video_player_metrics.js:21: SKIPPED: 0, On 2014/12/10 08:01:47, fukino wrote: > On 2014/12/10 07:20:26, yoshiki wrote: > > On 2014/12/10 06:49:51, fukino wrote: > > > optional: I like "UNAVAILABLE" more than "SKIPPED" because the meaning of > > "Cast > > > API extension is skipped" sounds unclear to me. > > > > I thinks UNAVAILABLE is ambiguous in this case. I added comment to make it > > clear, WDYT? > > OK, got it! > But the 'unavailable' in the comment also sounds ambiguous. > > IIUC, there are two cases for 'SKIPPED'. > - Any Google Cast extension is not installed. > - Loading Google Cast extension failed. > > If so, ', is not installed or load failed' might be more specific? Done https://codereview.chromium.org/760853003/diff/220001/ui/file_manager/video_p... File ui/file_manager/video_player/js/cast/cast_video_element.js (right): https://codereview.chromium.org/760853003/diff/220001/ui/file_manager/video_p... ui/file_manager/video_player/js/cast/cast_video_element.js:230: metrics.recordCastedVideoLength(this.currentMediaDuration_); On 2014/12/10 08:01:47, fukino wrote: > It seems that the code here is for avoiding sending redundant play request. I > couldn't understand why this point can record the length of each casted video > correctly... Sorry, I added it the wrong place. Fixed.
https://codereview.chromium.org/760853003/diff/220001/ui/file_manager/video_p... File ui/file_manager/video_player/js/cast/cast_video_element.js (right): https://codereview.chromium.org/760853003/diff/220001/ui/file_manager/video_p... ui/file_manager/video_player/js/cast/cast_video_element.js:230: metrics.recordCastedVideoLength(this.currentMediaDuration_); On 2014/12/10 09:33:19, yoshiki wrote: > On 2014/12/10 08:01:47, fukino wrote: > > It seems that the code here is for avoiding sending redundant play request. I > > couldn't understand why this point can record the length of each casted video > > correctly... > > Sorry, I added it the wrong place. Fixed. IIRC, when a user repeat PAUSE->PLAY->PAUSE->PLAY, the duration will be recorded multiple times. How about inside onMediaDiscovered_()?
On 2014/12/10 10:16:43, fukino wrote: > https://codereview.chromium.org/760853003/diff/220001/ui/file_manager/video_p... > File ui/file_manager/video_player/js/cast/cast_video_element.js (right): > > https://codereview.chromium.org/760853003/diff/220001/ui/file_manager/video_p... > ui/file_manager/video_player/js/cast/cast_video_element.js:230: > metrics.recordCastedVideoLength(this.currentMediaDuration_); > On 2014/12/10 09:33:19, yoshiki wrote: > > On 2014/12/10 08:01:47, fukino wrote: > > > It seems that the code here is for avoiding sending redundant play request. > I > > > couldn't understand why this point can record the length of each casted > video > > > correctly... > > > > Sorry, I added it the wrong place. Fixed. > > IIRC, when a user repeat PAUSE->PLAY->PAUSE->PLAY, the duration will be recorded > multiple times. > How about inside onMediaDiscovered_()? I think recording multiple times is acceptable. Even if we add it to onMediaDiscovered_(), if user plays multiple videos, the metrics will be recorded multiple times. WDYT?
すみません見逃してました。 急かしてしまってごめんなさい。ブランチカットまでに入れたいので… On Wed, Dec 10, 2014 at 7:16 PM, <fukino@chromium.org> wrote: > > https://codereview.chromium.org/760853003/diff/220001/ui/ > file_manager/video_player/js/cast/cast_video_element.js > File ui/file_manager/video_player/js/cast/cast_video_element.js (right): > > https://codereview.chromium.org/760853003/diff/220001/ui/ > file_manager/video_player/js/cast/cast_video_element.js#newcode230 > ui/file_manager/video_player/js/cast/cast_video_element.js:230: > metrics.recordCastedVideoLength(this.currentMediaDuration_); > On 2014/12/10 09:33:19, yoshiki wrote: > >> On 2014/12/10 08:01:47, fukino wrote: >> > It seems that the code here is for avoiding sending redundant play >> > request. I > >> > couldn't understand why this point can record the length of each >> > casted video > >> > correctly... >> > > Sorry, I added it the wrong place. Fixed. >> > > IIRC, when a user repeat PAUSE->PLAY->PAUSE->PLAY, the duration will be > recorded multiple times. > How about inside onMediaDiscovered_()? > > https://codereview.chromium.org/760853003/ > To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
On 2014/12/10 12:00:27, yoshiki wrote: > すみません見逃してました。 > > 急かしてしまってごめんなさい。ブランチカットまでに入れたいので… Please ignore the comment above. This intended a just personal message to ask review.
Thank you for explanation. lgtm! https://codereview.chromium.org/760853003/diff/220001/ui/file_manager/video_p... File ui/file_manager/video_player/js/cast/cast_video_element.js (right): https://codereview.chromium.org/760853003/diff/220001/ui/file_manager/video_p... ui/file_manager/video_player/js/cast/cast_video_element.js:230: metrics.recordCastedVideoLength(this.currentMediaDuration_); On 2014/12/10 10:16:42, fukino wrote: > On 2014/12/10 09:33:19, yoshiki wrote: > > On 2014/12/10 08:01:47, fukino wrote: > > > It seems that the code here is for avoiding sending redundant play request. > I > > > couldn't understand why this point can record the length of each casted > video > > > correctly... > > > > Sorry, I added it the wrong place. Fixed. > > IIRC, when a user repeat PAUSE->PLAY->PAUSE->PLAY, the duration will be recorded > multiple times. > How about inside onMediaDiscovered_()? It looks OK to record here, given that covering cases in which multiple media are casted will be complex.
@isherman: could you take a look again? @fukino-san: Thank you for quick review!
LGTM % my remaining comments. Thanks. https://codereview.chromium.org/760853003/diff/280001/tools/metrics/actions/a... File tools/metrics/actions/actions.xml (right): https://codereview.chromium.org/760853003/diff/280001/tools/metrics/actions/a... tools/metrics/actions/actions.xml:11381: Chrome OS Video Player: the error happens on casting. Optional nit: Suggested rephrasing: "An error occurred upon casting". https://codereview.chromium.org/760853003/diff/280001/tools/metrics/histogram... File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/760853003/diff/280001/tools/metrics/histogram... tools/metrics/histograms/histograms.xml:39467: + <summary>Chrome OS Video Player: the number of cast devices.</summary> Please document when this is recorded. https://codereview.chromium.org/760853003/diff/280001/tools/metrics/histogram... tools/metrics/histograms/histograms.xml:39480: + Chrome OS Video Player: type of playback (eg. local play, cast). Please document when this is recorded.
Thanks all! https://codereview.chromium.org/760853003/diff/280001/tools/metrics/actions/a... File tools/metrics/actions/actions.xml (right): https://codereview.chromium.org/760853003/diff/280001/tools/metrics/actions/a... tools/metrics/actions/actions.xml:11381: Chrome OS Video Player: the error happens on casting. On 2014/12/10 23:29:28, Ilya Sherman wrote: > Optional nit: Suggested rephrasing: "An error occurred upon casting". Done. https://codereview.chromium.org/760853003/diff/280001/tools/metrics/histogram... File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/760853003/diff/280001/tools/metrics/histogram... tools/metrics/histograms/histograms.xml:39467: + <summary>Chrome OS Video Player: the number of cast devices.</summary> On 2014/12/10 23:29:28, Ilya Sherman wrote: > Please document when this is recorded. Done. https://codereview.chromium.org/760853003/diff/280001/tools/metrics/histogram... tools/metrics/histograms/histograms.xml:39480: + Chrome OS Video Player: type of playback (eg. local play, cast). On 2014/12/10 23:29:28, Ilya Sherman wrote: > Please document when this is recorded. Done.
The CQ bit was checked by yoshiki@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/760853003/300001
Message was sent while issue was closed.
Committed patchset #7 (id:300001)
Message was sent while issue was closed.
Patchset 7 (id:??) landed as https://crrev.com/6717449ee689fdfaa59d54118f9e8ace29031217 Cr-Commit-Position: refs/heads/master@{#307859} |