Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(927)

Issue 542253003: Add a global on/off switch for content settings and expose a toggle on the Website Settings options… (Closed)

Created:
6 years, 3 months ago by Daniel Nishi
Modified:
6 years, 3 months ago
CC:
scheib, chromium-reviews, dbeam+watch-options_chromium.org, markusheintz_, arv+watch_chromium.org, Jun Mukai
Base URL:
https://chromium.googlesource.com/chromium/src.git@global-settings
Project:
chromium
Visibility:
Public.

Description

Add a global on/off switch for content settings and expose a toggle on the Website Settings options page. TBR=miguelg@chromium.org BUG=372607 Committed: https://crrev.com/d5770d216672950727a76e546aab7b610f78050b Cr-Commit-Position: refs/heads/master@{#295572}

Patch Set 1 #

Total comments: 26

Patch Set 2 : #

Patch Set 3 : Forgot to clarify ownership comments. #

Total comments: 6

Patch Set 4 : Now a real provider. #

Patch Set 5 : Forgot UsedContentSettingsProviders(). #

Total comments: 12

Patch Set 6 : Values removed from the OverrideProvider, now using an array. #

Total comments: 11

Patch Set 7 : #

Total comments: 2

Patch Set 8 : #

Total comments: 4

Patch Set 9 : #

Patch Set 10 : #

Patch Set 11 : #

Patch Set 12 : Rebase! #

Patch Set 13 : #

Patch Set 14 : Rebase. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+558 lines, -100 lines) Patch
M chrome/app/generated_resources.grd View 1 chunk +4 lines, -0 lines 0 comments Download
M chrome/browser/android/banners/app_banner_settings_helper.cc View 1 2 chunks +4 lines, -4 lines 0 comments Download
M chrome/browser/chrome_content_browser_client.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +3 lines, -2 lines 0 comments Download
A chrome/browser/content_settings/content_settings_override_provider.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +79 lines, -0 lines 0 comments Download
A chrome/browser/content_settings/content_settings_override_provider.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +134 lines, -0 lines 0 comments Download
M chrome/browser/content_settings/cookie_settings.cc View 1 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/content_settings/host_content_settings_map.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 6 chunks +39 lines, -2 lines 0 comments Download
M chrome/browser/content_settings/host_content_settings_map.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 13 chunks +111 lines, -32 lines 0 comments Download
M chrome/browser/content_settings/host_content_settings_map_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +67 lines, -0 lines 0 comments Download
M chrome/browser/plugins/plugin_info_message_filter.cc View 1 2 3 4 5 6 7 8 10 11 1 chunk +18 lines, -10 lines 0 comments Download
M chrome/browser/resources/options/website_settings.css View 1 2 3 4 5 6 7 8 9 10 11 13 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/resources/options/website_settings.html View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +6 lines, -0 lines 0 comments Download
M chrome/browser/resources/options/website_settings.js View 1 2 3 4 5 6 7 8 9 10 11 13 4 chunks +11 lines, -3 lines 0 comments Download
M chrome/browser/ui/android/content_settings/popup_blocked_infobar_delegate.cc View 1 2 3 4 5 6 7 8 10 11 1 chunk +2 lines, -7 lines 0 comments Download
M chrome/browser/ui/content_settings/content_setting_bubble_model.cc View 1 2 3 4 5 6 7 8 10 11 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/ui/website_settings/website_settings.cc View 1 2 3 4 5 6 7 8 9 10 3 chunks +20 lines, -16 lines 0 comments Download
M chrome/browser/ui/webui/options/website_settings_handler.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +4 lines, -1 line 0 comments Download
M chrome/browser/ui/webui/options/website_settings_handler.cc View 1 2 3 4 5 6 7 8 9 10 11 13 7 chunks +43 lines, -19 lines 0 comments Download
M chrome/chrome_browser.gypi View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/common/pref_names.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +1 line, -0 lines 0 comments Download
M chrome/common/pref_names.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +5 lines, -0 lines 0 comments Download

Messages

Total messages: 32 (8 generated)
Daniel Nishi
gbillock@chromium.org: Please review changes in chrome/browser/ui/website_settings/website_settings.cc. bauerb@chromium.org: Please review changes in rest. Thanks!
6 years, 3 months ago (2014-09-10 01:16:34 UTC) #2
Bernhard Bauer
+Dominic FYI https://codereview.chromium.org/542253003/diff/1/chrome/browser/content_settings/content_settings_override_provider.cc File chrome/browser/content_settings/content_settings_override_provider.cc (right): https://codereview.chromium.org/542253003/diff/1/chrome/browser/content_settings/content_settings_override_provider.cc#newcode26 chrome/browser/content_settings/content_settings_override_provider.cc:26: base::DictionaryValue* override_content_settings = Inline this? Actually, there ...
6 years, 3 months ago (2014-09-10 09:07:18 UTC) #3
Daniel Nishi
https://codereview.chromium.org/542253003/diff/1/chrome/browser/content_settings/content_settings_override_provider.cc File chrome/browser/content_settings/content_settings_override_provider.cc (right): https://codereview.chromium.org/542253003/diff/1/chrome/browser/content_settings/content_settings_override_provider.cc#newcode26 chrome/browser/content_settings/content_settings_override_provider.cc:26: base::DictionaryValue* override_content_settings = On 2014/09/10 09:07:17, Bernhard Bauer wrote: ...
6 years, 3 months ago (2014-09-10 16:38:29 UTC) #4
Bernhard Bauer
https://codereview.chromium.org/542253003/diff/1/chrome/browser/content_settings/content_settings_override_provider.cc File chrome/browser/content_settings/content_settings_override_provider.cc (right): https://codereview.chromium.org/542253003/diff/1/chrome/browser/content_settings/content_settings_override_provider.cc#newcode50 chrome/browser/content_settings/content_settings_override_provider.cc:50: if (is_incognito_) On 2014/09/10 16:38:28, Daniel Nishi wrote: > ...
6 years, 3 months ago (2014-09-10 17:44:54 UTC) #5
Daniel Nishi
https://codereview.chromium.org/542253003/diff/1/chrome/browser/content_settings/content_settings_override_provider.cc File chrome/browser/content_settings/content_settings_override_provider.cc (right): https://codereview.chromium.org/542253003/diff/1/chrome/browser/content_settings/content_settings_override_provider.cc#newcode61 chrome/browser/content_settings/content_settings_override_provider.cc:61: bool OverrideProvider::IsEnabled(ContentSettingsType content_type) const { On 2014/09/10 17:44:53, Bernhard ...
6 years, 3 months ago (2014-09-10 17:55:53 UTC) #6
Daniel Nishi
https://codereview.chromium.org/542253003/diff/1/chrome/browser/content_settings/content_settings_override_provider.cc File chrome/browser/content_settings/content_settings_override_provider.cc (right): https://codereview.chromium.org/542253003/diff/1/chrome/browser/content_settings/content_settings_override_provider.cc#newcode50 chrome/browser/content_settings/content_settings_override_provider.cc:50: if (is_incognito_) On 2014/09/10 17:44:53, Bernhard Bauer wrote: > ...
6 years, 3 months ago (2014-09-10 23:30:21 UTC) #7
Bernhard Bauer
https://codereview.chromium.org/542253003/diff/1/chrome/browser/content_settings/content_settings_override_provider.cc File chrome/browser/content_settings/content_settings_override_provider.cc (right): https://codereview.chromium.org/542253003/diff/1/chrome/browser/content_settings/content_settings_override_provider.cc#newcode61 chrome/browser/content_settings/content_settings_override_provider.cc:61: bool OverrideProvider::IsEnabled(ContentSettingsType content_type) const { On 2014/09/10 17:55:52, Daniel ...
6 years, 3 months ago (2014-09-11 16:49:57 UTC) #8
Daniel Nishi
https://codereview.chromium.org/542253003/diff/80001/chrome/browser/content_settings/content_settings_override_provider.cc File chrome/browser/content_settings/content_settings_override_provider.cc (right): https://codereview.chromium.org/542253003/diff/80001/chrome/browser/content_settings/content_settings_override_provider.cc#newcode45 chrome/browser/content_settings/content_settings_override_provider.cc:45: scoped_ptr<base::Value> value_; On 2014/09/11 16:49:56, Bernhard Bauer wrote: > ...
6 years, 3 months ago (2014-09-11 20:58:13 UTC) #9
Bernhard Bauer
Heads-up: https://codereview.chromium.org/545413002/ might change how content setting providers are registered. https://codereview.chromium.org/542253003/diff/100001/chrome/browser/content_settings/content_settings_override_provider.cc File chrome/browser/content_settings/content_settings_override_provider.cc (right): https://codereview.chromium.org/542253003/diff/100001/chrome/browser/content_settings/content_settings_override_provider.cc#newcode35 ...
6 years, 3 months ago (2014-09-12 09:37:38 UTC) #10
Daniel Nishi
https://codereview.chromium.org/542253003/diff/100001/chrome/browser/content_settings/content_settings_override_provider.cc File chrome/browser/content_settings/content_settings_override_provider.cc (right): https://codereview.chromium.org/542253003/diff/100001/chrome/browser/content_settings/content_settings_override_provider.cc#newcode35 chrome/browser/content_settings/content_settings_override_provider.cc:35: is_done_ = false; On 2014/09/12 09:37:38, Bernhard Bauer wrote: ...
6 years, 3 months ago (2014-09-12 16:40:36 UTC) #11
Bernhard Bauer
https://codereview.chromium.org/542253003/diff/100001/chrome/browser/content_settings/content_settings_override_provider.cc File chrome/browser/content_settings/content_settings_override_provider.cc (right): https://codereview.chromium.org/542253003/diff/100001/chrome/browser/content_settings/content_settings_override_provider.cc#newcode133 chrome/browser/content_settings/content_settings_override_provider.cc:133: for (size_t type = 0; type < CONTENT_SETTINGS_NUM_TYPES; ++type) ...
6 years, 3 months ago (2014-09-12 17:16:37 UTC) #12
Daniel Nishi
https://codereview.chromium.org/542253003/diff/120001/chrome/browser/content_settings/content_settings_override_provider.cc File chrome/browser/content_settings/content_settings_override_provider.cc (right): https://codereview.chromium.org/542253003/diff/120001/chrome/browser/content_settings/content_settings_override_provider.cc#newcode130 chrome/browser/content_settings/content_settings_override_provider.cc:130: allowed_settings_[content_setting] = false; On 2014/09/12 17:16:36, Bernhard Bauer wrote: ...
6 years, 3 months ago (2014-09-12 19:51:43 UTC) #13
Bernhard Bauer
LGTM with nits: https://codereview.chromium.org/542253003/diff/140001/chrome/browser/content_settings/content_settings_override_provider.cc File chrome/browser/content_settings/content_settings_override_provider.cc (right): https://codereview.chromium.org/542253003/diff/140001/chrome/browser/content_settings/content_settings_override_provider.cc#newcode113 chrome/browser/content_settings/content_settings_override_provider.cc:113: GetTypeName(content_type), new base::FundamentalValue(true)); You don't actually ...
6 years, 3 months ago (2014-09-15 09:17:16 UTC) #14
Daniel Nishi
gbillock: Ping! :) https://codereview.chromium.org/542253003/diff/140001/chrome/browser/content_settings/content_settings_override_provider.cc File chrome/browser/content_settings/content_settings_override_provider.cc (right): https://codereview.chromium.org/542253003/diff/140001/chrome/browser/content_settings/content_settings_override_provider.cc#newcode113 chrome/browser/content_settings/content_settings_override_provider.cc:113: GetTypeName(content_type), new base::FundamentalValue(true)); On 2014/09/15 09:17:16, ...
6 years, 3 months ago (2014-09-15 17:40:51 UTC) #15
Greg Billock
On 2014/09/15 17:40:51, Daniel Nishi wrote: > gbillock: Ping! :) > > https://codereview.chromium.org/542253003/diff/140001/chrome/browser/content_settings/content_settings_override_provider.cc > File ...
6 years, 3 months ago (2014-09-17 21:59:19 UTC) #16
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patchset/542253003/200001
6 years, 3 months ago (2014-09-17 22:02:03 UTC) #18
commit-bot: I haz the power
Try jobs failed on following builders: mac_gpu on tryserver.chromium.gpu (http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/57392) ios_dbg_simulator on tryserver.chromium.mac (http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator/builds/16052) mac_chromium_compile_dbg ...
6 years, 3 months ago (2014-09-17 22:05:51 UTC) #20
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patchset/542253003/220001
6 years, 3 months ago (2014-09-17 22:26:52 UTC) #22
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/11715)
6 years, 3 months ago (2014-09-17 22:40:29 UTC) #24
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patchset/542253003/220001
6 years, 3 months ago (2014-09-17 22:54:21 UTC) #26
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_rel_swarming on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_swarming/builds/17361)
6 years, 3 months ago (2014-09-18 00:12:47 UTC) #28
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/542253003/260001
6 years, 3 months ago (2014-09-18 21:29:41 UTC) #30
commit-bot: I haz the power
Committed patchset #14 (id:260001) as a8f1e8b78993a3c06d3a76bb7f5bd518522961e1
6 years, 3 months ago (2014-09-18 21:59:26 UTC) #31
commit-bot: I haz the power
6 years, 3 months ago (2014-09-18 22:00:02 UTC) #32
Message was sent while issue was closed.
Patchset 14 (id:??) landed as
https://crrev.com/d5770d216672950727a76e546aab7b610f78050b
Cr-Commit-Position: refs/heads/master@{#295572}

Powered by Google App Engine
This is Rietveld 408576698