New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 814351 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 1
Type: Bug



Sign in to add a comment

Media Engagement: send UKM event when "IsHigh" status changes

Project Member Reported by mlamouri@chromium.org, Feb 21 2018

Issue description

It will incorporate a status metric providing the type of change.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Feb 28 2018

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

commit fa7feb3a0670f3bd2190acc459207e2249c53cd5
Author: Mounir Lamouri <mlamouri@chromium.org>
Date: Wed Feb 28 12:58:03 2018

Media Engagement: record the number of "isHigh" status changes.

These changes are sent over with UKM data and wiped when local data are
wiped.

Bug:  814351 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I189a430e46fb2a68af34aa7d4b237adad45d781a
Reviewed-on: https://chromium-review.googlesource.com/931471
Commit-Queue: Mounir Lamouri <mlamouri@chromium.org>
Reviewed-by: Tommy Steimel <steimel@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Cr-Commit-Position: refs/heads/master@{#539807}
[modify] https://crrev.com/fa7feb3a0670f3bd2190acc459207e2249c53cd5/chrome/browser/media/media_engagement_score.cc
[modify] https://crrev.com/fa7feb3a0670f3bd2190acc459207e2249c53cd5/chrome/browser/media/media_engagement_score.h
[modify] https://crrev.com/fa7feb3a0670f3bd2190acc459207e2249c53cd5/chrome/browser/media/media_engagement_score_details.mojom
[modify] https://crrev.com/fa7feb3a0670f3bd2190acc459207e2249c53cd5/chrome/browser/media/media_engagement_score_unittest.cc
[modify] https://crrev.com/fa7feb3a0670f3bd2190acc459207e2249c53cd5/chrome/browser/media/media_engagement_session.cc
[modify] https://crrev.com/fa7feb3a0670f3bd2190acc459207e2249c53cd5/chrome/browser/media/media_engagement_session_unittest.cc
[modify] https://crrev.com/fa7feb3a0670f3bd2190acc459207e2249c53cd5/chrome/browser/resources/media/media_engagement.html
[modify] https://crrev.com/fa7feb3a0670f3bd2190acc459207e2249c53cd5/chrome/browser/resources/media/media_engagement.js
[modify] https://crrev.com/fa7feb3a0670f3bd2190acc459207e2249c53cd5/tools/metrics/ukm/ukm.xml

Status: Fixed (was: Started)
Cc: mlamouri@chromium.org
Owner: beccahughes@chromium.org
Status: Assigned (was: Fixed)
Following up from a discussion hbengali@ and I had yesterday, it sounds that we want something different.

To give some context, initially, my understanding was that we wanted a count so the UKM computation could just look at it. I did not want to keep track of it so I suggested to just have a boolean that is on *when* there is a change.

Then, I realised it wouldn't be much work to do the counting, talked to hbengali about it and agreed on how it should work.

Finally, it seems that the counting is more pain than help and the solution should be to have a IsHighChanged boolean that is 1 when the UKM event created an IsHigh status change.

We should try to land this soon and see if we can merge.
Status: Started (was: Assigned)
Privacy LGTM.
Project Member

Comment 6 by bugdroid1@chromium.org, Apr 9 2018

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

commit 902f59aca1722a518d4da15524bc55623670c8ea
Author: Becca Hughes <beccahughes@chromium.org>
Date: Mon Apr 09 19:34:44 2018

Media Engagement: Add a UKM bit if the session changed isHigh

This adds an Engagement.IsHigh.Changed bit to the SessionFinished UKM
that will be true if the IsHigh bit changed during the lifetime of the
session.

BUG= 814351 

Change-Id: I48b296301a59d7f92ce6fd8ac7750c41ed2bbecd
Reviewed-on: https://chromium-review.googlesource.com/964181
Commit-Queue: Becca Hughes <beccahughes@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549242}
[modify] https://crrev.com/902f59aca1722a518d4da15524bc55623670c8ea/chrome/browser/media/media_engagement_score.h
[modify] https://crrev.com/902f59aca1722a518d4da15524bc55623670c8ea/chrome/browser/media/media_engagement_session.cc
[modify] https://crrev.com/902f59aca1722a518d4da15524bc55623670c8ea/chrome/browser/media/media_engagement_session.h
[modify] https://crrev.com/902f59aca1722a518d4da15524bc55623670c8ea/chrome/browser/media/media_engagement_session_unittest.cc
[modify] https://crrev.com/902f59aca1722a518d4da15524bc55623670c8ea/tools/metrics/ukm/ukm.xml

Status: Fixed (was: Started)

Sign in to add a comment