Add hook for extension to check sync status.

Add hook for extension to check sync status.

Adds a new function to preferencesPrivate that returns a list of
unencrypted sync categories. Also changes preferencesPrivate to use a
whitelist instead of being restricted to component extensions.

BUG=326678
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=241977

[Rune Fevang, 2013-12-18 21:44:17]

kalman: extensions OWNER review
zea: Am I using the sync APIs correctly? In particular is using
syncer::ModelTypeToString reasonable or are there better alternatives?

[not at google - send to devlin, 2013-12-18 22:30:37]

some questions for zea but lgtm for extensions with a bunch of little things.

https://codereview.chromium.org/118413003/diff/1/chrome/browser/extensions/ap...
File
chrome/browser/extensions/api/preferences_private/preferences_private_api.cc
(right):

https://codereview.chromium.org/118413003/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/preferences_private/preferences_private_api.cc:25:
ProfileSyncService* sync_service =
@zea - can this ever be null (e.g. if the sync service has shut down or
something; these functions are refcounted obviously)

https://codereview.chromium.org/118413003/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/preferences_private/preferences_private_api.cc:31:
sync_service->AddObserver(this);
idiomatically all work is done in RunImpl and Run isn't overridden. I think this
would amount to just doing some simple renaming.

https://codereview.chromium.org/118413003/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/preferences_private/preferences_private_api.cc:60:
sync_service->GetEncryptedDataTypes());
is it possible for there to be things in EncryptedDataTypes that aren't in
PreferredDataTypes?

https://codereview.chromium.org/118413003/diff/1/chrome/browser/extensions/ap...
File chrome/browser/extensions/api/preferences_private/preferences_private_api.h
(right):

https://codereview.chromium.org/118413003/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/preferences_private/preferences_private_api.h:29:
virtual void OnStateChanged() OVERRIDE;
these can be private

https://codereview.chromium.org/118413003/diff/1/chrome/common/extensions/api...
File chrome/common/extensions/api/preferences_private.json (right):

https://codereview.chromium.org/118413003/diff/1/chrome/common/extensions/api...
chrome/common/extensions/api/preferences_private.json:10:
"googleGeolocationAccessEnabled": {
add "nocompile": true to this.

https://codereview.chromium.org/118413003/diff/1/chrome/test/data/extensions/...
File chrome/test/data/extensions/api_test/preferences_private/manifest.json
(right):

https://codereview.chromium.org/118413003/diff/1/chrome/test/data/extensions/...
chrome/test/data/extensions/api_test/preferences_private/manifest.json:3:
"name": "chrome.preferencesPrivate",
could you upload this CL with --no-detect-copies (or whatever the flag is)

https://codereview.chromium.org/118413003/diff/1/chrome/test/data/extensions/...
File chrome/test/data/extensions/api_test/preferences_private/test.js (right):

https://codereview.chromium.org/118413003/diff/1/chrome/test/data/extensions/...
chrome/test/data/extensions/api_test/preferences_private/test.js:7:
function(categories) {
use chrome.test.callbackPass(function(categories) { ... })

https://codereview.chromium.org/118413003/diff/1/chrome/test/data/extensions/...
chrome/test/data/extensions/api_test/preferences_private/test.js:19: }
Could you do like:

chrome.test.assertTrue(categories.indexOf("Bookmarks") >= 0, "bookmarks is
missing");
chrome.test.assertTrue(categories.indexOf("Preferences") >= 0, "preferences is
missing");
chrome.test.assertEq(2, categories.length, "unexpected categories");

https://codereview.chromium.org/118413003/diff/1/chrome/test/data/extensions/...
chrome/test/data/extensions/api_test/preferences_private/test.js:22:
chrome.test.succeed();
... and you won't need the succeed() at the end here.

[Rune Fevang, 2013-12-19 00:13:42]

https://codereview.chromium.org/118413003/diff/1/chrome/browser/extensions/ap...
File
chrome/browser/extensions/api/preferences_private/preferences_private_api.cc
(right):

https://codereview.chromium.org/118413003/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/preferences_private/preferences_private_api.cc:31:
sync_service->AddObserver(this);
On 2013/12/18 22:30:37, kalman wrote:
> idiomatically all work is done in RunImpl and Run isn't overridden. I think
this
> would amount to just doing some simple renaming.

Done.

https://codereview.chromium.org/118413003/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/preferences_private/preferences_private_api.cc:60:
sync_service->GetEncryptedDataTypes());
On 2013/12/18 22:30:37, kalman wrote:
> is it possible for there to be things in EncryptedDataTypes that aren't in
> PreferredDataTypes?

I'm not sure, but I don't really see how it matters.

https://codereview.chromium.org/118413003/diff/1/chrome/browser/extensions/ap...
File chrome/browser/extensions/api/preferences_private/preferences_private_api.h
(right):

https://codereview.chromium.org/118413003/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/preferences_private/preferences_private_api.h:29:
virtual void OnStateChanged() OVERRIDE;
On 2013/12/18 22:30:37, kalman wrote:
> these can be private

Done.

https://codereview.chromium.org/118413003/diff/1/chrome/common/extensions/api...
File chrome/common/extensions/api/preferences_private.json (right):

https://codereview.chromium.org/118413003/diff/1/chrome/common/extensions/api...
chrome/common/extensions/api/preferences_private.json:10:
"googleGeolocationAccessEnabled": {
On 2013/12/18 22:30:37, kalman wrote:
> add "nocompile": true to this.

Done.

https://codereview.chromium.org/118413003/diff/1/chrome/test/data/extensions/...
File chrome/test/data/extensions/api_test/preferences_private/manifest.json
(right):

https://codereview.chromium.org/118413003/diff/1/chrome/test/data/extensions/...
chrome/test/data/extensions/api_test/preferences_private/manifest.json:3:
"name": "chrome.preferencesPrivate",
On 2013/12/18 22:30:37, kalman wrote:
> could you upload this CL with --no-detect-copies (or whatever the flag is)

Done.

https://codereview.chromium.org/118413003/diff/1/chrome/test/data/extensions/...
File chrome/test/data/extensions/api_test/preferences_private/test.js (right):

https://codereview.chromium.org/118413003/diff/1/chrome/test/data/extensions/...
chrome/test/data/extensions/api_test/preferences_private/test.js:7:
function(categories) {
On 2013/12/18 22:30:37, kalman wrote:
> use chrome.test.callbackPass(function(categories) { ... })

Done.

https://codereview.chromium.org/118413003/diff/1/chrome/test/data/extensions/...
chrome/test/data/extensions/api_test/preferences_private/test.js:19: }
On 2013/12/18 22:30:37, kalman wrote:
> Could you do like:
> 
> chrome.test.assertTrue(categories.indexOf("Bookmarks") >= 0, "bookmarks is
> missing");
> chrome.test.assertTrue(categories.indexOf("Preferences") >= 0, "preferences is
> missing");
> chrome.test.assertEq(2, categories.length, "unexpected categories");

Done, except I don't want to make any assumptions on how many categories there
are here.

https://codereview.chromium.org/118413003/diff/1/chrome/test/data/extensions/...
chrome/test/data/extensions/api_test/preferences_private/test.js:22:
chrome.test.succeed();
On 2013/12/18 22:30:37, kalman wrote:
> ... and you won't need the succeed() at the end here.

Done.

[Nicolas Zea, 2013-12-19 00:23:59]

https://codereview.chromium.org/118413003/diff/1/chrome/browser/extensions/ap...
File
chrome/browser/extensions/api/preferences_private/preferences_private_api.cc
(right):

https://codereview.chromium.org/118413003/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/preferences_private/preferences_private_api.cc:25:
ProfileSyncService* sync_service =
On 2013/12/18 22:30:37, kalman wrote:
> @zea - can this ever be null (e.g. if the sync service has shut down or
> something; these functions are refcounted obviously)

If sync is disabled by command line flag this can be null. I'd recommend
checking before dereferencing.

https://codereview.chromium.org/118413003/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/preferences_private/preferences_private_api.cc:60:
sync_service->GetEncryptedDataTypes());
On 2013/12/19 00:13:43, Rune Fevang wrote:
> On 2013/12/18 22:30:37, kalman wrote:
> > is it possible for there to be things in EncryptedDataTypes that aren't in
> > PreferredDataTypes?
> 
> I'm not sure, but I don't really see how it matters.

It is possible, but Difference does an A & ~B operation, not an XOR, so this
should be fine.

https://codereview.chromium.org/118413003/diff/20001/chrome/browser/extension...
File
chrome/browser/extensions/api/preferences_private/preferences_private_api.cc
(right):

https://codereview.chromium.org/118413003/diff/20001/chrome/browser/extension...
chrome/browser/extensions/api/preferences_private/preferences_private_api.cc:50:
sync_service->GetPreferredDataTypes());
Do you want GetPreferred, or GetActive? Active is a subset of preferred, but
only includes those that are actually running (have fully started up and haven't
encountered an error). Depends on the use case.

[Rune Fevang, 2013-12-19 00:35:48]

https://codereview.chromium.org/118413003/diff/1/chrome/browser/extensions/ap...
File
chrome/browser/extensions/api/preferences_private/preferences_private_api.cc
(right):

https://codereview.chromium.org/118413003/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/preferences_private/preferences_private_api.cc:25:
ProfileSyncService* sync_service =
On 2013/12/19 00:23:59, Nicolas Zea wrote:
> On 2013/12/18 22:30:37, kalman wrote:
> > @zea - can this ever be null (e.g. if the sync service has shut down or
> > something; these functions are refcounted obviously)
> 
> If sync is disabled by command line flag this can be null. I'd recommend
> checking before dereferencing.

Done.

https://codereview.chromium.org/118413003/diff/20001/chrome/browser/extension...
File
chrome/browser/extensions/api/preferences_private/preferences_private_api.cc
(right):

https://codereview.chromium.org/118413003/diff/20001/chrome/browser/extension...
chrome/browser/extensions/api/preferences_private/preferences_private_api.cc:50:
sync_service->GetPreferredDataTypes());
On 2013/12/19 00:23:59, Nicolas Zea wrote:
> Do you want GetPreferred, or GetActive? Active is a subset of preferred, but
> only includes those that are actually running (have fully started up and
haven't
> encountered an error). Depends on the use case.

The use case is "which types do the user want to sync" and not "which types are
currently synced", so I think GetPreferred is correct.

[Nicolas Zea, 2013-12-19 01:07:40]

lgtm

[commit-bot: I haz the power, 2013-12-19 20:07:26]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rfevang@chromium.org/118413003/40001

[commit-bot: I haz the power, 2013-12-19 23:33:27]

Change committed as 241977

[Jeffrey Yasskin, 2013-12-19 23:47:23]

https://codereview.chromium.org/118413003/diff/40001/chrome/common/extensions...
File chrome/common/extensions/api/preferences_private.json (right):

https://codereview.chromium.org/118413003/diff/40001/chrome/common/extensions...
chrome/common/extensions/api/preferences_private.json:20: "description":
"Returns a list of sync categories the user has enabled without using a custom
passphrase for encryption",
Please document the list of possible category strings here, or say where in the
source the list is defined. These files are where we need to be able to look to
learn the contract for any new API.