Move content_settings_observer.h, content_settings_provider.h, content_settings_rule.* to the compo…

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}

[vasilii, 2014-09-17 11:41:58]

vasilii@chromium.org changed reviewers:
+ blundell@chromium.org

[vasilii, 2014-09-17 11:42:31]

vasilii@chromium.org changed reviewers:
+ bauerb@chromium.org

[vasilii, 2014-09-17 11:44:27]

vasilii@chromium.org changed reviewers:
+ jochen@chromium.org, vabr@chromium.org

[vasilii, 2014-09-17 11:44:29]

Hi guys,

please review.

[Bernhard Bauer, 2014-09-17 12:17:20]

LGTM

Generally, it's helpful to point out where the substantial changes are (as
opposed to mechanical ones like pure header renames), and who should look at
what (unless you actually want the full cross product of reviewers x files).

https://codereview.chromium.org/579673003/diff/1/components/content_settings....
File components/content_settings.gypi (right):

https://codereview.chromium.org/579673003/diff/1/components/content_settings....
components/content_settings.gypi:52: },
EOL whitespace?

[vasilii, 2014-09-17 12:43:05]

Substantial changes are in BUILD.gn and gyp/gypi files. I want to get an OWNER
approval from:
jochen@: chrome/browser
blundell@: components/

https://codereview.chromium.org/579673003/diff/1/components/content_settings....
File components/content_settings.gypi (right):

https://codereview.chromium.org/579673003/diff/1/components/content_settings....
components/content_settings.gypi:52: },
On 2014/09/17 12:17:20, Bernhard Bauer wrote:
> EOL whitespace?

Done.

[blundell, 2014-09-17 13:03:29]

This CL overlaps with https://codereview.chromium.org/544343002/ it looks like.

[vasilii, 2014-09-17 13:19:33]

On 2014/09/17 13:03:29, blundell wrote:
> This CL overlaps with https://codereview.chromium.org/544343002/ it looks
like.

Yes, though I move more files.

[jochen (gone - plz use gerrit), 2014-09-17 19:07:18]

please add a DEPS file to components/content_settings/core that disallows
common/ to include browser/

[jochen (gone - plz use gerrit), 2014-09-17 19:08:03]

lgtm with that change

[blundell, 2014-09-18 06:24:42]

//components LGTM

[vabr (Chromium), 2014-09-18 08:43:35]

LGTM, once the DEPS requested by Jochen are added.
Thanks!
Vaclav

[vabr (Chromium), 2014-09-18 08:47:41]

FTR, this also seems to cover changes in
https://codereview.chromium.org/430813004/

[vabr (Chromium), 2014-09-18 08:54:57]

A couple of more suggestions based on the partly reviewed
https://codereview.chromium.org/430813004/.

https://codereview.chromium.org/579673003/diff/40001/chrome/browser/notificat...
File chrome/browser/notifications/desktop_notification_service.cc (right):

https://codereview.chromium.org/579673003/diff/40001/chrome/browser/notificat...
chrome/browser/notifications/desktop_notification_service.cc:26: #include
"components/content_settings/core/browser/content_settings_provider.h"
You can remove this line, see
https://codereview.chromium.org/430813004/diff/70001/chrome/browser/notificat...

https://codereview.chromium.org/579673003/diff/40001/chrome/browser/notificat...
File chrome/browser/notifications/desktop_notification_service.h (right):

https://codereview.chromium.org/579673003/diff/40001/chrome/browser/notificat...
chrome/browser/notifications/desktop_notification_service.h:22: #include
"components/content_settings/core/browser/content_settings_provider.h"
You can remove this line, and also a fwd declaration of ContentSettingsPattern
below, see
https://codereview.chromium.org/430813004/diff/70001/chrome/browser/notificat...

https://codereview.chromium.org/579673003/diff/40001/chrome/browser/ui/search...
File chrome/browser/ui/search/instant_controller.cc (left):

https://codereview.chromium.org/579673003/diff/40001/chrome/browser/ui/search...
chrome/browser/ui/search/instant_controller.cc:11: #include
"chrome/browser/content_settings/host_content_settings_map.h"
This can also be dropped, as in
https://codereview.chromium.org/430813004/diff/70001/chrome/browser/ui/search...

https://codereview.chromium.org/579673003/diff/40001/chrome/browser/ui/search...
File chrome/browser/ui/search/instant_controller.cc (right):

https://codereview.chromium.org/579673003/diff/40001/chrome/browser/ui/search...
chrome/browser/ui/search/instant_controller.cc:24: #include
"components/content_settings/core/browser/content_settings_provider.h"
Following up on
https://codereview.chromium.org/430813004/diff/50001/chrome/browser/ui/search...,
I think you can remove this line.

https://codereview.chromium.org/579673003/diff/40001/chrome/browser/ui/websit...
File chrome/browser/ui/website_settings/website_settings_unittest.cc (right):

https://codereview.chromium.org/579673003/diff/40001/chrome/browser/ui/websit...
chrome/browser/ui/website_settings/website_settings_unittest.cc:18: #include
"components/content_settings/core/browser/content_settings_provider.h"
Seems like it can be dropped, see
https://codereview.chromium.org/430813004/diff/70001/chrome/browser/ui/websit...

https://codereview.chromium.org/579673003/diff/40001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/options/content_settings_handler.h (right):

https://codereview.chromium.org/579673003/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/options/content_settings_handler.h:16: #include
"components/content_settings/core/browser/content_settings_observer.h"
Drop this, as in
https://codereview.chromium.org/430813004/diff/70001/chrome/browser/ui/webui/...

[vasilii, 2014-09-18 09:45:42]

Vaclav, have a look again.

https://codereview.chromium.org/579673003/diff/40001/chrome/browser/notificat...
File chrome/browser/notifications/desktop_notification_service.cc (right):

https://codereview.chromium.org/579673003/diff/40001/chrome/browser/notificat...
chrome/browser/notifications/desktop_notification_service.cc:26: #include
"components/content_settings/core/browser/content_settings_provider.h"
On 2014/09/18 08:54:57, vabr (Chromium) wrote:
> You can remove this line, see
>
https://codereview.chromium.org/430813004/diff/70001/chrome/browser/notificat...

Done.

https://codereview.chromium.org/579673003/diff/40001/chrome/browser/notificat...
File chrome/browser/notifications/desktop_notification_service.h (right):

https://codereview.chromium.org/579673003/diff/40001/chrome/browser/notificat...
chrome/browser/notifications/desktop_notification_service.h:22: #include
"components/content_settings/core/browser/content_settings_provider.h"
On 2014/09/18 08:54:57, vabr (Chromium) wrote:
> You can remove this line, and also a fwd declaration of ContentSettingsPattern
> below, see
>
https://codereview.chromium.org/430813004/diff/70001/chrome/browser/notificat...

Done.

https://codereview.chromium.org/579673003/diff/40001/chrome/browser/ui/search...
File chrome/browser/ui/search/instant_controller.cc (left):

https://codereview.chromium.org/579673003/diff/40001/chrome/browser/ui/search...
chrome/browser/ui/search/instant_controller.cc:11: #include
"chrome/browser/content_settings/host_content_settings_map.h"
On 2014/09/18 08:54:57, vabr (Chromium) wrote:
> This can also be dropped, as in
>
https://codereview.chromium.org/430813004/diff/70001/chrome/browser/ui/search...

Done.

https://codereview.chromium.org/579673003/diff/40001/chrome/browser/ui/search...
File chrome/browser/ui/search/instant_controller.cc (right):

https://codereview.chromium.org/579673003/diff/40001/chrome/browser/ui/search...
chrome/browser/ui/search/instant_controller.cc:24: #include
"components/content_settings/core/browser/content_settings_provider.h"
On 2014/09/18 08:54:57, vabr (Chromium) wrote:
> Following up on
>
https://codereview.chromium.org/430813004/diff/50001/chrome/browser/ui/search...,
> I think you can remove this line.

Done.

https://codereview.chromium.org/579673003/diff/40001/chrome/browser/ui/websit...
File chrome/browser/ui/website_settings/website_settings_unittest.cc (right):

https://codereview.chromium.org/579673003/diff/40001/chrome/browser/ui/websit...
chrome/browser/ui/website_settings/website_settings_unittest.cc:18: #include
"components/content_settings/core/browser/content_settings_provider.h"
On 2014/09/18 08:54:57, vabr (Chromium) wrote:
> Seems like it can be dropped, see
>
https://codereview.chromium.org/430813004/diff/70001/chrome/browser/ui/websit...

Done.

https://codereview.chromium.org/579673003/diff/40001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/options/content_settings_handler.h (right):

https://codereview.chromium.org/579673003/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/options/content_settings_handler.h:16: #include
"components/content_settings/core/browser/content_settings_observer.h"
On 2014/09/18 08:54:57, vabr (Chromium) wrote:
> Drop this, as in
>
https://codereview.chromium.org/430813004/diff/70001/chrome/browser/ui/webui/...

Used in line 29.

[vabr (Chromium), 2014-09-18 09:57:11]

LGTM, with one corrected comment.

Thanks for teaching me the trick with the DEPS! ;-)

Cheers,
Vaclav

https://codereview.chromium.org/579673003/diff/40001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/options/content_settings_handler.h (right):

https://codereview.chromium.org/579673003/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/options/content_settings_handler.h:16: #include
"components/content_settings/core/browser/content_settings_observer.h"
On 2014/09/18 09:45:41, vasilii wrote:
> On 2014/09/18 08:54:57, vabr (Chromium) wrote:
> > Drop this, as in
> >
>
https://codereview.chromium.org/430813004/diff/70001/chrome/browser/ui/webui/...
> 
> Used in line 29.

Sorry, I put the comment in a wrong file. Corrected below.

https://codereview.chromium.org/579673003/diff/60001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/options/content_settings_handler.cc (right):

https://codereview.chromium.org/579673003/diff/60001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/options/content_settings_handler.cc:35: #include
"components/content_settings/core/browser/content_settings_provider.h"
You can drop this line, as in
https://codereview.chromium.org/430813004/diff/70001/chrome/browser/ui/webui/...

[vasilii, 2014-09-18 10:04:49]

https://codereview.chromium.org/579673003/diff/60001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/options/content_settings_handler.cc (right):

https://codereview.chromium.org/579673003/diff/60001/chrome/browser/ui/webui/...
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:
> You can drop this line, as in
>
https://codereview.chromium.org/430813004/diff/70001/chrome/browser/ui/webui/...

Done.

[vasilii, 2014-09-18 10:05:09]

The CQ bit was checked by vasilii@chromium.org

[commit-bot: I haz the power, 2014-09-18 10:05:56]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/579673003/80001

[commit-bot: I haz the power, 2014-09-18 11:35:28]

Committed patchset #5 (id:80001) as 2e0f140804fc8be28e2678d3045d709c3799dfbe

[commit-bot: I haz the power, 2014-09-18 11:36:09]

Patchset 5 (id:??) landed as
https://crrev.com/ac4613900b6ccb3de45b438c941d8b71a247187e
Cr-Commit-Position: refs/heads/master@{#295455}