Add a Preference for Google Services to Use the User's Location

chrome/browser/resources/options/geolocation_options.js

+// Copyright (c) 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+cr.define('options', function() {
+  var OptionsPage = options.OptionsPage;
+
+  /**
+   * GeolocationOptions class

[vadimt, 2013/06/07 00:56:07]

It would help to end sentences with '.'.

[robliao, 2013/06/07 07:34:36]

Done.

+   * Shows the component geolocation option when Google Now is enabled

[vadimt, 2013/06/07 00:56:07]

'when Google Now is enabled' belongs to the logic of the calling code, so
shouldn't be here. Ad addition, there will be other triggers in future.

[robliao, 2013/06/07 07:34:36]

Done.

+   * @constructor
+   * @class
+   */
+  function GeolocationOptions() {
+    OptionsPage.call(this,

[vadimt, 2013/06/07 00:56:07]

Curious: what does this call do?

[robliao, 2013/06/07 07:34:36]

This initializes the our "parent class". The other handlers also seem to do
this.

https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/res...
On 2013/06/07 00:56:07, vadimt wrote:

[robliao, 2013/06/07 07:34:36]

Done.

[vadimt, 2013/06/07 17:16:28]

No need to additionally add "done" comments when you already answered.

+                     'geolocationOptions',
+                     loadTimeData.getString('geolocationOptionsPageTabTitle'),
+                     'geolocation-options');
+  };
+
+  cr.addSingletonGetter(GeolocationOptions);
+
+  GeolocationOptions.prototype = {
+    __proto__: OptionsPage.prototype
+  };
+
+  GeolocationOptions.showGeolocationOption = function() {
+      var geolocationCheckboxContainer = $('geolocationCheckbox');
+      if (geolocationCheckboxContainer) {

[vadimt, 2013/06/07 00:56:07]

No need in curlies.

[robliao, 2013/06/07 07:34:36]

The Google JS style guide is written with curlies for one-liners. I like them
better as well because if future changes are required in this block, there's no
need to add the curlies.
On 2013/06/07 00:56:07, vadimt wrote:

[robliao, 2013/06/07 07:34:36]

Done.

[vadimt, 2013/06/07 17:16:28]

I'm OK with this; however reviewers used to require me to remove curlies from
single-liners...

+        geolocationCheckboxContainer.hidden = false;
+      }
+    };
+
+  return { GeolocationOptions: GeolocationOptions };

[vadimt, 2013/06/07 00:56:07]

return {GeolocationOptions: GeolocationOptions};

[robliao, 2013/06/07 07:34:36]

I went ahead and matched the style in the other files: three separate lines.
On 2013/06/07 00:56:07, vadimt wrote:

[robliao, 2013/06/07 07:34:36]

Done.

[vadimt, 2013/06/07 17:16:28]

Your choice, but if it fits in 1 line, you could also use the format I
suggested.

+});