[Webapp Refactor] Fix styles in SmartReconnector.

remoting/webapp/base/js/base.js

 // Copyright 2014 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.
 
 /**
  * @fileoverview
  * A module that contains basic utility components and methods for the
  * chromoting project
  *
  */
@@ skipping 659 matching lines @@
  */
 base.RepeatingTimer = function(callback, interval, opt_invokeNow) {
   /** @private */
   this.intervalId_ = window.setInterval(callback, interval);
   if (opt_invokeNow) {
     callback();
   }
 };
 
 base.RepeatingTimer.prototype.dispose = function() {
   window.clearInterval(this.intervalId_);

[Jamie, 2015/04/16 20:05:49]

I think you need to route the callback via a member function that will nullify
the intervalId, since I don't think they are guaranteed to be unique and the
current implementation risks cancelling another timer when this one is disposed.
You should also check for null here.

[kelvinp, 2015/04/16 20:57:58]

Added for the OneShotTimer.
For repeating timer, the callback is expected to call repeatedly so there is no
way to null out the timerId after the callback is invoked.
But good news is that the intervalId is guaranteed to be unique. See
https://developer.mozilla.org/en-US/docs/Web/API/WindowTimers/setInterval

[Jamie, 2015/04/17 00:02:47]

Yes, my mistake. I meant to attach this to OneShotTimer, though a null-check
can't hurt here too, just in case "unique" for setInterval only guarantees
uniqueness for timers running simultaneously.

   this.intervalId_ = null;
 };
 
 /**
+ * A disposable one shot timer.
+ *
+ * @constructor
+ * @implements {base.Disposable}
+ */
+base.OneShotTimer = function(/** Function */ callback, /** number */ timeout) {

[Jamie, 2015/04/16 20:05:48]

We have used the longer form of jsdoc annotation for regular functions,
particularly since you already have a comment block above the method. It
provides an option for documenting the parameters (not necessary in this case, I
don't think). This short form should be used for anonymous inline functions.

[kelvinp, 2015/04/16 20:57:58]

Done.

+  /** @private */
+  this.timerId_ = window.setTimeout(callback, timeout);
+};
+
+base.OneShotTimer.prototype.dispose = function() {
+  window.clearTimeout(this.timerId_);
+  this.timerId_ = null;
+};
+
+/**
   * Converts UTF-8 string to ArrayBuffer.
   *
   * @param {string} string
   * @return {ArrayBuffer}
   */
 base.encodeUtf8 = function(string) {
   var utf8String = unescape(encodeURIComponent(string));
   var result = new Uint8Array(utf8String.length);
   for (var i = 0; i < utf8String.length; i++)
     result[i] = utf8String.charCodeAt(i);
@@ skipping 56 matching lines @@
  */
 base.resizeWindowToContent = function() {
   var appWindow = chrome.app.window.current();
   var outerBounds = appWindow.outerBounds;
   var borderY = outerBounds.height - appWindow.innerBounds.height;
   appWindow.resizeTo(outerBounds.width, document.body.clientHeight + borderY);
   // Sometimes, resizing the window causes its position to be reset to (0, 0),
   // so restore it explicitly.
   appWindow.moveTo(outerBounds.left, outerBounds.top);
 };