Add public API generation with cr.makePublic() and handle it in compiler pass

chrome/browser/resources/options/browser_options.js

 // Copyright (c) 2012 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;
   var Page = cr.ui.pageManager.Page;
   var PageManager = cr.ui.pageManager.PageManager;
   var ArrayDataModel = cr.ui.ArrayDataModel;
   var RepeatingButton = cr.ui.RepeatingButton;
   var HotwordSearchSettingIndicator = options.HotwordSearchSettingIndicator;
   var NetworkPredictionOptions = {
     ALWAYS: 0,
     WIFI_ONLY: 1,
     NEVER: 2,
     UNSET: 3,
     DEFAULT: 1
   };
 
-  //
-  // BrowserOptions class
-  // Encapsulated handling of browser options page.
-  //
+  /**
+   * Encapsulated handling of browser options page.
+   * @constructor
+   * @extends {cr.ui.pageManager.Page}
+   */
   function BrowserOptions() {
     Page.call(this, 'settings', loadTimeData.getString('settingsTitle'),
               'settings');
   }
 
   cr.addSingletonGetter(BrowserOptions);
 
   /**
    * @param {HTMLElement} section The section to show or hide.
    * @return {boolean} Whether the section should be shown.
@@ skipping 1387 matching lines @@
 
     /**
      * Propmpts user to confirm deletion of the profile for this browser
      * window.
      * @private
      */
     deleteCurrentProfile_: function() {
       ManageProfileOverlay.showDeleteDialog(this.getCurrentProfile_());
     },
 
+    /**
+     * @param {boolean} enabled
+     */
     setNativeThemeButtonEnabled_: function(enabled) {
       var button = $('themes-native-button');
       if (button)
         button.disabled = !enabled;
     },
 
+    /**
+     * @param {boolean} enabled
+     */
     setThemesResetButtonEnabled_: function(enabled) {
       $('themes-reset').disabled = !enabled;
     },
 
+    /**
+     * @param {boolean} managed
+     */
     setAccountPictureManaged_: function(managed) {
       var picture = $('account-picture');
       if (managed || UIAccountTweaks.loggedInAsGuest()) {
         picture.disabled = true;
         ChangePictureOptions.closeOverlay();
       } else {
         picture.disabled = false;
       }
 
       // Create a synthetic pref change event decorated as
@@ skipping 11 matching lines @@
      * @private
      */
     updateAccountPicture_: function() {
       var picture = $('account-picture');
       if (picture) {
         picture.src = 'chrome://userimage/' + this.username_ + '?id=' +
             Date.now();
       }
     },
 
+    /**
+     * @param {boolean} managed
+     */
     setWallpaperManaged_: function(managed) {
       var button = $('set-wallpaper');
       button.disabled = !!managed;
 
       // Create a synthetic pref change event decorated as
       // CoreOptionsHandler::CreateValueForPref() does.
       var event = new Event('wallpaper');
       if (managed)
         event.value = { controlledBy: 'policy' };
       else
@@ skipping 423 matching lines @@
 
     /**
      * Handle the 'set date and time' button click.
      * @private
      */
     handleSetTime_: function() {
       chrome.send('showSetTime');
     },
   };
 
+  /**
+   * Fake declaration made for Closure Compiler. Actual method definition is
+   * in the base class.
+   * @param {Array} pages List of startup pages.
+   * @private
+   */
+  BrowserOptions.prototype.updateStartupPages_;

[Dan Beam, 2014/09/10 19:26:27]

please minimize the amount of useless code we write for just the compiler at all
costs

[Vitaly Pavlenko, 2014/09/10 20:25:21]

For this feature I need exactly these three fake declarations, no more.
Otherwise I can write some kind of suppression options for makePublic, for the
cost of additional awkward code in JS and in Compiler pass.

So I suggest to leave these three fake declarations. What do you think?

[Tyler Breisacher (Chromium), 2014/09/10 20:50:32]

Hang on, if this is a private method of the base class, it shouldn't be
accessible here. It should probably be protected, not private, no?

[Dan Beam, 2014/09/10 21:08:19]

see other comments on this review

[Dan Beam, 2014/09/12 00:57:51]

no, if possible, don't add them

[Vitaly Pavlenko, 2014/09/12 18:38:07]

Ok, then I'd like to remove that as a dead code :)

+
   //Forward public APIs to private implementations.
-  [
+  cr.makePublic(BrowserOptions, [
     'addBluetoothDevice',
     'deleteCurrentProfile',
     'enableCertificateButton',
     'enableFactoryResetSection',
     'getCurrentProfile',
     'getStartStopSyncButton',
     'hideBluetoothSettings',
     'notifyInitializationComplete',
     'removeBluetoothDevice',
     'scrollToSection',
@@ skipping 26 matching lines @@
     'showTouchpadControls',
     'toggleExtensionIndicators',
     'updateAccountPicture',
     'updateAutoLaunchState',
     'updateDefaultBrowserState',
     'updateEasyUnlock',
     'updateManagesSupervisedUsers',
     'updateSearchEngines',
     'updateStartupPages',
     'updateSyncState',
-  ].forEach(function(name) {
-    BrowserOptions[name] = function() {
-      var instance = BrowserOptions.getInstance();
-      return instance[name + '_'].apply(instance, arguments);
-    };
-  });
+  ]);
 
   if (cr.isChromeOS) {
     /**
      * Returns username (canonical email) of the user logged in (ChromeOS only).
      * @return {string} user email.
      */
     // TODO(jhawkins): Investigate the use case for this method.
     BrowserOptions.getLoggedInUsername = function() {
       return BrowserOptions.getInstance().username_;
     };
   }
 
   // Export
   return {
     BrowserOptions: BrowserOptions
   };
 });