[Webapp Refactor] Fix styles in SmartReconnector.

[Webapp Refactor] Fix styles in SmartReconnector to be consistent with the rest of the codebase.

Summary of changes:
1. Uses base.OneShotTimer
2. Uses base.EventHooks
3. Uses
     SmartReconnector.prototype.foo = function() {...}
   instead of
     SmartReconnector.prototype = {
       foo: function() {...},
     }
4 Removes this.bound_

BUG=

Committed: https://crrev.com/18bad48c097ce68bccf2064daf41f6f6bb3649df
Cr-Commit-Position: refs/heads/master@{#325578}

[kelvinp, 2015-04-16 19:33:44]

kelvinp@chromium.org changed reviewers:
+ jamiewalch@chromium.org

[kelvinp, 2015-04-16 19:35:03]

PTAL

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

https://codereview.chromium.org/1091103002/diff/20001/remoting/webapp/base/js...
File remoting/webapp/base/js/base.js (right):

https://codereview.chromium.org/1091103002/diff/20001/remoting/webapp/base/js...
remoting/webapp/base/js/base.js:680: window.clearInterval(this.intervalId_);
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.

https://codereview.chromium.org/1091103002/diff/20001/remoting/webapp/base/js...
remoting/webapp/base/js/base.js:690: base.OneShotTimer = function(/** Function
*/ callback, /** number */ timeout) {
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.

https://codereview.chromium.org/1091103002/diff/20001/remoting/webapp/crd/js/...
File remoting/webapp/crd/js/smart_reconnector.js (right):

https://codereview.chromium.org/1091103002/diff/20001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/smart_reconnector.js:43: this.pending_ = null;
You're using this for two separate pending operations, which forces you to give
it a nondescript name. How do you feel about using separate members with better
names?

https://codereview.chromium.org/1091103002/diff/20001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/smart_reconnector.js:64: base.dispose(this.pending_);
Reset pending_ to null, here and below.

https://codereview.chromium.org/1091103002/diff/20001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/smart_reconnector.js:100: this.pending_ = new
base.DomEventHook(
Indentation.

[kelvinp, 2015-04-16 20:57:59]

PTAL

https://codereview.chromium.org/1091103002/diff/20001/remoting/webapp/base/js...
File remoting/webapp/base/js/base.js (right):

https://codereview.chromium.org/1091103002/diff/20001/remoting/webapp/base/js...
remoting/webapp/base/js/base.js:680: window.clearInterval(this.intervalId_);
On 2015/04/16 20:05:49, Jamie wrote:
> 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.

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

https://codereview.chromium.org/1091103002/diff/20001/remoting/webapp/base/js...
remoting/webapp/base/js/base.js:690: base.OneShotTimer = function(/** Function
*/ callback, /** number */ timeout) {
On 2015/04/16 20:05:48, Jamie wrote:
> 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.

Done.

https://codereview.chromium.org/1091103002/diff/20001/remoting/webapp/crd/js/...
File remoting/webapp/crd/js/smart_reconnector.js (right):

https://codereview.chromium.org/1091103002/diff/20001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/smart_reconnector.js:43: this.pending_ = null;
On 2015/04/16 20:05:49, Jamie wrote:
> You're using this for two separate pending operations, which forces you to
give
> it a nondescript name. How do you feel about using separate members with
better
> names?

|this.pending_| is intended to be a placeholder of a pending reconnect
operation that is cancel-able, regardless of the type.

https://codereview.chromium.org/1091103002/diff/20001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/smart_reconnector.js:64: base.dispose(this.pending_);
On 2015/04/16 20:05:49, Jamie wrote:
> Reset pending_ to null, here and below.

Done.

https://codereview.chromium.org/1091103002/diff/20001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/smart_reconnector.js:100: this.pending_ = new
base.DomEventHook(
On 2015/04/16 20:05:49, Jamie wrote:
> Indentation.

Done.

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

lgtm

https://codereview.chromium.org/1091103002/diff/20001/remoting/webapp/base/js...
File remoting/webapp/base/js/base.js (right):

https://codereview.chromium.org/1091103002/diff/20001/remoting/webapp/base/js...
remoting/webapp/base/js/base.js:680: window.clearInterval(this.intervalId_);
On 2015/04/16 20:57:58, kelvinp wrote:
> On 2015/04/16 20:05:49, Jamie wrote:
> > 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.
> 
> 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

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.

https://codereview.chromium.org/1091103002/diff/20001/remoting/webapp/crd/js/...
File remoting/webapp/crd/js/smart_reconnector.js (right):

https://codereview.chromium.org/1091103002/diff/20001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/smart_reconnector.js:100: this.pending_ = new
base.DomEventHook(
On 2015/04/16 20:57:58, kelvinp wrote:
> On 2015/04/16 20:05:49, Jamie wrote:
> > Indentation.
> 
> Done.

This still looks wrong. This line is indented by 4 spaces w.r.t. the previous,
not 2.

[kelvinp, 2015-04-17 00:12:53]

FYI

https://codereview.chromium.org/1091103002/diff/20001/remoting/webapp/crd/js/...
File remoting/webapp/crd/js/smart_reconnector.js (right):

https://codereview.chromium.org/1091103002/diff/20001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/smart_reconnector.js:100: this.pending_ = new
base.DomEventHook(
On 2015/04/17 00:02:47, Jamie wrote:
> On 2015/04/16 20:57:58, kelvinp wrote:
> > On 2015/04/16 20:05:49, Jamie wrote:
> > > Indentation.
> > 
> > Done.
> 
> This still looks wrong. This line is indented by 4 spaces w.r.t. the previous,
> not 2.

My bad.  My this.pending_ change overwrote the indenting fix.  Now fixed.

[kelvinp, 2015-04-17 00:13:00]

The CQ bit was checked by kelvinp@chromium.org

[kelvinp, 2015-04-17 00:13:00]

The patchset sent to the CQ was uploaded after l-g-t-m from
jamiewalch@chromium.org
Link to the patchset: https://codereview.chromium.org/1091103002/#ps60001
(title: "Fix Indent")

[commit-bot: I haz the power, 2015-04-17 00:13:59]

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

[commit-bot: I haz the power, 2015-04-17 01:57:42]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-04-17 01:58:40]

Patchset 4 (id:??) landed as
https://crrev.com/18bad48c097ce68bccf2064daf41f6f6bb3649df
Cr-Commit-Position: refs/heads/master@{#325578}