base.EventHook

base.EventHook

Currently most objects in the Chromoting Webapp doesn't unhook events until
the page is unloaded.  This introduces unintentional coupling between an object's
lifetime and the app's lifetime.

This CL implements a lightweight utility class to make unhooking listeners easier.

BUG=454552
Committed: https://crrev.com/c67ab01d1b437f04de1441160ba013bb37ceffc2
Cr-Commit-Position: refs/heads/master@{#314278}

[kelvinp, 2015-02-02 22:56:25]

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

[kelvinp, 2015-02-02 22:56:43]

Patchset #2 (id:20001) has been deleted

[kelvinp, 2015-02-02 23:07:29]

Patchset #1 (id:1) has been deleted

[kelvinp, 2015-02-02 23:08:34]

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

[kelvinp, 2015-02-02 23:08:34]

PTAL

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

https://codereview.chromium.org/863863003/diff/40001/remoting/webapp/base/js/...
remoting/webapp/base/js/base.js:477: * @param {Function} listener
Function is the JavaScript official type name of function.  It is used in flavor
of function(?) here as it doesn't place restrictions on the number/type of
arguments, and the return type.

[Jamie, 2015-02-02 23:46:19]

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

[Jamie, 2015-02-02 23:46:20]

LGTM with nits.

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

https://codereview.chromium.org/863863003/diff/40001/remoting/webapp/base/js/...
remoting/webapp/base/js/base.js:455: * An light weight object that helps
managing the lifetime of an event
s/An/A/
s/light weight/lightweight/
s/managing/manage/
s/event/event./

https://codereview.chromium.org/863863003/diff/40001/remoting/webapp/base/js/...
remoting/webapp/base/js/base.js:470: * MyConstructor.prototype.dispose =
function () {
No space before ()--even for comments :)

https://codereview.chromium.org/863863003/diff/40001/remoting/webapp/base/js/...
remoting/webapp/base/js/base.js:490: 'opt_capture is only valid for DOM
Events.');
I think this would be better guarded against by separate EventHook and
DomEventHook classes. The fact that the two types of event use the same names
for the un/hook functions shouldn't translate to using the same wrapper
functions. If they both implemented the same interface, then that would be
another matter...

[kelvinp, 2015-02-03 01:41:52]

kelvinp@chromium.org changed reviewers:
- jmadill@chromium.org

[kelvinp, 2015-02-03 01:42:01]

FYI

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

https://codereview.chromium.org/863863003/diff/40001/remoting/webapp/base/js/...
remoting/webapp/base/js/base.js:470: * MyConstructor.prototype.dispose =
function () {
On 2015/02/02 23:46:20, Jamie wrote:
> No space before ()--even for comments :)

Done.

[kelvinp, 2015-02-03 01:42:05]

The CQ bit was checked by kelvinp@chromium.org

[commit-bot: I haz the power, 2015-02-03 01:42:56]

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

[commit-bot: I haz the power, 2015-02-03 03:44:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-02-03 03:44:18]

Try jobs failed on following builders:
  win8_chromium_rel on tryserver.chromium.win (None)

[kelvinp, 2015-02-03 05:36:41]

The CQ bit was checked by kelvinp@chromium.org

[commit-bot: I haz the power, 2015-02-03 05:37:42]

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

[commit-bot: I haz the power, 2015-02-03 06:14:52]

Committed patchset #2 (id:60001)

[commit-bot: I haz the power, 2015-02-03 06:15:46]

Patchset 2 (id:??) landed as
https://crrev.com/c67ab01d1b437f04de1441160ba013bb37ceffc2
Cr-Commit-Position: refs/heads/master@{#314278}