New issue
Advanced search Search tips

Issue 807269 link

Starred by 1 user
Status: Fixed
Owner:
mlamouri@chromium.org
Closed: Jan 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 1
Type: Bug



Sign in to add a comment

Media Engagement: incognito profiles may end up using regular profile data

Project Member Reported by mlamouri@chromium.org, Jan 30 2018 
This was triggered while working on  bug 807268  (a test failed). Because we use `GetSettingsForOneType`, we receive the entries for incognito and regular profile when incognito overrides the regular profiles.

It can be easily tested by doing the following:
1. Use a profile with some MEI data
2. Open incognito profile
3. Open chrome://media-engagement and look for an entry
4. Navigate to that entry
5. Close the tab
6. Open chrome://media-engagement

Expected result: entry updated
Actual result: two entries, one with updated value and one with the old one.
Project Member

Comment 1 by bugdroid1@chromium.org, Jan 30 2018 

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

commit 88f5b767023929735eb358d7dffba90e9c51be14
Author: Mounir Lamouri <mlamouri@chromium.org>
Date: Tue Jan 30 14:50:06 2018

Media Engagement: in incognito, properly override regular profile data.

This is fixing a bug where methods returning all scores would
sometimes have two entries when the incognito profile received new MEI
related events. This is due to some unexpected behaviour from content
settings (in `GetSettingsForOneType`).

Bug:  807269 
Change-Id: Id6b8be8ddd2dac6e1b1fec1922896d760e78d31f
Reviewed-on: https://chromium-review.googlesource.com/893260
Reviewed-by: Tommy Steimel <steimel@chromium.org>
Commit-Queue: Mounir Lamouri (slow) <mlamouri@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532867}
[modify] https://crrev.com/88f5b767023929735eb358d7dffba90e9c51be14/chrome/browser/media/media_engagement_service.cc
[modify] https://crrev.com/88f5b767023929735eb358d7dffba90e9c51be14/chrome/browser/media/media_engagement_service_unittest.cc

Comment 2 by mlamouri@chromium.org, Jan 30 2018

Status: Fixed (was: Started)

Comment 3 by mlamouri@chromium.org, Feb 8 2018

Labels: -M-66 M-65 Merge-Request-65
Requesting merge to M65 so our data gathering will be consistent between M65 and M66. It will also fix the latest merge conflict with have with merging  bug 807268

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

Components: Internals>Media>Engagement
Project Member

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

Labels: -Merge-Request-65 Merge-Review-65 Hotlist-Merge-Review
This bug requires manual review: M65 has already been promoted to the beta branch, so this requires manual review
Please contact the 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

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

Labels: -Merge-Review-65 Merge-Approved-65
Approving merge to M65 branch 3325 based on comment #3 and per offline chat with  mlamouri@.
Project Member

Comment 7 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/+/f06d869dedb4c397a987930a309373bf9e165c19

commit f06d869dedb4c397a987930a309373bf9e165c19
Author: Mounir Lamouri <mlamouri@chromium.org>
Date: Thu Feb 08 20:06:34 2018

Media Engagement: in incognito, properly override regular profile data.

This is fixing a bug where methods returning all scores would
sometimes have two entries when the incognito profile received new MEI
related events. This is due to some unexpected behaviour from content
settings (in `GetSettingsForOneType`).

Bug:  807269 
Change-Id: Id6b8be8ddd2dac6e1b1fec1922896d760e78d31f
Reviewed-on: https://chromium-review.googlesource.com/893260
Reviewed-by: Tommy Steimel <steimel@chromium.org>
Commit-Queue: Mounir Lamouri (slow) <mlamouri@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#532867}(cherry picked from commit 88f5b767023929735eb358d7dffba90e9c51be14)
Reviewed-on: https://chromium-review.googlesource.com/909668
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Cr-Commit-Position: refs/branch-heads/3325@{#387}
Cr-Branched-From: bc084a8b5afa3744a74927344e304c02ae54189f-refs/heads/master@{#530369}
[modify] https://crrev.com/f06d869dedb4c397a987930a309373bf9e165c19/chrome/browser/media/media_engagement_service.cc
[modify] https://crrev.com/f06d869dedb4c397a987930a309373bf9e165c19/chrome/browser/media/media_engagement_service_unittest.cc
►

Sign in to add a comment