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

Issue 807983 link

Starred by 2 users

Issue metadata

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



Sign in to add a comment

Media Engagement: don't record visits with no playback on restored sessions

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

Issue description

We should mark a MediaEngagementSession as restored when `DidFinishNavigation`'s `navigation_handle` is flagged as coming from a restore action.

There are three type of restores:
- startup from crash;
- startup (clean);
- same session (such as "reopen closed tabs")

It's unclear if we should do this for the latter but for simplicity, we will. We may want to check what happens when a tab is restored when killed by Chrome.
 
Labels: -M-66 M-65
Project Member

Comment 2 by bugdroid1@chromium.org, Feb 7 2018

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

commit bb66af1ec5213a479b85de3daed53fe88b2de02d
Author: Mounir Lamouri <mlamouri@chromium.org>
Date: Wed Feb 07 10:20:58 2018

Media Engagement: do not record visits from restored sessions with no playback.

This will avoid restored pages from consistently reducing the MEI of a website
if the user did not actually play something in them.

Bug:  807983 
Change-Id: I81d88a22224f76eb77ee820ffd250c10c30583bb
Reviewed-on: https://chromium-review.googlesource.com/901623
Commit-Queue: Mounir Lamouri <mlamouri@chromium.org>
Reviewed-by: Tommy Steimel <steimel@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534974}
[modify] https://crrev.com/bb66af1ec5213a479b85de3daed53fe88b2de02d/chrome/browser/media/media_engagement_browsertest.cc
[modify] https://crrev.com/bb66af1ec5213a479b85de3daed53fe88b2de02d/chrome/browser/media/media_engagement_contents_observer.cc
[modify] https://crrev.com/bb66af1ec5213a479b85de3daed53fe88b2de02d/chrome/browser/media/media_engagement_contents_observer.h
[modify] https://crrev.com/bb66af1ec5213a479b85de3daed53fe88b2de02d/chrome/browser/media/media_engagement_contents_observer_unittest.cc
[modify] https://crrev.com/bb66af1ec5213a479b85de3daed53fe88b2de02d/chrome/browser/media/media_engagement_session.cc
[modify] https://crrev.com/bb66af1ec5213a479b85de3daed53fe88b2de02d/chrome/browser/media/media_engagement_session.h
[modify] https://crrev.com/bb66af1ec5213a479b85de3daed53fe88b2de02d/chrome/browser/media/media_engagement_session_unittest.cc
[modify] https://crrev.com/bb66af1ec5213a479b85de3daed53fe88b2de02d/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/bb66af1ec5213a479b85de3daed53fe88b2de02d/tools/metrics/histograms/histograms.xml

Labels: Merge-Request-65
We would like to get this merged into M65 in order to be able to compare data (Histograms & UKM) from Dev and Beta populations.
Project Member

Comment 4 by sheriffbot@chromium.org, Feb 8 2018

Labels: -Merge-Request-65 Hotlist-Merge-Approved Merge-Approved-65
Your change meets the bar and is auto-approved for M65. Please go ahead and merge the CL to branch 3325 manually. Please contact milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Pls merge your change to M65 branch 3325 ASAP so we can pick it up for next M65 Beta release. Thank you.
Project Member

Comment 6 by bugdroid1@chromium.org, Feb 8 2018

Labels: -merge-approved-65 merge-merged-3325
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7539d7feb538a8c10b71716c9afd361e63c72c9f

commit 7539d7feb538a8c10b71716c9afd361e63c72c9f
Author: Mounir Lamouri <mlamouri@chromium.org>
Date: Thu Feb 08 20:10:14 2018

Media Engagement: do not record visits from restored sessions with no playback.

This will avoid restored pages from consistently reducing the MEI of a website
if the user did not actually play something in them.

Bug:  807983 
Change-Id: I81d88a22224f76eb77ee820ffd250c10c30583bb
Reviewed-on: https://chromium-review.googlesource.com/901623
Commit-Queue: Mounir Lamouri <mlamouri@chromium.org>
Reviewed-by: Tommy Steimel <steimel@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#534974}(cherry picked from commit bb66af1ec5213a479b85de3daed53fe88b2de02d)
Reviewed-on: https://chromium-review.googlesource.com/909370
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Cr-Commit-Position: refs/branch-heads/3325@{#389}
Cr-Branched-From: bc084a8b5afa3744a74927344e304c02ae54189f-refs/heads/master@{#530369}
[modify] https://crrev.com/7539d7feb538a8c10b71716c9afd361e63c72c9f/chrome/browser/media/media_engagement_browsertest.cc
[modify] https://crrev.com/7539d7feb538a8c10b71716c9afd361e63c72c9f/chrome/browser/media/media_engagement_contents_observer.cc
[modify] https://crrev.com/7539d7feb538a8c10b71716c9afd361e63c72c9f/chrome/browser/media/media_engagement_contents_observer.h
[modify] https://crrev.com/7539d7feb538a8c10b71716c9afd361e63c72c9f/chrome/browser/media/media_engagement_contents_observer_unittest.cc
[modify] https://crrev.com/7539d7feb538a8c10b71716c9afd361e63c72c9f/chrome/browser/media/media_engagement_session.cc
[modify] https://crrev.com/7539d7feb538a8c10b71716c9afd361e63c72c9f/chrome/browser/media/media_engagement_session.h
[modify] https://crrev.com/7539d7feb538a8c10b71716c9afd361e63c72c9f/chrome/browser/media/media_engagement_session_unittest.cc
[modify] https://crrev.com/7539d7feb538a8c10b71716c9afd361e63c72c9f/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/7539d7feb538a8c10b71716c9afd361e63c72c9f/tools/metrics/histograms/histograms.xml

Status: Fixed (was: Started)

Sign in to add a comment