Migrate ContentSettingPatternSource.source to SettingSource |
|||||
Issue descriptionThe |source| field in ContentSettingPatternSource exposes the content setting provider where a given setting came from, instead of the conceptual source SettingSource. This breaks in cases where settings are distributed over multiple providers (e.g. preferences and notification_android, aka issue 762515 ). It's also brittle (and slightly wasteful in terms of memory) to pass raw strings around instead of identifiers that could be checked at compile time. What we should do is: 1) Add a new field |setting_source| to ContentSettingPatternSource and populate that in HostContentSettingsMap::AddSettingsForOneType() with the provider_source. 2) Migrate existing clients over to use |setting_source| instead of |source|. 3) Remove |source| (unless it's used for sth like debugging)
,
Feb 18 2018
,
Jul 26
rhalavati@: You added a new content settings provider. What does it use as "source" parameter? We might need to add it to the list of user defined settings [1] or finally fix this bug :) https://cs.chromium.org/chromium/src/chrome/browser/browsing_data/counters/site_settings_counter.cc?l=55&rcl=138f2bbb67216d68bfc2c1ba95806bbcd11300cb
,
Jul 26
,
Jul 31
#3: It uses 'ephemeral': https://cs.chromium.org/chromium/src/components/content_settings/core/browser/host_content_settings_map.cc?rcl=1f75f99ff2f94234223b089297b66ecd791906ee&l=69 I think it would be good to add a function that tells if a provider has a specific feature. E.g., IsUserPreference() instead of checking if the source is 'preference' or 'notification_android' or 'ephemeral'.
,
Jul 31
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/af76c0eed30c1026acb4a160659081d5a5847c8c commit af76c0eed30c1026acb4a160659081d5a5847c8c Author: Ramin Halavati <rhalavati@chromium.org> Date: Tue Jul 31 09:17:15 2018 Add Ephemeral provider to site settings counter. Ephemeral provider is added to the list of content settings sources that are counted in site settings change counter. Bug: 762560 Change-Id: I526f7280b61b4f777560c58a99cf5148bd9b9ff9 Reviewed-on: https://chromium-review.googlesource.com/1155603 Reviewed-by: Christian Dullweber <dullweber@chromium.org> Commit-Queue: Ramin Halavati <rhalavati@chromium.org> Cr-Commit-Position: refs/heads/master@{#579355} [modify] https://crrev.com/af76c0eed30c1026acb4a160659081d5a5847c8c/chrome/browser/browsing_data/counters/site_settings_counter.cc
,
Aug 2
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by est...@chromium.org
, Nov 10 2017