Add a datatype for supervised user whitelists.

Add a datatype for supervised user whitelists.

BUG=436459
Committed: https://crrev.com/8a4355ef1a592e6eba484c354e6805fe8e097494
Cr-Commit-Position: refs/heads/master@{#308569}

[Bernhard Bauer, 2014-12-12 17:05:51]

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

[Bernhard Bauer, 2014-12-12 17:05:52]

Please review. Thanks!

[Nicolas Zea, 2014-12-15 21:47:53]

Mostly LG

https://codereview.chromium.org/754963004/diff/60001/sync/protocol/managed_us...
File sync/protocol/managed_user_whitelist_specifics.proto (right):

https://codereview.chromium.org/754963004/diff/60001/sync/protocol/managed_us...
sync/protocol/managed_user_whitelist_specifics.proto:22: optional
ExtensionSpecifics extension = 1;
Do you really need the full extension specifics? Or is the CRX URL enough?

[Bernhard Bauer, 2014-12-15 22:36:42]

https://codereview.chromium.org/754963004/diff/60001/sync/protocol/managed_us...
File sync/protocol/managed_user_whitelist_specifics.proto (right):

https://codereview.chromium.org/754963004/diff/60001/sync/protocol/managed_us...
sync/protocol/managed_user_whitelist_specifics.proto:22: optional
ExtensionSpecifics extension = 1;
On 2014/12/15 21:47:53, Nicolas Zea wrote:
> Do you really need the full extension specifics? Or is the CRX URL enough?

Hmm... I really only need the extension ID and the name. Originally I was
thinking to use a full ExtensionSpecifics so I could pass it straight to the
ExtensionSyncService on desktop, but we've since basically decided to use the
Component Updater for whitelists on desktop as well, so it wouldn't be
necessary. It would make switching back to extensions more difficult, but I
think I'm okay with that. Soo... done! :)

[Nicolas Zea, 2014-12-15 22:45:36]

LGTM with some nits

https://codereview.chromium.org/754963004/diff/80001/sync/internal_api/public...
File sync/internal_api/public/base/model_type.h (right):

https://codereview.chromium.org/754963004/diff/80001/sync/internal_api/public...
sync/internal_api/public/base/model_type.h:118: // an extension. The data type
is separate to allow syncing only whitelists.
nit: update comment.

https://codereview.chromium.org/754963004/diff/80001/sync/protocol/managed_us...
File sync/protocol/managed_user_whitelist_specifics.proto (right):

https://codereview.chromium.org/754963004/diff/80001/sync/protocol/managed_us...
sync/protocol/managed_user_whitelist_specifics.proto:21: optional string id = 1;
perhaps mention that this is a webstore id?

https://codereview.chromium.org/754963004/diff/80001/sync/protocol/managed_us...
sync/protocol/managed_user_whitelist_specifics.proto:23: // The name of the
whitelist.
One word of caution, not sure if it necessarily applies here. Extensions has the
problem that it syncs the extension name and version, which are automatically
updated based on what the extension author does. As such, if a user deletes an
extension on one machine, but another machine upgrades to a new version of the
extension, the conflict resolution logic will undelete the extension (since the
update will have a new version, and entity modification > entity deletion).

If that's not an issue here (because for example they entities are read only on
the client, or the name can never change), then feel free to ignore.

[Bernhard Bauer, 2014-12-15 22:56:13]

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

[Bernhard Bauer, 2014-12-15 22:56:14]

Ilya, can I get an OWNERS review for histograms.xml?

https://codereview.chromium.org/754963004/diff/80001/sync/internal_api/public...
File sync/internal_api/public/base/model_type.h (right):

https://codereview.chromium.org/754963004/diff/80001/sync/internal_api/public...
sync/internal_api/public/base/model_type.h:118: // an extension. The data type
is separate to allow syncing only whitelists.
On 2014/12/15 22:45:35, Nicolas Zea wrote:
> nit: update comment.

Done.

https://codereview.chromium.org/754963004/diff/80001/sync/protocol/managed_us...
File sync/protocol/managed_user_whitelist_specifics.proto (right):

https://codereview.chromium.org/754963004/diff/80001/sync/protocol/managed_us...
sync/protocol/managed_user_whitelist_specifics.proto:21: optional string id = 1;
On 2014/12/15 22:45:36, Nicolas Zea wrote:
> perhaps mention that this is a webstore id?

Done.

https://codereview.chromium.org/754963004/diff/80001/sync/protocol/managed_us...
sync/protocol/managed_user_whitelist_specifics.proto:23: // The name of the
whitelist.
On 2014/12/15 22:45:36, Nicolas Zea wrote:
> One word of caution, not sure if it necessarily applies here. Extensions has
the
> problem that it syncs the extension name and version, which are automatically
> updated based on what the extension author does. As such, if a user deletes an
> extension on one machine, but another machine upgrades to a new version of the
> extension, the conflict resolution logic will undelete the extension (since
the
> update will have a new version, and entity modification > entity deletion).
> 
> If that's not an issue here (because for example they entities are read only
on
> the client, or the name can never change), then feel free to ignore.

Yeah, we neatly sidestep all of these problems by never allowing any local
changes :)

[Ilya Sherman, 2014-12-15 23:45:02]

histograms.xml lgtm

[Bernhard Bauer, 2014-12-16 09:22:02]

The CQ bit was checked by bauerb@chromium.org

[commit-bot: I haz the power, 2014-12-16 09:22:12]

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

[commit-bot: I haz the power, 2014-12-16 11:23:54]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2014-12-16 11:24:39]

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