Move ManagedBookmarksTracker from policy/core/browser to bookmarks/managed

Move ManagedBookmarksTracker from policy/core/browser to (new) bookmarks/managed.

After https://codereview.chromium.org/769153007, it handles more than policy.

BUG=443606
Committed: https://crrev.com/a9ea2fbe3dff3bc3561a89182caadedad89f3ea0
Cr-Commit-Position: refs/heads/master@{#321539}

[Marc Treib, 2015-02-26 15:25:17]

treib@chromium.org changed reviewers:
+ bartfab@chromium.org, sky@chromium.org

[Marc Treib, 2015-02-26 15:25:17]

PTAL - it's really just a trivial move + corresponding namespace change. Thanks!

[tfarina, 2015-02-26 15:40:34]

tfarina@chromium.org changed reviewers:
+ tfarina@chromium.org

[tfarina, 2015-02-26 15:40:35]

Why can't it stay in policy?

[Marc Treib, 2015-02-26 15:44:42]

On 2015/02/26 15:40:35, tfarina wrote:
> Why can't it stay in policy?

Well, it can, technically. But it's also used for supervised users/child
accounts now, so policy is not really the right place anymore.
In the comments to https://codereview.chromium.org/769153007, bartfab suggested
moving it to bookmarks, which imo makes sense.

[bartfab (slow), 2015-02-26 15:51:23]

LGTM with one nit:

Please remove "components/bookmarks" from components/policy/core/browser/DEPS if
it is no longer used by anyone else.

[tfarina, 2015-02-26 16:12:29]

What supervised users has to be with the core of bookmarks system? It looks like
a layer above to me.

[Marc Treib, 2015-02-26 16:40:00]

On 2015/02/26 16:12:29, tfarina wrote:
> What supervised users has to be with the core of bookmarks system? It looks
like
> a layer above to me.

To me, it's functionality provided by the bookmarks system (namely, "enforced"
bookmarks that can't be edited by the user), which is used by both supervised
users and enterprise policy.
Where would you suggest to put it?

[Marc Treib, 2015-03-13 11:48:01]

Ping: sky, WDYT?

[sky, 2015-03-13 15:32:23]

On 2015/03/13 11:48:01, Marc Treib wrote:
> Ping: sky, WDYT?

I tend to favor keeping the core bookmarks, and really any API, at the bare
minimum. As managed bookmarks are optional, and can be layered on top of the
bookmark system, I prefer keeping managed bookmarks separate from the core.
Maybe I'm missing a compelling reason to move managed bookmarks?

[Marc Treib, 2015-03-13 15:51:46]

On 2015/03/13 15:32:23, sky wrote:
> On 2015/03/13 11:48:01, Marc Treib wrote:
> > Ping: sky, WDYT?
> 
> I tend to favor keeping the core bookmarks, and really any API, at the bare
> minimum. As managed bookmarks are optional, and can be layered on top of the
> bookmark system, I prefer keeping managed bookmarks separate from the core.
> Maybe I'm missing a compelling reason to move managed bookmarks?

The reason to move it out of policy is that it's not specific to policy anymore.
The reason to move it into bookmarks is simply that I don't know a better place
to put it. And arguably, a big part of it (the special "managed node" and
"supervised node") is already in bookmarks anyway (chrome/browser/bookmarks
though, not components/bookmarks).

[sky, 2015-03-13 15:55:11]

I'm ok with it in components/bookmarks, but I think it should be a
separate directory than browser. Perhaps cmponents/bookmarks/policy?

On Fri, Mar 13, 2015 at 8:51 AM,  <treib@chromium.org> wrote:
> On 2015/03/13 15:32:23, sky wrote:
>>
>> On 2015/03/13 11:48:01, Marc Treib wrote:
>> > Ping: sky, WDYT?
>
>
>> I tend to favor keeping the core bookmarks, and really any API, at the
>> bare
>> minimum. As managed bookmarks are optional, and can be layered on top of
>> the
>> bookmark system, I prefer keeping managed bookmarks separate from the
>> core.
>> Maybe I'm missing a compelling reason to move managed bookmarks?
>
>
> The reason to move it out of policy is that it's not specific to policy
> anymore.
> The reason to move it into bookmarks is simply that I don't know a better
> place
> to put it. And arguably, a big part of it (the special "managed node" and
> "supervised node") is already in bookmarks anyway (chrome/browser/bookmarks
> though, not components/bookmarks).
>
>
> https://codereview.chromium.org/961683002/

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Marc Treib, 2015-03-13 15:57:52]

"policy" isn't really accurate - components/bookmarks/managed? Or
chrome/browser/bookmarks/managed?

On 2015/03/13 15:55:11, sky wrote:
> I'm ok with it in components/bookmarks, but I think it should be a
> separate directory than browser. Perhaps cmponents/bookmarks/policy?
> 
> On Fri, Mar 13, 2015 at 8:51 AM,  <mailto:treib@chromium.org> wrote:
> > On 2015/03/13 15:32:23, sky wrote:
> >>
> >> On 2015/03/13 11:48:01, Marc Treib wrote:
> >> > Ping: sky, WDYT?
> >
> >
> >> I tend to favor keeping the core bookmarks, and really any API, at the
> >> bare
> >> minimum. As managed bookmarks are optional, and can be layered on top of
> >> the
> >> bookmark system, I prefer keeping managed bookmarks separate from the
> >> core.
> >> Maybe I'm missing a compelling reason to move managed bookmarks?
> >
> >
> > The reason to move it out of policy is that it's not specific to policy
> > anymore.
> > The reason to move it into bookmarks is simply that I don't know a better
> > place
> > to put it. And arguably, a big part of it (the special "managed node" and
> > "supervised node") is already in bookmarks anyway (chrome/browser/bookmarks
> > though, not components/bookmarks).
> >
> >
> > https://codereview.chromium.org/961683002/
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

[sky, 2015-03-13 16:00:38]

components/bookmarks/managed

On Fri, Mar 13, 2015 at 8:57 AM,  <treib@chromium.org> wrote:
> "policy" isn't really accurate - components/bookmarks/managed? Or
> chrome/browser/bookmarks/managed?
>
> On 2015/03/13 15:55:11, sky wrote:
>>
>> I'm ok with it in components/bookmarks, but I think it should be a
>> separate directory than browser. Perhaps cmponents/bookmarks/policy?
>
>
>> On Fri, Mar 13, 2015 at 8:51 AM,  <mailto:treib@chromium.org> wrote:
>> > On 2015/03/13 15:32:23, sky wrote:
>> >>
>> >> On 2015/03/13 11:48:01, Marc Treib wrote:
>> >> > Ping: sky, WDYT?
>> >
>> >
>> >> I tend to favor keeping the core bookmarks, and really any API, at the
>> >> bare
>> >> minimum. As managed bookmarks are optional, and can be layered on top
>> >> of
>> >> the
>> >> bookmark system, I prefer keeping managed bookmarks separate from the
>> >> core.
>> >> Maybe I'm missing a compelling reason to move managed bookmarks?
>> >
>> >
>> > The reason to move it out of policy is that it's not specific to policy
>> > anymore.
>> > The reason to move it into bookmarks is simply that I don't know a
>> > better
>> > place
>> > to put it. And arguably, a big part of it (the special "managed node"
>> > and
>> > "supervised node") is already in bookmarks anyway
>> > (chrome/browser/bookmarks
>> > though, not components/bookmarks).
>> >
>> >
>> > https://codereview.chromium.org/961683002/
>
>
>> To unsubscribe from this group and stop receiving emails from it, send an
>
> email
>>
>> to mailto:chromium-reviews+unsubscribe@chromium.org.
>
>
>
>
> https://codereview.chromium.org/961683002/

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[tfarina, 2015-03-13 16:26:05]

On Friday, March 13, 2015, Scott Violet <sky@chromium.org> wrote:

> components/bookmarks/managed

Can't it be in chrome/browser/bookmarks/managed? though I think a separate
directory like managed is good enough to keep the code separately from the
core. I also prefer slightly the term supervised.



-- 
Thiago Farina

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[noyau (Ping after 24h), 2015-03-13 16:38:47]

On 2015/03/13 16:26:05, tfarina wrote:
> On Friday, March 13, 2015, Scott Violet <mailto:sky@chromium.org> wrote:
> 
> > components/bookmarks/managed
> 
> Can't it be in chrome/browser/bookmarks/managed? though I think a separate
> directory like managed is good enough to keep the code separately from the
> core. I also prefer slightly the term supervised.
> 

It needs to migrate to components, so it is usable by ios/.

[sky, 2015-03-13 16:42:53]

components/bookmarks/managed works for ios, right?

On Fri, Mar 13, 2015 at 9:38 AM,  <noyau@chromium.org> wrote:
> On 2015/03/13 16:26:05, tfarina wrote:
>>
>> On Friday, March 13, 2015, Scott Violet <mailto:sky@chromium.org> wrote:
>
>
>> > components/bookmarks/managed
>
>
>> Can't it be in chrome/browser/bookmarks/managed? though I think a separate
>> directory like managed is good enough to keep the code separately from the
>> core. I also prefer slightly the term supervised.
>
>
>
> It needs to migrate to components, so it is usable by ios/.
>
> https://codereview.chromium.org/961683002/

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[tfarina, 2015-03-13 16:43:15]

On Friday, March 13, 2015, <noyau@chromium.org> wrote:

> On 2015/03/13 16:26:05, tfarina wrote:
>
>> On Friday, March 13, 2015, Scott Violet <mailto:sky@chromium.org> wrote:
>>
>
>  > components/bookmarks/managed
>>
>
>  Can't it be in chrome/browser/bookmarks/managed? though I think a
>> separate
>> directory like managed is good enough to keep the code separately from the
>> core. I also prefer slightly the term supervised.
>>
>
>
> It needs to migrate to components, so it is usable by ios/.

That is a compeling argument. SGTM!



-- 
Thiago Farina

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[noyau (Ping after 24h), 2015-03-13 17:02:54]

On 2015/03/13 16:42:53, sky wrote:
> components/bookmarks/managed works for ios, right?
> 
Yes, anything in a non-layered component like bookmarks is fair game for iOS.

> On Fri, Mar 13, 2015 at 9:38 AM,  <mailto:noyau@chromium.org> wrote:
> > On 2015/03/13 16:26:05, tfarina wrote:
> >>
> >> On Friday, March 13, 2015, Scott Violet <mailto:sky@chromium.org> wrote:
> >
> >
> >> > components/bookmarks/managed
> >
> >
> >> Can't it be in chrome/browser/bookmarks/managed? though I think a separate
> >> directory like managed is good enough to keep the code separately from the
> >> core. I also prefer slightly the term supervised.
> >
> >
> >
> > It needs to migrate to components, so it is usable by ios/.
> >
> > https://codereview.chromium.org/961683002/
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

[Marc Treib, 2015-03-17 11:56:52]

Moved to components/bookmarks/managed!

bartfab, fyi: I added components/bookmarks back to the policy DEPS, since it's
still needed in managed_bookmarks_policy_handler.cc. (I guess you only get
errors about missing DEPS when adding new includes?)

[bartfab (slow), 2015-03-17 13:31:36]

Still LGTM.

[sky, 2015-03-17 17:08:02]

LGTM

[Marc Treib, 2015-03-17 17:14:31]

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

[Marc Treib, 2015-03-17 17:14:32]

+brettw for components/BUILD.gn OWNERS approval. PTAL!

[brettw, 2015-03-19 23:28:42]

lgtm

[Marc Treib, 2015-03-20 08:31:27]

The CQ bit was checked by treib@chromium.org

[commit-bot: I haz the power, 2015-03-20 08:31:39]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/961683002/140001

[commit-bot: I haz the power, 2015-03-20 10:06:17]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2015-03-20 10:07:10]

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