Create //components/ntp_tiles.

Create //components/ntp_tiles.

Move its prefs and switches in immediately. Later will follow
//chrome/browser/android/ntp/{most_visited_sites,popular_sites}*, as the
work to refactor out their dependencies is ongoing.

BUG=603026
Committed: https://crrev.com/0dc4da510e5dbbf2a6197c0eb03500d64568652a
Cr-Commit-Position: refs/heads/master@{#390605}

[sfiera, 2016-04-28 11:43:40]

sfiera@chromium.org changed reviewers:
+ treib@chromium.org

[sfiera, 2016-04-28 11:43:41]

We'll need much wider approvals to get this in but I figured you should take a
look first. I wanted to get the component started with a minimal change.

[Marc Treib, 2016-04-28 12:43:34]

Looks like you're missing some dependencies in the GYP files, but otherwise
LGTM!

[sfiera, 2016-04-28 14:29:26]

sfiera@chromium.org changed reviewers:
+ jochen@chromium.org

[jochen (gone - plz use gerrit), 2016-04-28 15:46:37]

https://codereview.chromium.org/1928913002/diff/20001/chrome/browser/android/...
File chrome/browser/android/ntp/most_visited_sites.cc (right):

https://codereview.chromium.org/1928913002/diff/20001/chrome/browser/android/...
chrome/browser/android/ntp/most_visited_sites.cc:45: namespace switches =
ntp_tiles::switches;
I don't see this pattern anywhere else, and it looks confusing, I'd rather avoid
this.

https://codereview.chromium.org/1928913002/diff/20001/components/ntp_tiles.gypi
File components/ntp_tiles.gypi (right):

https://codereview.chromium.org/1928913002/diff/20001/components/ntp_tiles.gy...
components/ntp_tiles.gypi:8: # GN version: //components/ntp_tiles
where's the gyp unit test version?

https://codereview.chromium.org/1928913002/diff/20001/components/ntp_tiles/pr...
File components/ntp_tiles/pref_names.h (right):

https://codereview.chromium.org/1928913002/diff/20001/components/ntp_tiles/pr...
components/ntp_tiles/pref_names.h:11: extern const char kNTPSuggestionsURL[];
this is drop the buildflag check, is that intentional?

[sfiera, 2016-04-28 16:11:04]

https://codereview.chromium.org/1928913002/diff/20001/chrome/browser/android/...
File chrome/browser/android/ntp/most_visited_sites.cc (right):

https://codereview.chromium.org/1928913002/diff/20001/chrome/browser/android/...
chrome/browser/android/ntp/most_visited_sites.cc:45: namespace switches =
ntp_tiles::switches;
On 2016/04/28 15:46:37, jochen wrote:
> I don't see this pattern anywhere else, and it looks confusing, I'd rather
avoid
> this.

Done.

https://codereview.chromium.org/1928913002/diff/20001/components/ntp_tiles.gypi
File components/ntp_tiles.gypi (right):

https://codereview.chromium.org/1928913002/diff/20001/components/ntp_tiles.gy...
components/ntp_tiles.gypi:8: # GN version: //components/ntp_tiles
On 2016/04/28 15:46:37, jochen wrote:
> where's the gyp unit test version?

Do you mean in components_tests.gyp? There aren't yet any tests to list there,
so I haven't touched it yet.

https://codereview.chromium.org/1928913002/diff/20001/components/ntp_tiles/pr...
File components/ntp_tiles/pref_names.h (right):

https://codereview.chromium.org/1928913002/diff/20001/components/ntp_tiles/pr...
components/ntp_tiles/pref_names.h:11: extern const char kNTPSuggestionsURL[];
On 2016/04/28 15:46:37, jochen wrote:
> this is drop the buildflag check, is that intentional?

Sort of. In the long run it will definitely be dropped, since this component
will be used on all platforms. I would have kept it for the time being, except
that it would introduce a dependency on //chrome.

[jochen (gone - plz use gerrit), 2016-04-28 16:16:41]

https://codereview.chromium.org/1928913002/diff/60001/components/ntp_tiles/BU...
File components/ntp_tiles/BUILD.gn (right):

https://codereview.chromium.org/1928913002/diff/60001/components/ntp_tiles/BU...
components/ntp_tiles/BUILD.gn:19: source_set("unit_tests") {
delete this then?

[sfiera, 2016-04-28 16:27:59]

https://codereview.chromium.org/1928913002/diff/60001/components/ntp_tiles/BU...
File components/ntp_tiles/BUILD.gn (right):

https://codereview.chromium.org/1928913002/diff/60001/components/ntp_tiles/BU...
components/ntp_tiles/BUILD.gn:19: source_set("unit_tests") {
On 2016/04/28 16:16:41, jochen wrote:
> delete this then?

I'd done this for OWNERS. components_test.gyp lists all the tests, and has an
OWNERS exception for adding them. ../BUILD.gn references the list here, so I'd
have to get approval again to add tests later. I can come back to you for a
second approval next week if you'd prefer.

[jochen (gone - plz use gerrit), 2016-04-28 16:33:15]

On 2016/04/28 at 16:27:59, sfiera wrote:
>
https://codereview.chromium.org/1928913002/diff/60001/components/ntp_tiles/BU...
> File components/ntp_tiles/BUILD.gn (right):
> 
>
https://codereview.chromium.org/1928913002/diff/60001/components/ntp_tiles/BU...
> components/ntp_tiles/BUILD.gn:19: source_set("unit_tests") {
> On 2016/04/28 16:16:41, jochen wrote:
> > delete this then?
> 
> I'd done this for OWNERS. components_test.gyp lists all the tests, and has an
OWNERS exception for adding them. ../BUILD.gn references the list here, so I'd
have to get approval again to add tests later. I can come back to you for a
second approval next week if you'd prefer.

yes, please

[jochen (gone - plz use gerrit), 2016-04-28 16:34:23]

https://codereview.chromium.org/1928913002/diff/60001/components/ntp_tiles/OW...
File components/ntp_tiles/OWNERS (right):

https://codereview.chromium.org/1928913002/diff/60001/components/ntp_tiles/OW...
components/ntp_tiles/OWNERS:3: sfiera@chromium.org
only full committers can be owners btw :-/

[sfiera, 2016-04-29 08:18:49]

https://codereview.chromium.org/1928913002/diff/60001/components/ntp_tiles/BU...
File components/ntp_tiles/BUILD.gn (right):

https://codereview.chromium.org/1928913002/diff/60001/components/ntp_tiles/BU...
components/ntp_tiles/BUILD.gn:19: source_set("unit_tests") {
jochen wrote:
> On 2016/04/28 16:27:59, sfiera wrote:
> > On 2016/04/28 16:16:41, jochen wrote:
> > > delete this then?
> > 
> > I'd done this for OWNERS. components_test.gyp lists all the tests, and has
an
> > OWNERS exception for adding them. ../BUILD.gn references the list here, so
I'd
> > have to get approval again to add tests later. I can come back to you for a
> > second approval next week if you'd prefer.
>
> yes, please

OK, removed.

https://codereview.chromium.org/1928913002/diff/60001/components/ntp_tiles/OW...
File components/ntp_tiles/OWNERS (right):

https://codereview.chromium.org/1928913002/diff/60001/components/ntp_tiles/OW...
components/ntp_tiles/OWNERS:3: sfiera@chromium.org
On 2016/04/28 16:34:23, jochen wrote:
> only full committers can be owners btw :-/

Removed. Everyone submits through the CQ so I didn't realize there was a status
I didn't have :)

[jochen (gone - plz use gerrit), 2016-04-29 08:20:45]

lgtm

[sfiera, 2016-04-29 08:23:10]

The CQ bit was checked by sfiera@chromium.org

[sfiera, 2016-04-29 08:23:10]

The patchset sent to the CQ was uploaded after l-g-t-m from treib@chromium.org
Link to the patchset: https://codereview.chromium.org/1928913002/#ps80001
(title: " ")

[commit-bot: I haz the power, 2016-04-29 08:23:13]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1928913002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1928913002/80001

[sfiera, 2016-04-29 08:23:20]

The CQ bit was unchecked by sfiera@chromium.org

[sfiera, 2016-04-29 08:24:38]

The CQ bit was checked by sfiera@chromium.org

[sfiera, 2016-04-29 08:24:38]

The patchset sent to the CQ was uploaded after l-g-t-m from treib@chromium.org,
jochen@chromium.org
Link to the patchset: https://codereview.chromium.org/1928913002/#ps100001
(title: " ")

[commit-bot: I haz the power, 2016-04-29 08:24:44]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1928913002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1928913002/100001

[sfiera, 2016-04-29 08:24:48]

Thanks for the quick review! I'll get back to you when we have some tests to add
:)

[commit-bot: I haz the power, 2016-04-29 09:23:21]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-04-30 17:24:43]

Patchset 6 (id:??) landed as
https://crrev.com/0dc4da510e5dbbf2a6197c0eb03500d64568652a
Cr-Commit-Position: refs/heads/master@{#390605}

[commit-bot: I haz the power, 2016-04-30 17:59:55]

Description was changed from

==========
Create //components/ntp_tiles.

Move its prefs and switches in immediately. Later will follow
//chrome/browser/android/ntp/{most_visited_sites,popular_sites}*, as the
work to refactor out their dependencies is ongoing.

BUG=603026
==========

to

==========
Create //components/ntp_tiles.

Move its prefs and switches in immediately. Later will follow
//chrome/browser/android/ntp/{most_visited_sites,popular_sites}*, as the
work to refactor out their dependencies is ongoing.

BUG=603026

Committed: https://crrev.com/0dc4da510e5dbbf2a6197c0eb03500d64568652a
Cr-Commit-Position: refs/heads/master@{#390605}
==========