Add IDL and stub implementation for chrome.usersPrivate API.

Add IDL and stub implementation for chrome.usersPrivate API.

Proposal is here: https://docs.google.com/document/d/1ucnVkZSkOyY0R7kyryyLlvNq7rkN8EyAtvE6WsCxcFY/edit#heading=h.w8inspeo32bj

BUG=491369
Committed: https://crrev.com/5fbbf2ae2eba1359ae4f0d1b0737197468b27385
Cr-Commit-Position: refs/heads/master@{#332261}

[Dan Beam, 2015-05-23 00:51:13]

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

[Dan Beam, 2015-05-23 00:51:14]

https://codereview.chromium.org/1143303004/diff/1/chrome/common/extensions/ap...
File chrome/common/extensions/api/users_private.idl (right):

https://codereview.chromium.org/1143303004/diff/1/chrome/common/extensions/ap...
chrome/common/extensions/api/users_private.idl:11: static void
getWhitelistedUsers(DOMString[] emails);
why not make this onWhiteListedUsersChanged and fire it immediately when
subscribing?  right now this API has no way to know when a user is
added/removed...

[Dan Beam, 2015-05-23 00:51:20]

dbeam@chromium.org changed reviewers:
- dbeam@chromium.org

[Oren Blasberg, 2015-05-26 17:04:07]

https://codereview.chromium.org/1143303004/diff/1/chrome/common/extensions/ap...
File chrome/common/extensions/api/users_private.idl (right):

https://codereview.chromium.org/1143303004/diff/1/chrome/common/extensions/ap...
chrome/common/extensions/api/users_private.idl:11: static void
getWhitelistedUsers(DOMString[] emails);
On 2015/05/23 00:51:14, Dan Beam wrote:
> why not make this onWhiteListedUsersChanged and fire it immediately when
> subscribing?  right now this API has no way to know when a user is
> added/removed...

Implementing that looks pretty challenging if we want to leave the
OwnersSettingsServiceChromeOS alone (which would be preferred).. I see that the
OSSCOS's observe method doesn't provide observers with a way to observe just a
specific path (i.e. the list of whitelisted users), just a generic observe
method [1].

Instead, I suppose we could emulate in C++ what the existing JS implementation
of the user list does, which is to listen on the generic preferences object for
the 'cros.accounts.users' path [2]. In C++ this would mean observing the correct
PrefService for changes, and when the changes affect that path, pass along the
new data to the JS. 

This isn't as simple as it sounds though because the logic for determining which
is the correct PrefService (see "FindServiceForPref" method) is originally in
the CoreOptionsHandler [3] and has since been migrated to the settingsPrivate
API implementation [4]. In the usersPrivate implementation, I could hard code
knowledge about which pref service to use (if indeed it's the same every time)
but I am wary of doing this since if the answer to which pref service to use
ends up changing, we'd have to make the change in two places and not just the
one (settingsPrivate) place.

So.. in the Users page JS, maybe we could just rely on
"chrome.settingsPrivate.onPrefsChanged," filter on 'cros.accounts.users' pref
and in response to that, call the getWhitelistedUsers method to re-populate the
user list. It isn't ideal but maybe a worthwhile tradeoff to make.

I welcome your thoughts on it.

[1]
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/chr...
[2]
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/res...
[3]
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/...
[4]
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ext...

[Oren Blasberg, 2015-05-26 18:23:10]

orenb@chromium.org changed reviewers:
+ kalman@chromium.org, mpearson@chromium.org, tbreisacher@chromium.org

[Oren Blasberg, 2015-05-26 18:23:38]

orenb@chromium.org changed reviewers:
+ jlklein@chromium.org

[Oren Blasberg, 2015-05-26 18:24:35]

I've added the stub implementation (methods work from js but do nothing).

[Mark P, 2015-05-26 18:33:13]

histograms.xml lgtm

[Dan Beam, 2015-05-28 20:09:30]

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

[Dan Beam, 2015-05-28 20:09:30]

https://codereview.chromium.org/1143303004/diff/1/chrome/common/extensions/ap...
File chrome/common/extensions/api/users_private.idl (right):

https://codereview.chromium.org/1143303004/diff/1/chrome/common/extensions/ap...
chrome/common/extensions/api/users_private.idl:11: static void
getWhitelistedUsers(DOMString[] emails);
On 2015/05/26 17:04:07, Oren Blasberg wrote:
> On 2015/05/23 00:51:14, Dan Beam wrote:
> > why not make this onWhiteListedUsersChanged and fire it immediately when
> > subscribing?  right now this API has no way to know when a user is
> > added/removed...
> 
> Implementing that looks pretty challenging if we want to leave the
> OwnersSettingsServiceChromeOS alone (which would be preferred).. I see that
the
> OSSCOS's observe method doesn't provide observers with a way to observe just a
> specific path (i.e. the list of whitelisted users), just a generic observe
> method [1].
> 
> Instead, I suppose we could emulate in C++ what the existing JS implementation
> of the user list does, which is to listen on the generic preferences object
for
> the 'cros.accounts.users' path [2]. In C++ this would mean observing the
correct
> PrefService for changes, and when the changes affect that path, pass along the
> new data to the JS. 
> 
> This isn't as simple as it sounds though because the logic for determining
which
> is the correct PrefService (see "FindServiceForPref" method) is originally in
> the CoreOptionsHandler [3] and has since been migrated to the settingsPrivate
> API implementation [4]. In the usersPrivate implementation, I could hard code
> knowledge about which pref service to use (if indeed it's the same every time)
> but I am wary of doing this since if the answer to which pref service to use
> ends up changing, we'd have to make the change in two places and not just the
> one (settingsPrivate) place.
> 
> So.. in the Users page JS, maybe we could just rely on
> "chrome.settingsPrivate.onPrefsChanged," filter on 'cros.accounts.users' pref
> and in response to that, call the getWhitelistedUsers method to re-populate
the
> user list. It isn't ideal but maybe a worthwhile tradeoff to make.
> 
> I welcome your thoughts on it.

Yeah, this seems fine as long as you're cool with the page knowing this API's
underlying storage is a pref with that name.

> 
> [1]
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/chr...
> [2]
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/res...
> [3]
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/...
> [4]
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ext...

[Ken Rockot(use gerrit already), 2015-05-29 14:52:34]

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

[Ken Rockot(use gerrit already), 2015-05-29 14:52:35]

idl lgtm

[Oren Blasberg, 2015-05-29 23:48:07]

https://codereview.chromium.org/1143303004/diff/1/chrome/common/extensions/ap...
File chrome/common/extensions/api/users_private.idl (right):

https://codereview.chromium.org/1143303004/diff/1/chrome/common/extensions/ap...
chrome/common/extensions/api/users_private.idl:11: static void
getWhitelistedUsers(DOMString[] emails);
On 2015/05/28 20:09:30, Dan Beam wrote:
> On 2015/05/26 17:04:07, Oren Blasberg wrote:
> > On 2015/05/23 00:51:14, Dan Beam wrote:
> > > why not make this onWhiteListedUsersChanged and fire it immediately when
> > > subscribing?  right now this API has no way to know when a user is
> > > added/removed...
> > 
> > Implementing that looks pretty challenging if we want to leave the
> > OwnersSettingsServiceChromeOS alone (which would be preferred).. I see that
> the
> > OSSCOS's observe method doesn't provide observers with a way to observe just
a
> > specific path (i.e. the list of whitelisted users), just a generic observe
> > method [1].
> > 
> > Instead, I suppose we could emulate in C++ what the existing JS
implementation
> > of the user list does, which is to listen on the generic preferences object
> for
> > the 'cros.accounts.users' path [2]. In C++ this would mean observing the
> correct
> > PrefService for changes, and when the changes affect that path, pass along
the
> > new data to the JS. 
> > 
> > This isn't as simple as it sounds though because the logic for determining
> which
> > is the correct PrefService (see "FindServiceForPref" method) is originally
in
> > the CoreOptionsHandler [3] and has since been migrated to the
settingsPrivate
> > API implementation [4]. In the usersPrivate implementation, I could hard
code
> > knowledge about which pref service to use (if indeed it's the same every
time)
> > but I am wary of doing this since if the answer to which pref service to use
> > ends up changing, we'd have to make the change in two places and not just
the
> > one (settingsPrivate) place.
> > 
> > So.. in the Users page JS, maybe we could just rely on
> > "chrome.settingsPrivate.onPrefsChanged," filter on 'cros.accounts.users'
pref
> > and in response to that, call the getWhitelistedUsers method to re-populate
> the
> > user list. It isn't ideal but maybe a worthwhile tradeoff to make.
> > 
> > I welcome your thoughts on it.
> 
> Yeah, this seems fine as long as you're cool with the page knowing this API's
> underlying storage is a pref with that name.
> 
> > 
> > [1]
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/chr...
> > [2]
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/res...
> > [3]
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/...
> > [4]
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ext...

Yep, I'm cool with that. In fact I'm currently adding support for observing
changes to CROS-specific settings in settingsPrivate right now so that will be a
logical mechanism for observing those changes.

Would you mind stamping the CL since you're an owner of the externs file?

[Dan Beam, 2015-05-29 23:51:22]

https://codereview.chromium.org/1143303004/diff/100001/third_party/closure_co...
File third_party/closure_compiler/externs/users_private.js (right):

https://codereview.chromium.org/1143303004/diff/100001/third_party/closure_co...
third_party/closure_compiler/externs/users_private.js:1:
users_private_externs.js
why is this here?

[Jeremy Klein, 2015-05-30 00:06:36]

lgtm besides dbeam's question

[Oren Blasberg, 2015-05-30 00:13:53]

https://codereview.chromium.org/1143303004/diff/100001/third_party/closure_co...
File third_party/closure_compiler/externs/users_private.js (right):

https://codereview.chromium.org/1143303004/diff/100001/third_party/closure_co...
third_party/closure_compiler/externs/users_private.js:1:
users_private_externs.js
On 2015/05/29 23:51:22, Dan Beam wrote:
> why is this here?
Ah, long story short it's an accident; I was ">"'ing the output of the generator
script to a file, but the output starts with the name of the file first for some
reason.

Thanks for catching it. Removed.

[Dan Beam, 2015-05-30 00:15:13]

lgtm

[Oren Blasberg, 2015-05-30 00:21:28]

The CQ bit was checked by orenb@chromium.org

[Oren Blasberg, 2015-05-30 00:21:28]

The patchset sent to the CQ was uploaded after l-g-t-m from
mpearson@chromium.org, rockot@chromium.org, jlklein@chromium.org,
dbeam@chromium.org
Link to the patchset: https://codereview.chromium.org/1143303004/#ps140001
(title: "Sync/merge changes")

[Oren Blasberg, 2015-05-30 00:22:04]

orenb@chromium.org changed reviewers:
- tbreisacher@chromium.org

[Oren Blasberg, 2015-05-30 00:22:04]

orenb@chromium.org changed reviewers:
- tbreisacher@chromium.org

[commit-bot: I haz the power, 2015-05-30 00:22:11]

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

[commit-bot: I haz the power, 2015-05-30 03:08:24]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-05-30 03:08:25]

Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Oren Blasberg, 2015-06-01 16:33:32]

The CQ bit was checked by orenb@chromium.org

[commit-bot: I haz the power, 2015-06-01 16:34:19]

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

[Oren Blasberg, 2015-06-01 17:11:24]

The CQ bit was unchecked by orenb@chromium.org

[Ken Rockot(use gerrit already), 2015-06-01 19:14:46]

lgtm

https://codereview.chromium.org/1143303004/diff/160001/chrome/common/extensio...
File chrome/common/extensions/api/users_private.idl (right):

https://codereview.chromium.org/1143303004/diff/160001/chrome/common/extensio...
chrome/common/extensions/api/users_private.idl:14: boolean? isOwner;
nit: IMHO it's kind of weird to have a field that is either true or undefined.
Consider making this a required field.

[Oren Blasberg, 2015-06-01 19:17:32]

https://codereview.chromium.org/1143303004/diff/160001/chrome/common/extensio...
File chrome/common/extensions/api/users_private.idl (right):

https://codereview.chromium.org/1143303004/diff/160001/chrome/common/extensio...
chrome/common/extensions/api/users_private.idl:14: boolean? isOwner;
On 2015/06/01 19:14:45, Ken Rockot wrote:
> nit: IMHO it's kind of weird to have a field that is either true or undefined.
> Consider making this a required field.

Done.

[Oren Blasberg, 2015-06-01 19:17:37]

The CQ bit was checked by orenb@chromium.org

[Oren Blasberg, 2015-06-01 19:17:38]

The patchset sent to the CQ was uploaded after l-g-t-m from
jlklein@chromium.org, mpearson@chromium.org, dbeam@chromium.org,
rockot@chromium.org
Link to the patchset: https://codereview.chromium.org/1143303004/#ps180001
(title: "Make isOwner a required field")

[commit-bot: I haz the power, 2015-06-01 19:21:25]

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

[commit-bot: I haz the power, 2015-06-01 20:40:27]

Committed patchset #10 (id:180001)

[commit-bot: I haz the power, 2015-06-01 20:41:24]

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