Issue metadata
Sign in to add a comment
|
It should be hard to implement a new ContentSettingsProvider without considering Incognito mode |
||||||||||||||||||||||
Issue descriptionRight now it's the responsibility of every ContentSettingsProvider to remember to handle incognito mode properly, which is too easy to get wrong (see https://docs.google.com/document/d/1laGXNTTeUvRkVJ-qBTdC1DCjpragae_yEjnm0EkNPO4/edit?ts=5af1c5b0). It should be hard to implement a new ContentSettingsProvider without considering Incognito mode, e.g. the 'SetWebsiteSetting' method should take an 'isIncognito' boolean parameter, or there should be a method on the interface which implementations must implement to return true or false depending on whether they handle incognito settings or not, the then only permission updates in incognito mode should be sent to the providers which explicitly handle incognito content settings.
,
May 9 2018
There is a test that checks if all possible content settings types are deletable: https://cs.chromium.org/chromium/src/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc?l=2963&rcl=9f11faaa13c0238b5d29cfe312149987119bca92 We could write a similar test that checks if all content settings types, when handed to an incognito profile, are not visible to regular mode. That would handle content setting specific providers. This still wouldn't catch other kinds of providers
,
May 11 2018
Assigning to rhalavati@ who is already looking at Incognito hardening from all angles.
,
Nov 16
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3f9e831d443b970e6c6d1ba1574f25587e355415 commit 3f9e831d443b970e6c6d1ba1574f25587e355415 Author: Ramin Halavati <rhalavati@chromium.org> Date: Fri Nov 16 11:46:32 2018 Add Incognito persistency test to host content settings map unittest. A test is added to ensure that changing content settings in incognito mode does not affect the regular mode. Bug: 841412 Change-Id: I473ba39a1f37e69ef5682c2856e4fe3f62e03681 Reviewed-on: https://chromium-review.googlesource.com/c/1286419 Reviewed-by: Martin Šrámek <msramek@chromium.org> Commit-Queue: Ramin Halavati <rhalavati@chromium.org> Cr-Commit-Position: refs/heads/master@{#608734} [modify] https://crrev.com/3f9e831d443b970e6c6d1ba1574f25587e355415/chrome/browser/content_settings/host_content_settings_map_unittest.cc |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by awdf@chromium.org
, May 9 2018