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

Issue 579673003: Move content_settings_observer.h, content_settings_provider.h, content_settings_rule.* to the compo… (Closed)

Created:
6 years, 3 months ago by vasilii
Modified:
6 years, 3 months ago
CC:
chromium-apps-reviews_chromium.org, chromium-reviews, dbeam+watch-options_chromium.org, David Black, dhollowa+watch_chromium.org, dominich, donnd+watch_chromium.org, dougw+watch_chromium.org, extensions-reviews_chromium.org, feature-media-reviews_chromium.org, Jered, jfweitz+watch_chromium.org, kmadhusu+watch_chromium.org, markusheintz_, mcasas+watch_chromium.org, melevin+watch_chromium.org, posciak+watch_chromium.org, samarth+watch_chromium.org, skanuj+watch_chromium.org, wjia+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

Move content_settings_observer.h, content_settings_provider.h, content_settings_rule.* to the component. BUG=384872, 384865 Committed: https://crrev.com/ac4613900b6ccb3de45b438c941d8b71a247187e Cr-Commit-Position: refs/heads/master@{#295455}

Patch Set 1 #

Total comments: 2

Patch Set 2 : remove spaces #

Patch Set 3 : fix chrome/browser/BUILD.gn #

Total comments: 13

Patch Set 4 : add DEPS #

Total comments: 2

Patch Set 5 : last comment #

Unified diffs Side-by-side diffs Delta from patch set Stats (+111 lines, -472 lines) Patch
M chrome/browser/BUILD.gn View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/chrome_content_browser_client.cc View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/content_settings/content_settings_default_provider.cc View 1 chunk +1 line, -1 line 0 comments Download
D chrome/browser/content_settings/content_settings_details.h View 1 chunk +0 lines, -67 lines 0 comments Download
D chrome/browser/content_settings/content_settings_details.cc View 1 chunk +0 lines, -16 lines 0 comments Download
M chrome/browser/content_settings/content_settings_internal_extension_provider.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/content_settings/content_settings_mock_observer.h View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/content_settings/content_settings_observable_provider.h View 1 chunk +2 lines, -2 lines 0 comments Download
D chrome/browser/content_settings/content_settings_observer.h View 1 chunk +0 lines, -34 lines 0 comments Download
M chrome/browser/content_settings/content_settings_origin_identifier_value_map.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/content_settings/content_settings_origin_identifier_value_map_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/content_settings/content_settings_policy_provider.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/content_settings/content_settings_policy_provider_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/content_settings/content_settings_pref_provider.cc View 1 chunk +1 line, -1 line 0 comments Download
D chrome/browser/content_settings/content_settings_provider.h View 1 chunk +0 lines, -76 lines 0 comments Download
D chrome/browser/content_settings/content_settings_rule.h View 1 chunk +0 lines, -63 lines 0 comments Download
D chrome/browser/content_settings/content_settings_rule.cc View 1 chunk +0 lines, -69 lines 0 comments Download
D chrome/browser/content_settings/content_settings_rule_unittest.cc View 1 chunk +0 lines, -82 lines 0 comments Download
M chrome/browser/content_settings/content_settings_utils.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/content_settings/host_content_settings_map.h View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/content_settings/host_content_settings_map.cc View 1 chunk +3 lines, -3 lines 0 comments Download
M chrome/browser/content_settings/host_content_settings_map_unittest.cc View 2 chunks +1 line, -1 line 0 comments Download
M chrome/browser/content_settings/mock_settings_observer.h View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/content_settings/mock_settings_observer.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/content_settings/tab_specific_content_settings.h View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/content_settings/tab_specific_content_settings.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/extensions/api/content_settings/content_settings_store.h View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/extensions/api/content_settings/content_settings_store.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/extensions/api/content_settings/content_settings_store_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/media/media_capture_devices_dispatcher.cc View 2 chunks +1 line, -1 line 0 comments Download
M chrome/browser/media/media_stream_devices_controller.cc View 2 chunks +1 line, -1 line 0 comments Download
M chrome/browser/notifications/desktop_notification_profile_util.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/notifications/desktop_notification_service.h View 1 2 3 2 chunks +0 lines, -2 lines 0 comments Download
M chrome/browser/notifications/desktop_notification_service.cc View 1 2 3 1 chunk +0 lines, -2 lines 0 comments Download
M chrome/browser/profiles/profile_io_data.cc View 2 chunks +1 line, -1 line 0 comments Download
M chrome/browser/ui/search/instant_controller.cc View 1 2 3 2 chunks +0 lines, -3 lines 0 comments Download
M chrome/browser/ui/website_settings/website_settings_unittest.cc View 1 2 3 1 chunk +0 lines, -3 lines 0 comments Download
M chrome/browser/ui/webui/options/content_settings_handler.h View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/webui/options/content_settings_handler.cc View 1 2 3 4 2 chunks +1 line, -2 lines 0 comments Download
M chrome/browser/ui/webui/options/website_settings_handler.h View 1 chunk +1 line, -1 line 0 comments Download
M chrome/chrome_browser.gypi View 2 chunks +1 line, -6 lines 0 comments Download
M chrome/chrome_tests_unit.gypi View 1 chunk +0 lines, -1 line 0 comments Download
M components/BUILD.gn View 2 chunks +3 lines, -0 lines 0 comments Download
M components/components_tests.gyp View 2 chunks +2 lines, -0 lines 0 comments Download
M components/content_settings.gypi View 1 1 chunk +22 lines, -0 lines 0 comments Download
A components/content_settings/core/DEPS View 1 2 3 1 chunk +4 lines, -0 lines 0 comments Download
A components/content_settings/core/browser/BUILD.gn View 1 chunk +31 lines, -0 lines 0 comments Download
A + components/content_settings/core/browser/content_settings_details.h View 2 chunks +3 lines, -3 lines 0 comments Download
A + components/content_settings/core/browser/content_settings_details.cc View 1 chunk +1 line, -1 line 0 comments Download
A + components/content_settings/core/browser/content_settings_observer.h View 2 chunks +3 lines, -3 lines 0 comments Download
A + components/content_settings/core/browser/content_settings_provider.h View 2 chunks +3 lines, -4 lines 0 comments Download
A + components/content_settings/core/browser/content_settings_rule.h View 2 chunks +3 lines, -3 lines 0 comments Download
A + components/content_settings/core/browser/content_settings_rule.cc View 1 chunk +1 line, -1 line 0 comments Download
A + components/content_settings/core/browser/content_settings_rule_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 21 (4 generated)
vasilii
Hi guys, please review.
6 years, 3 months ago (2014-09-17 11:44:29 UTC) #4
Bernhard Bauer
LGTM Generally, it's helpful to point out where the substantial changes are (as opposed to ...
6 years, 3 months ago (2014-09-17 12:17:20 UTC) #5
vasilii
Substantial changes are in BUILD.gn and gyp/gypi files. I want to get an OWNER approval ...
6 years, 3 months ago (2014-09-17 12:43:05 UTC) #6
blundell
This CL overlaps with https://codereview.chromium.org/544343002/ it looks like.
6 years, 3 months ago (2014-09-17 13:03:29 UTC) #7
vasilii
On 2014/09/17 13:03:29, blundell wrote: > This CL overlaps with https://codereview.chromium.org/544343002/ it looks like. Yes, ...
6 years, 3 months ago (2014-09-17 13:19:33 UTC) #8
jochen (gone - plz use gerrit)
please add a DEPS file to components/content_settings/core that disallows common/ to include browser/
6 years, 3 months ago (2014-09-17 19:07:18 UTC) #9
jochen (gone - plz use gerrit)
lgtm with that change
6 years, 3 months ago (2014-09-17 19:08:03 UTC) #10
blundell
//components LGTM
6 years, 3 months ago (2014-09-18 06:24:42 UTC) #11
vabr (Chromium)
LGTM, once the DEPS requested by Jochen are added. Thanks! Vaclav
6 years, 3 months ago (2014-09-18 08:43:35 UTC) #12
vabr (Chromium)
FTR, this also seems to cover changes in https://codereview.chromium.org/430813004/
6 years, 3 months ago (2014-09-18 08:47:41 UTC) #13
vabr (Chromium)
A couple of more suggestions based on the partly reviewed https://codereview.chromium.org/430813004/. https://codereview.chromium.org/579673003/diff/40001/chrome/browser/notifications/desktop_notification_service.cc File chrome/browser/notifications/desktop_notification_service.cc (right): ...
6 years, 3 months ago (2014-09-18 08:54:57 UTC) #14
vasilii
Vaclav, have a look again. https://codereview.chromium.org/579673003/diff/40001/chrome/browser/notifications/desktop_notification_service.cc File chrome/browser/notifications/desktop_notification_service.cc (right): https://codereview.chromium.org/579673003/diff/40001/chrome/browser/notifications/desktop_notification_service.cc#newcode26 chrome/browser/notifications/desktop_notification_service.cc:26: #include "components/content_settings/core/browser/content_settings_provider.h" On 2014/09/18 ...
6 years, 3 months ago (2014-09-18 09:45:42 UTC) #15
vabr (Chromium)
LGTM, with one corrected comment. Thanks for teaching me the trick with the DEPS! ;-) ...
6 years, 3 months ago (2014-09-18 09:57:11 UTC) #16
vasilii
https://codereview.chromium.org/579673003/diff/60001/chrome/browser/ui/webui/options/content_settings_handler.cc File chrome/browser/ui/webui/options/content_settings_handler.cc (right): https://codereview.chromium.org/579673003/diff/60001/chrome/browser/ui/webui/options/content_settings_handler.cc#newcode35 chrome/browser/ui/webui/options/content_settings_handler.cc:35: #include "components/content_settings/core/browser/content_settings_provider.h" On 2014/09/18 09:57:10, vabr (Chromium) wrote: > ...
6 years, 3 months ago (2014-09-18 10:04:49 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patchset/579673003/80001
6 years, 3 months ago (2014-09-18 10:05:56 UTC) #19
commit-bot: I haz the power
Committed patchset #5 (id:80001) as 2e0f140804fc8be28e2678d3045d709c3799dfbe
6 years, 3 months ago (2014-09-18 11:35:28 UTC) #20
commit-bot: I haz the power
6 years, 3 months ago (2014-09-18 11:36:09 UTC) #21
Message was sent while issue was closed.
Patchset 5 (id:??) landed as
https://crrev.com/ac4613900b6ccb3de45b438c941d8b71a247187e
Cr-Commit-Position: refs/heads/master@{#295455}

Powered by Google App Engine
This is Rietveld 408576698