From c67ab01d1b437f04de1441160ba013bb37ceffc2 Mon Sep 17 00:00:00 2001 From: kelvinp Date: Mon, 2 Feb 2015 22:14:26 -0800 Subject: [PATCH] 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 Review URL: https://codereview.chromium.org/863863003 Cr-Commit-Position: refs/heads/master@{#314278} --- remoting/remoting_webapp_files.gypi | 1 + remoting/webapp/base/js/base.js | 110 ++++++++++++++++++++++- remoting/webapp/unittests/event_hook_unittest.js | 68 ++++++++++++++ 3 files changed, 175 insertions(+), 4 deletions(-) create mode 100644 remoting/webapp/unittests/event_hook_unittest.js diff --git a/remoting/remoting_webapp_files.gypi b/remoting/remoting_webapp_files.gypi index 79f10e2cc085..da0597859043 100644 --- a/remoting/remoting_webapp_files.gypi +++ b/remoting/remoting_webapp_files.gypi @@ -178,6 +178,7 @@ 'webapp/unittests/apps_v2_migration_unittest.js', 'webapp/unittests/base_unittest.js', 'webapp/unittests/dns_blackhole_checker_unittest.js', + 'webapp/unittests/event_hook_unittest.js', 'webapp/unittests/fallback_signal_strategy_unittest.js', 'webapp/unittests/ipc_unittest.js', 'webapp/unittests/it2me_helpee_channel_unittest.js', diff --git a/remoting/webapp/base/js/base.js b/remoting/webapp/base/js/base.js index 11bd091acf0c..30f13a0aae24 100644 --- a/remoting/webapp/base/js/base.js +++ b/remoting/webapp/base/js/base.js @@ -62,6 +62,25 @@ base.Disposable = function() {}; base.Disposable.prototype.dispose = function() {}; /** + * @constructor + * @param {...base.Disposable} var_args + * @implements {base.Disposable} + */ +base.Disposables = function(var_args) { + /** + * @type {Array.} + * @private + */ + this.disposables_ = Array.prototype.slice.call(arguments, 0); +}; + +base.Disposables.prototype.dispose = function() { + for (var i = 0; i < this.disposables_.length; i++) { + this.disposables_[i].dispose(); + } +}; + +/** * A utility function to invoke |obj|.dispose without a null check on |obj|. * @param {base.Disposable} obj */ @@ -116,7 +135,7 @@ base.values = function(dict) { * @return {*} a recursive copy of |value| or null if |value| is not copyable * (e.g. undefined, NaN). */ -base.deepCopy = function (value) { +base.deepCopy = function(value) { try { return JSON.parse(JSON.stringify(value)); } catch (e) {} @@ -432,6 +451,89 @@ base.EventSource.prototype = { } }; + +/** + * A lightweight object that helps manage the lifetime of an event listener. + * + * For example, do the following if you want to automatically unhook events + * when your object is disposed: + * + * var MyConstructor = function(domElement) { + * this.eventHooks_ = new base.Disposables( + * new base.EventHook(domElement, 'click', this.onClick_.bind(this)), + * new base.EventHook(domElement, 'keydown', this.onClick_.bind(this)), + * new base.ChromeEventHook(chrome.runtime.onMessage, + * this.onMessage_.bind(this)) + * ); + * } + * + * MyConstructor.prototype.dispose = function() { + * this.eventHooks_.dispose(); + * this.eventHooks_ = null; + * } + * + * @param {base.EventSource} src + * @param {string} eventName + * @param {function(...?)} listener + * + * @constructor + * @implements {base.Disposable} + */ +base.EventHook = function(src, eventName, listener) { + this.src_ = src; + this.eventName_ = eventName; + this.listener_ = listener; + src.addEventListener(eventName, listener); +}; + +base.EventHook.prototype.dispose = function() { + this.src_.removeEventListener(this.eventName_, this.listener_); +}; + +/** + * An event hook implementation for DOM Events. + * + * @param {HTMLElement} src + * @param {string} eventName + * @param {function(...?)} listener + * @param {boolean} capture + * + * @constructor + * @implements {base.Disposable} + */ +base.DomEventHook = function(src, eventName, listener, capture) { + this.src_ = src; + this.eventName_ = eventName; + this.listener_ = listener; + this.capture_ = capture; + src.addEventListener(eventName, listener, capture); +}; + +base.DomEventHook.prototype.dispose = function() { + this.src_.removeEventListener(this.eventName_, this.listener_, this.capture_); +}; + + +/** + * An event hook implementation for Chrome Events. + * + * @param {chrome.Event} src + * @param {function(...?)} listener + * + * @constructor + * @implements {base.Disposable} + */ +base.ChromeEventHook = function(src, listener) { + this.src_ = src; + this.listener_ = listener; + src.addListener(listener); +}; + +base.ChromeEventHook.prototype.dispose = function() { + this.src_.removeListener(this.listener_); +}; + + /** * Converts UTF-8 string to ArrayBuffer. * @@ -444,7 +546,7 @@ base.encodeUtf8 = function(string) { for (var i = 0; i < utf8String.length; i++) result[i] = utf8String.charCodeAt(i); return result.buffer; -} +}; /** * Decodes UTF-8 string from ArrayBuffer. @@ -455,7 +557,7 @@ base.encodeUtf8 = function(string) { base.decodeUtf8 = function(buffer) { return decodeURIComponent( escape(String.fromCharCode.apply(null, new Uint8Array(buffer)))); -} +}; /** * Generate a nonce, to be used as an xsrf protection token. @@ -479,4 +581,4 @@ base.jsonParseSafe = function(jsonString) { } catch (err) { return undefined; } -} +}; diff --git a/remoting/webapp/unittests/event_hook_unittest.js b/remoting/webapp/unittests/event_hook_unittest.js new file mode 100644 index 000000000000..83e5dcb70f79 --- /dev/null +++ b/remoting/webapp/unittests/event_hook_unittest.js @@ -0,0 +1,68 @@ +// 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. + +(function() { + +'use strict'; + +var eventSource = null; +var domElement = null; +var myChromeEvent = null; +var listener = null; + +var Listener = function(element) { + this.onChromeEvent = sinon.stub(); + this.onClickEvent = sinon.stub(); + this.onCustomEvent = sinon.stub(); + this.eventHooks_ = new base.Disposables( + new base.DomEventHook(element, 'click', this.onClickEvent.bind(this), + false), + new base.EventHook(eventSource, 'customEvent', + this.onCustomEvent.bind(this)), + new base.ChromeEventHook(myChromeEvent, this.onChromeEvent.bind(this))); +}; + +Listener.prototype.dispose = function() { + this.eventHooks_.dispose(); +}; + +function raiseAllEvents() { + domElement.click(); + myChromeEvent.mock$fire(); + eventSource.raiseEvent('customEvent'); +} + +module('base.EventHook', { + setup: function() { + domElement = document.createElement('div'); + eventSource = new base.EventSource(); + eventSource.defineEvents(['customEvent']); + myChromeEvent = new chromeMocks.Event(); + listener = new Listener(domElement); + }, + tearDown: function() { + domElement = null; + eventSource = null; + myChromeEvent = null; + listener = null; + } +}); + +test('EventHook should hook events when constructed', function() { + raiseAllEvents(); + sinon.assert.calledOnce(listener.onClickEvent); + sinon.assert.calledOnce(listener.onChromeEvent); + sinon.assert.calledOnce(listener.onCustomEvent); + listener.dispose(); +}); + +test('EventHook should unhook events when disposed', function() { + listener.dispose(); + raiseAllEvents(); + sinon.assert.notCalled(listener.onClickEvent); + sinon.assert.notCalled(listener.onChromeEvent); + sinon.assert.notCalled(listener.onCustomEvent); +}); + +})(); -- 2.11.4.GIT