Add types.private.ChromeDirectSetting and Connect it to preferencesPrivate.googleGeolocationAccessE…

chrome/common/extensions/api/types_private.json

Index: chrome/common/extensions/api/types_private.json
diff --git a/chrome/common/extensions/api/types_private.json b/chrome/common/extensions/api/types_private.json
new file mode 100644
index 0000000000000000000000000000000000000000..10df291b7776180e319c5ce1f5995edd3a934424
--- /dev/null
+++ b/chrome/common/extensions/api/types_private.json
@@ -0,0 +1,143 @@
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.

[miket_OOO, 2013/07/10 18:34:47]

new file, current year

[robliao, 2013/07/10 20:20:09]

Done.

+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+[
+  {
+    "namespace": "types.private",
+    "description": "The <code>chrome.types.private</code> API contains private type declarations for Chrome. ",

[miket_OOO, 2013/07/10 18:34:47]

extra whitespace inside quote

[robliao, 2013/07/10 20:20:09]

Done.

+    "types": [
+      {
+        "id": "ChromeDirectSetting",
+        "type": "object",
+        "js_module": "ChromeDirectSetting",
+        "customBindings": "ChromeDirectSetting",
+        "description": "An interface which allows component extensions direct access to a Chrome browser setting.",

[miket_OOO, 2013/07/10 18:34:47]

which => that (restrictive, not descriptive)

[robliao, 2013/07/10 20:20:09]

Done.

+        "functions": [
+          {
+            "name": "get",
+            "type": "function",
+            "description": "Gets the value of a setting.",
+            "parameters": [
+              {
+                "name": "details",
+                "type": "object",
+                "description": "What setting to consider.",

[miket_OOO, 2013/07/10 18:34:47]

What => Which

[robliao, 2013/07/10 20:20:09]

All of these maintain consistency with the published types.json. We would need
to update types.json as well.
On 2013/07/10 18:34:47, miket wrote:

[miket_OOO, 2013/07/10 21:11:48]

There's absolutely nothing stopping you from improving the existing code,
especially in the documentation case where there isn't any
backward-compatibility issue. See
http://programmer.97things.oreilly.com/wiki/index.php/The_Boy_Scout_Rule. But
it's your CL. Do what you want.

+                "properties": {
+                  "incognito": {
+                    "type": "boolean",
+                    "optional": true,
+                    "description": "Whether to return the setting that applies to the incognito session (default false)."

[miket_OOO, 2013/07/10 18:34:47]

Is it the setting, or the value?

[robliao, 2013/07/10 20:20:09]

Value. Fixed. 

A comment question above that was missed is can anyone envision a scenario where
component extensions would need access to the incognito settings?
If not, I'd prefer to remove these right here.

On 2013/07/10 18:34:47, miket wrote:

+                  }
+                }
+              },
+              {
+                "name": "callback",
+                "type": "function",
+                "parameters": [
+                  {
+                    "name": "details",
+                    "type": "object",
+                    "description": "Details of the currently effective value.",
+                    "properties": {
+                      "value": {
+                        "description": "The value of the setting.",
+                        "type": "any"
+                      },
+                      "incognitoSpecific": {
+                        "description": "Whether the effective value is specific to the incognito session.<br>This property will <em>only</em> be present if the <var>incognito</var> property in the <var>details</var> parameter of <code>get()</code> was true.",
+                        "type": "boolean",
+                        "optional": true
+                      }
+                    }
+                  }
+                ]
+              }
+            ]
+          },
+          {
+            "name": "set",
+            "type": "function",
+            "description": "Sets the value of a setting.",
+            "parameters": [
+              {
+                "name": "details",
+                "type": "object",
+                "description": "What setting to change.",

[miket_OOO, 2013/07/10 18:34:47]

Which

[robliao, 2013/07/10 20:20:09]

Done.

+                "properties": {
+                  "value": {
+                    "description": "The value of the setting. <br>Note that every setting has a specific value type, which is described together with the setting. An extension should <em>not</em> set a value of a different type.",
+                    "type": "any"
+                  },
+                  "scope": {
+                    "type": "string",
+                    "enum": ["regular", "regular_only", "incognito_persistent", "incognito_session_only"],
+                    "optional": true,
+                    "description": "Where to set the setting (default: regular). One of<br><var>regular</var>: setting for the regular profile (which is inherited by the incognito profile if not overridden elsewhere),<br><var>regular_only</var>: setting for the regular profile only (not inherited by the incognito profile),<br><var>incognito_persistent</var>: setting for the incognito profile that survives browser restarts (overrides regular preferences),<br><var>incognito_session_only</var>: setting for the incognito profile that can only be set during an incognito session and is deleted when the incognito session ends (overrides regular and incognito_persistent preferences)."

[miket_OOO, 2013/07/10 18:34:47]

Consider a <ul> rather than series of <br>. It might look nicer and will give
more clues to accessibility tools.

[robliao, 2013/07/10 20:20:09]

The follows the style of types.json.

If we want to update it, I'll send a separate CL for types.json.
On 2013/07/10 18:34:47, miket wrote:

+                  }
+                }
+              },
+              {
+                "name": "callback",
+                "type": "function",
+                "description": "Called after the setting has been set.",

[miket_OOO, 2013/07/10 18:34:47]

To be insanely hyper-technical, this description could be misleading. Someone
might read it as a guarantee that the callback will be called *immediately*
after the setting's value changes. But it might actually happen after
housekeeping, or firing other events. Who knows. For this reason, you might want
to describe at a higher level: "Called at the completion of this operation" or
something like that, which clearly doesn't make the same promises, and which is
actually more true.

[robliao, 2013/07/10 20:20:09]

This was copy+pasted from types.json.

We would have to update both for consistency.
On 2013/07/10 18:34:47, miket wrote:

+                "optional": true,
+                "parameters": []
+              }
+            ]
+          },
+          {
+            "name": "clear",
+            "type": "function",
+            "description": "Clears the setting. This way default settings can become effective again.",

[miket_OOO, 2013/07/10 18:34:47]

The style of this description isn't consistent with the others. How about
something like this instead?

"Clears the setting and restores any default value."

[robliao, 2013/07/10 20:20:09]

Agreed. types.json would need to be updated.
On 2013/07/10 18:34:47, miket wrote:

+            "parameters": [
+              {
+                "name": "details",
+                "type": "object",
+                "description": "What setting to clear.",

[miket_OOO, 2013/07/10 18:34:47]

Which

+                "properties": {
+                  "scope": {
+                    "type": "string",
+                    "enum": ["regular", "regular_only", "incognito_persistent", "incognito_session_only"],
+                    "optional": true,
+                    "description": "Where to clear the setting (default: regular). One of<br><var>regular</var>: setting for the regular profile (which is inherited by the incognito profile if not overridden elsewhere),<br><var>regular_only</var>: setting for the regular profile only (not inherited by the incognito profile),<br><var>incognito_persistent</var>: setting for the incognito profile that survives browser restarts (overrides regular preferences),<br><var>incognito_session_only</var>: setting for the incognito profile that can only be set during an incognito session and is deleted when the incognito session ends (overrides regular and incognito_persistent preferences)."

[miket_OOO, 2013/07/10 18:34:47]

I think it'd be OK to say simply that this works the same way as set, rather
than an error-prone copy/paste of the same text.

[robliao, 2013/07/10 20:20:09]

For API reference docs, it's better to restate. Sometimes, methods are viewed in
isolation. It's annoying to have to jump around.
On 2013/07/10 18:34:47, miket wrote:

+                  }
+                }
+              },
+              {
+                "name": "callback",
+                "type": "function",
+                "description": "Called after the setting has been cleared.",

[miket_OOO, 2013/07/10 18:34:47]

Same as above

[robliao, 2013/07/10 20:20:09]

See response above.
On 2013/07/10 18:34:47, miket wrote:

+                "optional": true,
+                "parameters": []
+              }
+            ]
+          }
+        ],
+        "events": [
+          {
+            "name": "onChange",
+            "description": "Fired when the value of the setting changes.",

[miket_OOO, 2013/07/10 18:34:47]

I wonder whether "the value of" is overly verbose. Can a setting ever change
without the value of the setting changing?

[robliao, 2013/07/10 20:20:09]

I would be better to omit this. types.json would also need updating.
On 2013/07/10 18:34:47, miket wrote:

+            "parameters": [
+              {
+                "type": "object",
+                "name": "details",
+                "properties": {
+                  "value": {
+                    "description": "The value of the setting.",

[miket_OOO, 2013/07/10 18:34:47]

The *new* value?

[robliao, 2013/07/10 20:20:09]

The new is redundant. The setting has already changed. (types.json copy as well)
On 2013/07/10 18:34:47, miket wrote:

[miket_OOO, 2013/07/10 21:11:48]

A common pattern in some APIs like this is to return the existing value,
figuring that if you changed it, you already know what you changed it to, but
you're interested in the old value if you were doing some kind of audit log or
undo functionality.

It's up to you if you want it to be ambiguous. I suppose developers can test it
if their natural reading is opposite yours.

+                    "type": "any"
+                  },
+                  "incognitoSpecific": {
+                    "description": "Whether the value that has changed is specific to the incognito session.<br>This property will <em>only</em> be present if the user has enabled the extension in incognito mode.",

[miket_OOO, 2013/07/10 18:34:47]

It's more correct to say "This property will be present only if" rather than
"This property will only be present if."

[robliao, 2013/07/10 20:20:09]

Agreed. 

On all of these comments, if we can modify types.json with the new language, we
can update these strings.
If we cannot touch types.json (since it is already published), we should
maintain consistency with the existing language.

On 2013/07/10 18:34:47, miket wrote:

+                    "type": "boolean",
+                    "optional": true
+                  }
+                }
+              }
+            ]
+          }
+        ]
+      }
+    ]
+  }
+]