Add the IDL for chrome.instanceID API.

Add the IDL for chrome.instanceID API.

It is not implemented yet. So all InstanceID***Functions are just placeholders.

BUG=477084
TEST=none

Committed: https://crrev.com/d01af7ce4d760cc647964d835505bbe2df7be8b8
Cr-Commit-Position: refs/heads/master@{#326909}

[jianli, 2015-04-22 00:13:49]

jianli@chromium.org changed reviewers:
+ fgorski@chromium.org, kalman@chromium.org

[not at google - send to devlin, 2015-04-22 17:21:42]

https://codereview.chromium.org/1088593004/diff/20001/chrome/browser/extensio...
File chrome/browser/extensions/api/instance_id/instance_id_api.h (right):

https://codereview.chromium.org/1088593004/diff/20001/chrome/browser/extensio...
chrome/browser/extensions/api/instance_id/instance_id_api.h:14: class
InstanceIDGetIDFunction : public AsyncExtensionFunction {
Extend UIThreadExtensionFunction, not AsyncExtensionFunction (for all
functions).

https://codereview.chromium.org/1088593004/diff/20001/chrome/common/extension...
File chrome/common/extensions/api/instance_id.idl (right):

https://codereview.chromium.org/1088593004/diff/20001/chrome/common/extension...
chrome/common/extensions/api/instance_id.idl:6: [implemented_in =
"chrome/browser/extensions/api/instance_id/instance_id_api.h"]
the implemented_in is unnecessary; this should be the default.

[fgorski, 2015-04-22 17:34:59]

Mostly looks good.

https://codereview.chromium.org/1088593004/diff/20001/chrome/browser/extensio...
File chrome/browser/extensions/api/instance_id/instance_id_api.h (right):

https://codereview.chromium.org/1088593004/diff/20001/chrome/browser/extensio...
chrome/browser/extensions/api/instance_id/instance_id_api.h:16:
DECLARE_EXTENSION_FUNCTION("InstanceID.getId", INSTANCEID_GETID);
getID

https://codereview.chromium.org/1088593004/diff/20001/chrome/browser/extensio...
chrome/browser/extensions/api/instance_id/instance_id_api.h:69:
DECLARE_EXTENSION_FUNCTION("InstanceID.deleteInstanceID",
deleteID

https://codereview.chromium.org/1088593004/diff/20001/chrome/common/extension...
File chrome/common/extensions/api/_api_features.json (right):

https://codereview.chromium.org/1088593004/diff/20001/chrome/common/extension...
chrome/common/extensions/api/_api_features.json:499: "dependencies":
["permission:gcm"],
Is this choice conscious?
When we start using Instance ID beyond GCM and someone does not use GCM, will
you still require GCM permission?

Given that to take advantage of the API, developers will make changes in the
code, there is a chance to add permission at that time.

https://codereview.chromium.org/1088593004/diff/20001/chrome/common/extension...
File chrome/common/extensions/api/instance_id.idl (right):

https://codereview.chromium.org/1088593004/diff/20001/chrome/common/extension...
chrome/common/extensions/api/instance_id.idl:20: object? options;
Please ask Costin if this should be a string to string map

https://codereview.chromium.org/1088593004/diff/20001/extensions/browser/exte...
File extensions/browser/extension_function_histogram_value.h (right):

https://codereview.chromium.org/1088593004/diff/20001/extensions/browser/exte...
extensions/browser/extension_function_histogram_value.h:1076:
INSTANCEID_DELETEINSTANCEID,
INSTANCEID_DELETEID

[jianli, 2015-04-22 20:18:21]

https://codereview.chromium.org/1088593004/diff/20001/chrome/browser/extensio...
File chrome/browser/extensions/api/instance_id/instance_id_api.h (right):

https://codereview.chromium.org/1088593004/diff/20001/chrome/browser/extensio...
chrome/browser/extensions/api/instance_id/instance_id_api.h:14: class
InstanceIDGetIDFunction : public AsyncExtensionFunction {
On 2015/04/22 17:21:42, kalman wrote:
> Extend UIThreadExtensionFunction, not AsyncExtensionFunction (for all
> functions).

Done.

https://codereview.chromium.org/1088593004/diff/20001/chrome/browser/extensio...
chrome/browser/extensions/api/instance_id/instance_id_api.h:16:
DECLARE_EXTENSION_FUNCTION("InstanceID.getId", INSTANCEID_GETID);
On 2015/04/22 17:34:59, fgorski wrote:
> getID

Done.

https://codereview.chromium.org/1088593004/diff/20001/chrome/browser/extensio...
chrome/browser/extensions/api/instance_id/instance_id_api.h:69:
DECLARE_EXTENSION_FUNCTION("InstanceID.deleteInstanceID",
On 2015/04/22 17:34:59, fgorski wrote:
> deleteID

Done.

https://codereview.chromium.org/1088593004/diff/20001/chrome/common/extension...
File chrome/common/extensions/api/_api_features.json (right):

https://codereview.chromium.org/1088593004/diff/20001/chrome/common/extension...
chrome/common/extensions/api/_api_features.json:499: "dependencies":
["permission:gcm"],
On 2015/04/22 17:34:59, fgorski wrote:
> Is this choice conscious?
> When we start using Instance ID beyond GCM and someone does not use GCM, will
> you still require GCM permission?
> 
> Given that to take advantage of the API, developers will make changes in the
> code, there is a chance to add permission at that time.

For now I think it would be better to put it under GCM permission since we need
to get the device ID in order to talk with the server. We can update the
permission upon more discussion.

https://codereview.chromium.org/1088593004/diff/20001/chrome/common/extension...
File chrome/common/extensions/api/instance_id.idl (right):

https://codereview.chromium.org/1088593004/diff/20001/chrome/common/extension...
chrome/common/extensions/api/instance_id.idl:6: [implemented_in =
"chrome/browser/extensions/api/instance_id/instance_id_api.h"]
On 2015/04/22 17:21:42, kalman wrote:
> the implemented_in is unnecessary; this should be the default.

Done.

https://codereview.chromium.org/1088593004/diff/20001/chrome/common/extension...
chrome/common/extensions/api/instance_id.idl:20: object? options;
On 2015/04/22 17:34:59, fgorski wrote:
> Please ask Costin if this should be a string to string map

Yes, this was what I am told.

https://codereview.chromium.org/1088593004/diff/20001/extensions/browser/exte...
File extensions/browser/extension_function_histogram_value.h (right):

https://codereview.chromium.org/1088593004/diff/20001/extensions/browser/exte...
extensions/browser/extension_function_histogram_value.h:1076:
INSTANCEID_DELETEINSTANCEID,
On 2015/04/22 17:34:59, fgorski wrote:
> INSTANCEID_DELETEID

Done.

[not at google - send to devlin, 2015-04-22 20:47:42]

lgtm

https://codereview.chromium.org/1088593004/diff/20001/chrome/common/extension...
File chrome/common/extensions/api/_api_features.json (right):

https://codereview.chromium.org/1088593004/diff/20001/chrome/common/extension...
chrome/common/extensions/api/_api_features.json:499: "dependencies":
["permission:gcm"],
On 2015/04/22 20:18:20, jianli wrote:
> On 2015/04/22 17:34:59, fgorski wrote:
> > Is this choice conscious?
> > When we start using Instance ID beyond GCM and someone does not use GCM,
will
> > you still require GCM permission?
> > 
> > Given that to take advantage of the API, developers will make changes in the
> > code, there is a chance to add permission at that time.
> 
> For now I think it would be better to put it under GCM permission since we
need
> to get the device ID in order to talk with the server. We can update the
> permission upon more discussion.

If you add other APIs that depend on instanceID you can add multiple rules which
grant access to instanceID based on those, too.

[fgorski, 2015-04-22 20:55:48]

LGTM with one comment

https://codereview.chromium.org/1088593004/diff/20001/chrome/common/extension...
File chrome/common/extensions/api/instance_id.idl (right):

https://codereview.chromium.org/1088593004/diff/20001/chrome/common/extension...
chrome/common/extensions/api/instance_id.idl:20: object? options;
On 2015/04/22 20:18:21, jianli wrote:
> On 2015/04/22 17:34:59, fgorski wrote:
> > Please ask Costin if this should be a string to string map
> 
> Yes, this was what I am told.

OK, what I was looking for is perhaps a way to specify that in IDL. (I know it
is doable in JSON). If not possible we'll have to enforce it in the code.

[jianli, 2015-04-22 21:26:52]

The CQ bit was checked by jianli@chromium.org

[commit-bot: I haz the power, 2015-04-22 21:28:16]

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

[jianli, 2015-04-22 21:30:55]

The CQ bit was unchecked by jianli@chromium.org

[not at google - send to devlin, 2015-04-22 22:01:54]

I forgot to mention - add an OWNERS file for this API with yourself and fgorski,
so you don't need my lgtm for changes to the C++.

[jianli, 2015-04-22 22:55:23]

The CQ bit was checked by jianli@chromium.org

[jianli, 2015-04-22 22:55:24]

The patchset sent to the CQ was uploaded after l-g-t-m from
fgorski@chromium.org, kalman@chromium.org
Link to the patchset: https://codereview.chromium.org/1088593004/#ps60001
(title: "Add OWNERS and fix name in DECLARE_EXTENSION_FUNCTION macro")

[commit-bot: I haz the power, 2015-04-22 22:56:57]

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

[commit-bot: I haz the power, 2015-04-23 02:26:24]

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

[commit-bot: I haz the power, 2015-04-23 02:26:25]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[jianli, 2015-04-23 20:50:56]

jianli@chromium.org changed reviewers:
+ asvitkine@chromium.org

[jianli, 2015-04-23 20:50:57]

asvitkine, could you please approve the change to
extension_function_histogram_value.h? Thanks.

[Alexei Svitkine (slow), 2015-04-23 21:04:05]

please update histograms.xml too

[jianli, 2015-04-23 22:18:05]

On 2015/04/23 21:04:05, Alexei Svitkine wrote:
> please update histograms.xml too

Done. Thanks.

[Alexei Svitkine (slow), 2015-04-24 16:43:58]

lgtm

[jianli, 2015-04-24 19:53:29]

The CQ bit was checked by jianli@chromium.org

[jianli, 2015-04-24 19:53:30]

The patchset sent to the CQ was uploaded after l-g-t-m from
fgorski@chromium.org, kalman@chromium.org, asvitkine@chromium.org
Link to the patchset: https://codereview.chromium.org/1088593004/#ps100001
(title: "Sync")

[commit-bot: I haz the power, 2015-04-24 19:53:40]

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

[commit-bot: I haz the power, 2015-04-24 21:01:10]

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

[commit-bot: I haz the power, 2015-04-24 21:01:12]

Failed to apply patch for
extensions/browser/extension_function_histogram_value.h:
While running git apply --index -3 -p1;
  error: patch failed:
extensions/browser/extension_function_histogram_value.h:1071
  Falling back to three-way merge...
  Applied patch to 'extensions/browser/extension_function_histogram_value.h'
with conflicts.
  U extensions/browser/extension_function_histogram_value.h

Patch:       extensions/browser/extension_function_histogram_value.h
Index: extensions/browser/extension_function_histogram_value.h
diff --git a/extensions/browser/extension_function_histogram_value.h
b/extensions/browser/extension_function_histogram_value.h
index
00ed4430205fbca19767d2e07933b29d430d5a8b..194de4119d8c2223b7a2528fc789bf3476a371f6
100644
--- a/extensions/browser/extension_function_histogram_value.h
+++ b/extensions/browser/extension_function_histogram_value.h
@@ -1071,6 +1071,11 @@ enum HistogramValue {
   FILEMANAGERPRIVATE_CONFIGUREPROVIDEDFILESYSTEM,
   SEARCHENGINESPRIVATE_GETDEFAULTSEARCHENGINES,
   SEARCHENGINESPRIVATE_SETSELECTEDSEARCHENGINE,
+  INSTANCEID_GETID,
+  INSTANCEID_GETCREATIONTIME,
+  INSTANCEID_GETTOKEN,
+  INSTANCEID_DELETETOKEN,
+  INSTANCEID_DELETEID,
   // Last entry: Add new entries above and ensure to update
   // tools/metrics/histograms/histograms.xml.
   ENUM_BOUNDARY

[commit-bot: I haz the power, 2015-04-24 21:02:08]

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

[jianli, 2015-04-24 21:19:53]

The CQ bit was checked by jianli@chromium.org

[jianli, 2015-04-24 21:19:53]

The patchset sent to the CQ was uploaded after l-g-t-m from
fgorski@chromium.org, asvitkine@chromium.org, kalman@chromium.org
Link to the patchset: https://codereview.chromium.org/1088593004/#ps120001
(title: "Sync again")

[commit-bot: I haz the power, 2015-04-24 21:20:56]

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

[commit-bot: I haz the power, 2015-04-24 21:40:52]

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

[commit-bot: I haz the power, 2015-04-24 21:40:52]

Try jobs failed on following builders:
  android_chromium_gn_compile_dbg on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)

[jianli, 2015-04-24 21:58:49]

The CQ bit was checked by jianli@chromium.org

[commit-bot: I haz the power, 2015-04-24 22:00:06]

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

[commit-bot: I haz the power, 2015-04-24 22:31:40]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2015-04-24 22:33:21]

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