New issue
Advanced search Search tips

Issue 796280 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 13
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 3
Type: Bug



Sign in to add a comment

Media Capabilities: db is empty for Guest accounts

Project Member Reported by chcunningham@chromium.org, Dec 19 2017

Issue description

The unintended consequence of the design for incognito mode is that guest mode will create a new video decode stats database and will be unable to write to it.

In incognito mode, we reuse the database (in read only fashion) from the original profile that spawned the incognito window [0]. This follows other incognito behaviors (e.g. history and bookmarks all cary over in read-only mode from the original profile). But this breaks down for guest mode which doesn't have an original profile in the same since (i.e. by design, nothing in guest mode is carried over from some other profile). 

We disable writing to the DB whenever browsing context is "off the record". This check is done when we expose the VideoDecodeStatsRecorder service from the RenderFrameHost [1]. Both guest mode and incognito mode qualify as "off the record".

[0] https://cs.chromium.org/chromium/src/chrome/browser/profiles/off_the_record_profile_impl.cc?rcl=63617d021c1fcc93edb668cc425028cd17631232&l=475

[1] https://cs.chromium.org/chromium/src/content/browser/frame_host/render_frame_host_impl.cc?rcl=a25e33d686e0ccb2b204091de20077a92af480e3&l=3210
 
After discussion with jam@ - deciding to leave as is for now. He recommends building some more momentum before investing in guest-specific plumbing to sort this out. P3
If I understand correctly, the practical problem here is that a Chrome instance that only runs in Guest Mode will have no data. Is that correct?
Correct. It will have no data and it will not be able to add data as playbacks occur. 
Status: Started (was: Assigned)
Project Member

Comment 5 by bugdroid1@chromium.org, Jun 20

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

commit e34efffbfefec5d70b39bcbda16a5806fa4bf7c4
Author: chcunningham <chcunningham@chromium.org>
Date: Wed Jun 20 22:19:14 2018

InMemory MediaCapabilities DB for Guest/Incognito profiles

Prior to this change, Guest mode has an empty read-only DB and Incongito
has a read-only DB. This CL allows both profiles to write to an "in
memory" database that disapears when the profile is destroyed.

This improves the experience for the user and avoids the possibility
of using MediaCapabilites to detect incognito/guest profiles.

R=jam@chromium.org

Bug:  796280 
Change-Id: I37e25478ce6efe8c25bf49f77929f94c9ca27655
Reviewed-on: https://chromium-review.googlesource.com/1092156
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Chrome Cunningham <chcunningham@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569046}
[modify] https://crrev.com/e34efffbfefec5d70b39bcbda16a5806fa4bf7c4/chrome/browser/DEPS
[modify] https://crrev.com/e34efffbfefec5d70b39bcbda16a5806fa4bf7c4/chrome/browser/profiles/off_the_record_profile_impl.cc
[modify] https://crrev.com/e34efffbfefec5d70b39bcbda16a5806fa4bf7c4/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/e34efffbfefec5d70b39bcbda16a5806fa4bf7c4/media/capabilities/BUILD.gn
[add] https://crrev.com/e34efffbfefec5d70b39bcbda16a5806fa4bf7c4/media/capabilities/in_memory_video_decode_stats_db_impl.cc
[add] https://crrev.com/e34efffbfefec5d70b39bcbda16a5806fa4bf7c4/media/capabilities/in_memory_video_decode_stats_db_impl.h
[add] https://crrev.com/e34efffbfefec5d70b39bcbda16a5806fa4bf7c4/media/capabilities/in_memory_video_decode_stats_db_unittest.cc
[modify] https://crrev.com/e34efffbfefec5d70b39bcbda16a5806fa4bf7c4/media/capabilities/video_decode_stats_db.cc
[modify] https://crrev.com/e34efffbfefec5d70b39bcbda16a5806fa4bf7c4/media/capabilities/video_decode_stats_db.h
[modify] https://crrev.com/e34efffbfefec5d70b39bcbda16a5806fa4bf7c4/media/capabilities/video_decode_stats_db_impl.cc
[modify] https://crrev.com/e34efffbfefec5d70b39bcbda16a5806fa4bf7c4/media/capabilities/video_decode_stats_db_impl.h
[add] https://crrev.com/e34efffbfefec5d70b39bcbda16a5806fa4bf7c4/media/capabilities/video_decode_stats_db_provider.cc
[add] https://crrev.com/e34efffbfefec5d70b39bcbda16a5806fa4bf7c4/media/capabilities/video_decode_stats_db_provider.h
[modify] https://crrev.com/e34efffbfefec5d70b39bcbda16a5806fa4bf7c4/media/capabilities/video_decode_stats_db_unittest.cc
[modify] https://crrev.com/e34efffbfefec5d70b39bcbda16a5806fa4bf7c4/media/mojo/services/video_decode_perf_history.cc
[modify] https://crrev.com/e34efffbfefec5d70b39bcbda16a5806fa4bf7c4/media/mojo/services/video_decode_perf_history.h
[modify] https://crrev.com/e34efffbfefec5d70b39bcbda16a5806fa4bf7c4/media/mojo/services/video_decode_perf_history_unittest.cc

Status: Fixed (was: Started)
Guest profiles now get an in-memory DB they can write to. 

Incognito profiles will have in-memory seeded by the on-disk DB of the originating profile. 


Project Member

Comment 7 by bugdroid1@chromium.org, Jun 25

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

commit 80a416c8129c930347eaaa7e571fd8af6b7487db
Author: Mounir Lamouri <mlamouri@chromium.org>
Date: Mon Jun 25 22:18:01 2018

Revert "InMemory MediaCapabilities DB for Guest/Incognito profiles"

This reverts commit e34efffbfefec5d70b39bcbda16a5806fa4bf7c4.

Reason for revert: see bug 855215

Original change's description:
> InMemory MediaCapabilities DB for Guest/Incognito profiles
>
> Prior to this change, Guest mode has an empty read-only DB and Incongito
> has a read-only DB. This CL allows both profiles to write to an "in
> memory" database that disapears when the profile is destroyed.
>
> This improves the experience for the user and avoids the possibility
> of using MediaCapabilites to detect incognito/guest profiles.
>
> R=​jam@chromium.org
>
> Bug:  796280 
> Change-Id: I37e25478ce6efe8c25bf49f77929f94c9ca27655
> Reviewed-on: https://chromium-review.googlesource.com/1092156
> Reviewed-by: John Abd-El-Malek <jam@chromium.org>
> Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
> Reviewed-by: Scott Violet <sky@chromium.org>
> Commit-Queue: Chrome Cunningham <chcunningham@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#569046}

TBR=sky@chromium.org,jam@chromium.org,mlamouri@chromium.org,chcunningham@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  796280 
Change-Id: Idc97eee2871e940e36b9c19f820d9f6207d7f6ef
No-Try: True
Reviewed-on: https://chromium-review.googlesource.com/1113958
Commit-Queue: Mounir Lamouri <mlamouri@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570199}
[modify] https://crrev.com/80a416c8129c930347eaaa7e571fd8af6b7487db/chrome/browser/DEPS
[modify] https://crrev.com/80a416c8129c930347eaaa7e571fd8af6b7487db/chrome/browser/profiles/off_the_record_profile_impl.cc
[modify] https://crrev.com/80a416c8129c930347eaaa7e571fd8af6b7487db/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/80a416c8129c930347eaaa7e571fd8af6b7487db/media/capabilities/BUILD.gn
[delete] https://crrev.com/31e592b5823badfedeeb824ab6521615e5f78329/media/capabilities/in_memory_video_decode_stats_db_impl.cc
[delete] https://crrev.com/31e592b5823badfedeeb824ab6521615e5f78329/media/capabilities/in_memory_video_decode_stats_db_impl.h
[delete] https://crrev.com/31e592b5823badfedeeb824ab6521615e5f78329/media/capabilities/in_memory_video_decode_stats_db_unittest.cc
[modify] https://crrev.com/80a416c8129c930347eaaa7e571fd8af6b7487db/media/capabilities/video_decode_stats_db.cc
[modify] https://crrev.com/80a416c8129c930347eaaa7e571fd8af6b7487db/media/capabilities/video_decode_stats_db.h
[modify] https://crrev.com/80a416c8129c930347eaaa7e571fd8af6b7487db/media/capabilities/video_decode_stats_db_impl.cc
[modify] https://crrev.com/80a416c8129c930347eaaa7e571fd8af6b7487db/media/capabilities/video_decode_stats_db_impl.h
[delete] https://crrev.com/31e592b5823badfedeeb824ab6521615e5f78329/media/capabilities/video_decode_stats_db_provider.cc
[delete] https://crrev.com/31e592b5823badfedeeb824ab6521615e5f78329/media/capabilities/video_decode_stats_db_provider.h
[modify] https://crrev.com/80a416c8129c930347eaaa7e571fd8af6b7487db/media/capabilities/video_decode_stats_db_unittest.cc
[modify] https://crrev.com/80a416c8129c930347eaaa7e571fd8af6b7487db/media/mojo/services/video_decode_perf_history.cc
[modify] https://crrev.com/80a416c8129c930347eaaa7e571fd8af6b7487db/media/mojo/services/video_decode_perf_history.h
[modify] https://crrev.com/80a416c8129c930347eaaa7e571fd8af6b7487db/media/mojo/services/video_decode_perf_history_unittest.cc

Status: Assigned (was: Fixed)
Project Member

Comment 9 by bugdroid1@chromium.org, Jun 25

Labels: merge-merged-3472
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d2503effd5315c6f71544e8df83ecb59b2fc2c03

commit d2503effd5315c6f71544e8df83ecb59b2fc2c03
Author: Mounir Lamouri <mlamouri@chromium.org>
Date: Mon Jun 25 22:19:20 2018

Revert "InMemory MediaCapabilities DB for Guest/Incognito profiles"

This reverts commit e34efffbfefec5d70b39bcbda16a5806fa4bf7c4.

Reason for revert: see bug 855215

Original change's description:
> InMemory MediaCapabilities DB for Guest/Incognito profiles
>
> Prior to this change, Guest mode has an empty read-only DB and Incongito
> has a read-only DB. This CL allows both profiles to write to an "in
> memory" database that disapears when the profile is destroyed.
>
> This improves the experience for the user and avoids the possibility
> of using MediaCapabilites to detect incognito/guest profiles.
>
> R=​jam@chromium.org
>
> Bug:  796280 
> Change-Id: I37e25478ce6efe8c25bf49f77929f94c9ca27655
> Reviewed-on: https://chromium-review.googlesource.com/1092156
> Reviewed-by: John Abd-El-Malek <jam@chromium.org>
> Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
> Reviewed-by: Scott Violet <sky@chromium.org>
> Commit-Queue: Chrome Cunningham <chcunningham@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#569046}

TBR=sky@chromium.org,jam@chromium.org,mlamouri@chromium.org,chcunningham@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  796280 
Change-Id: Idc97eee2871e940e36b9c19f820d9f6207d7f6ef
No-Try: True
Reviewed-on: https://chromium-review.googlesource.com/1113958
Commit-Queue: Mounir Lamouri <mlamouri@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#570199}(cherry picked from commit 80a416c8129c930347eaaa7e571fd8af6b7487db)
Reviewed-on: https://chromium-review.googlesource.com/1113844
Cr-Commit-Position: refs/branch-heads/3472@{#4}
Cr-Branched-From: 3816ddd3a9aa8a35aaff936186a6ac7e0ecf18da-refs/heads/master@{#569948}
[modify] https://crrev.com/d2503effd5315c6f71544e8df83ecb59b2fc2c03/chrome/browser/DEPS
[modify] https://crrev.com/d2503effd5315c6f71544e8df83ecb59b2fc2c03/chrome/browser/profiles/off_the_record_profile_impl.cc
[modify] https://crrev.com/d2503effd5315c6f71544e8df83ecb59b2fc2c03/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/d2503effd5315c6f71544e8df83ecb59b2fc2c03/media/capabilities/BUILD.gn
[delete] https://crrev.com/3a00338665eb7648261c10694409614c19c91584/media/capabilities/in_memory_video_decode_stats_db_impl.cc
[delete] https://crrev.com/3a00338665eb7648261c10694409614c19c91584/media/capabilities/in_memory_video_decode_stats_db_impl.h
[delete] https://crrev.com/3a00338665eb7648261c10694409614c19c91584/media/capabilities/in_memory_video_decode_stats_db_unittest.cc
[modify] https://crrev.com/d2503effd5315c6f71544e8df83ecb59b2fc2c03/media/capabilities/video_decode_stats_db.cc
[modify] https://crrev.com/d2503effd5315c6f71544e8df83ecb59b2fc2c03/media/capabilities/video_decode_stats_db.h
[modify] https://crrev.com/d2503effd5315c6f71544e8df83ecb59b2fc2c03/media/capabilities/video_decode_stats_db_impl.cc
[modify] https://crrev.com/d2503effd5315c6f71544e8df83ecb59b2fc2c03/media/capabilities/video_decode_stats_db_impl.h
[delete] https://crrev.com/3a00338665eb7648261c10694409614c19c91584/media/capabilities/video_decode_stats_db_provider.cc
[delete] https://crrev.com/3a00338665eb7648261c10694409614c19c91584/media/capabilities/video_decode_stats_db_provider.h
[modify] https://crrev.com/d2503effd5315c6f71544e8df83ecb59b2fc2c03/media/capabilities/video_decode_stats_db_unittest.cc
[modify] https://crrev.com/d2503effd5315c6f71544e8df83ecb59b2fc2c03/media/mojo/services/video_decode_perf_history.cc
[modify] https://crrev.com/d2503effd5315c6f71544e8df83ecb59b2fc2c03/media/mojo/services/video_decode_perf_history.h
[modify] https://crrev.com/d2503effd5315c6f71544e8df83ecb59b2fc2c03/media/mojo/services/video_decode_perf_history_unittest.cc

Project Member

Comment 10 by bugdroid1@chromium.org, Jul 11

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

commit 9febeb19dab5b13383b5b4145c0a3e6e0af82531
Author: chcunningham <chcunningham@chromium.org>
Date: Wed Jul 11 13:53:21 2018

VideoDecodePerfHistory - safer SaveCB.

This changes the interface such that saves can only performed via the
SaveCB. Internally the CB is weakly bound to the perf history so post
destruction saves are quietly dropped.

This unblocks re-landing
https://chromium-review.googlesource.com/c/chromium/src/+/1123687/1

The above CL will give incognito profiles a distinct
VideoDecodePerfHistory instance. Incognito profile's are rapidly deleted
when the last tab closes, creating a race condition for any per stats
that are inbound from video's on that tab. Now, using the SaveCB,
those late stats will be safely dropped.

Bug:  855631 , 855215,  796280 
Change-Id: Ia978dbca14d5584ca92301aee84020437a90311e
Reviewed-on: https://chromium-review.googlesource.com/1123396
Reviewed-by: Markus Heintz <markusheintz@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Commit-Queue: Chrome Cunningham (In Paris) <chcunningham@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574144}
[modify] https://crrev.com/9febeb19dab5b13383b5b4145c0a3e6e0af82531/chrome/browser/browsing_data/browsing_data_remover_browsertest.cc
[modify] https://crrev.com/9febeb19dab5b13383b5b4145c0a3e6e0af82531/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/9febeb19dab5b13383b5b4145c0a3e6e0af82531/media/blink/webmediaplayer_impl_unittest.cc
[modify] https://crrev.com/9febeb19dab5b13383b5b4145c0a3e6e0af82531/media/mojo/services/media_metrics_provider.cc
[modify] https://crrev.com/9febeb19dab5b13383b5b4145c0a3e6e0af82531/media/mojo/services/media_metrics_provider.h
[modify] https://crrev.com/9febeb19dab5b13383b5b4145c0a3e6e0af82531/media/mojo/services/media_metrics_provider_unittest.cc
[modify] https://crrev.com/9febeb19dab5b13383b5b4145c0a3e6e0af82531/media/mojo/services/video_decode_perf_history.cc
[modify] https://crrev.com/9febeb19dab5b13383b5b4145c0a3e6e0af82531/media/mojo/services/video_decode_perf_history.h
[modify] https://crrev.com/9febeb19dab5b13383b5b4145c0a3e6e0af82531/media/mojo/services/video_decode_perf_history_unittest.cc
[modify] https://crrev.com/9febeb19dab5b13383b5b4145c0a3e6e0af82531/media/mojo/services/video_decode_stats_recorder.cc
[modify] https://crrev.com/9febeb19dab5b13383b5b4145c0a3e6e0af82531/media/mojo/services/video_decode_stats_recorder.h
[modify] https://crrev.com/9febeb19dab5b13383b5b4145c0a3e6e0af82531/media/mojo/services/watch_time_recorder_unittest.cc

Project Member

Comment 11 by bugdroid1@chromium.org, Jul 12

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

commit 9205ea89ebb4bd23b2f0099726f641a27ed269bb
Author: chcunningham <chcunningham@chromium.org>
Date: Thu Jul 12 18:04:20 2018

RELAND: InMemory MediaCapabilities DB for Guest/Incognito

Original CL was reverted:
https://chromium-review.googlesource.com/c/chromium/src/+/1092156

Reland now possible thanks to:
https://chromium-review.googlesource.com/c/chromium/src/+/1123396


Prior to this change, Guest mode has an empty read-only DB and Incongito
has a read-only DB. This CL allows both profiles to write to an "in
memory" database that disapears when the profile is destroyed.

This improves the experience for the user and avoids the possibility
of using MediaCapabilites to detect incognito/guest profiles.

Bug:  796280 ,  855631 , 855215
Change-Id: Ideb9dd2eabab2e594a24dd9b514557e473c3afc0
Reviewed-on: https://chromium-review.googlesource.com/1123687
Commit-Queue: Mounir Lamouri <mlamouri@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574633}
[modify] https://crrev.com/9205ea89ebb4bd23b2f0099726f641a27ed269bb/chrome/browser/DEPS
[modify] https://crrev.com/9205ea89ebb4bd23b2f0099726f641a27ed269bb/chrome/browser/profiles/off_the_record_profile_impl.cc
[modify] https://crrev.com/9205ea89ebb4bd23b2f0099726f641a27ed269bb/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/9205ea89ebb4bd23b2f0099726f641a27ed269bb/media/capabilities/BUILD.gn
[add] https://crrev.com/9205ea89ebb4bd23b2f0099726f641a27ed269bb/media/capabilities/in_memory_video_decode_stats_db_impl.cc
[add] https://crrev.com/9205ea89ebb4bd23b2f0099726f641a27ed269bb/media/capabilities/in_memory_video_decode_stats_db_impl.h
[add] https://crrev.com/9205ea89ebb4bd23b2f0099726f641a27ed269bb/media/capabilities/in_memory_video_decode_stats_db_unittest.cc
[modify] https://crrev.com/9205ea89ebb4bd23b2f0099726f641a27ed269bb/media/capabilities/video_decode_stats_db.cc
[modify] https://crrev.com/9205ea89ebb4bd23b2f0099726f641a27ed269bb/media/capabilities/video_decode_stats_db.h
[modify] https://crrev.com/9205ea89ebb4bd23b2f0099726f641a27ed269bb/media/capabilities/video_decode_stats_db_impl.cc
[modify] https://crrev.com/9205ea89ebb4bd23b2f0099726f641a27ed269bb/media/capabilities/video_decode_stats_db_impl.h
[add] https://crrev.com/9205ea89ebb4bd23b2f0099726f641a27ed269bb/media/capabilities/video_decode_stats_db_provider.cc
[add] https://crrev.com/9205ea89ebb4bd23b2f0099726f641a27ed269bb/media/capabilities/video_decode_stats_db_provider.h
[modify] https://crrev.com/9205ea89ebb4bd23b2f0099726f641a27ed269bb/media/capabilities/video_decode_stats_db_unittest.cc
[modify] https://crrev.com/9205ea89ebb4bd23b2f0099726f641a27ed269bb/media/mojo/services/video_decode_perf_history.cc
[modify] https://crrev.com/9205ea89ebb4bd23b2f0099726f641a27ed269bb/media/mojo/services/video_decode_perf_history.h
[modify] https://crrev.com/9205ea89ebb4bd23b2f0099726f641a27ed269bb/media/mojo/services/video_decode_perf_history_unittest.cc

Status: Fixed (was: Assigned)

Sign in to add a comment