Requiem for client_screen.js

client_screen.js currently defines a few global functions and a few 'private'
functions that are called in application.js and desktop_connected_view.js

This CL
1. Moves onClientStateChange_, updateStatistics_ into application.js as
   they are called from there and their implementation depends on application.js.
2. Moves declaration of globals clientSession and desktopConnectedView
   into application.js
3. Moves remoting.disconnect into remoting.Application.
4. Moves onResize and onVisibilityChanged into desktop_connected_view.js
5. Removes client_screen.js

BUG=461995

Review URL: https://codereview.chromium.org/952353002

Cr-Commit-Position: refs/heads/master@{#319195}

chrome/test/remoting/remote_desktop_browsertest.cc

@@ -387,7 +387,7 @@ void RemoteDesktopBrowserTest::DisconnectMe2Me() {
 
   ASSERT_TRUE(RemoteDesktopBrowserTest::IsSessionConnected());
 
-  ExecuteScript("remoting.disconnect();");
+  ExecuteScript("remoting.app.disconnect();");
 
   EXPECT_TRUE(HtmlElementVisible("client-dialog"));
   EXPECT_TRUE(HtmlElementVisible("client-reconnect-button"));

remoting/remoting_webapp_files.gypi

@@ -146,10 +146,6 @@
       'webapp/crd/js/client_plugin.js',
       'webapp/crd/js/client_plugin_impl.js',
       'webapp/crd/js/client_plugin_host_desktop_impl.js',
-      # TODO(garykac) For client_screen:
-      # * Split out pin/access code stuff into separate file.
-      # * Move client logic into session_connector
-      'webapp/crd/js/client_screen.js',
       'webapp/crd/js/client_session.js',
       'webapp/crd/js/clipboard.js',
       'webapp/crd/js/credentials_provider.js',

remoting/webapp/app_remoting/js/app_remoting.js

@@ -144,7 +144,7 @@ remoting.AppRemoting.prototype.init = function(connector) {
 
         var idleDetector = new remoting.IdleDetector(
             document.getElementById('idle-dialog'),
-            remoting.disconnect);
+            remoting.app.disconnect.bind(remoting.app));
 
         /**
          * @param {string} tokenUrl Token-issue URL received from the host.

remoting/webapp/base/js/application.js

@@ -12,6 +12,19 @@
 /** @suppress {duplicate} */
 var remoting = remoting || {};
 
+/**
+ * @type {remoting.ClientSession} The client session object, set once the
+ *     connector has invoked its onOk callback.
+ * TODO(garykac): Make clientSession a member var of Application.
+ */
+remoting.clientSession = null;
+
+/**
+ * @type {remoting.DesktopConnectedView} The client session view object, set
+ *     once the connector has invoked its onOk callback.
+ */
+remoting.desktopConnectedView = null;
+
 /**
  * @param {Array<string>} app_capabilities Array of application capabilities.
  * @constructor
@@ -40,6 +53,9 @@ remoting.Application = function(app_capabilities) {
    * @private
    */
   this.session_connector_ = null;
+
+  /** @private {base.Disposable} */
+  this.sessionConnectedHooks_ = null;
 };
 
 /**
@@ -93,6 +109,14 @@ remoting.Application.prototype.start = function() {
   this.delegate_.init(this.getSessionConnector());
 };
 
+/** Disconnect the remoting client. */
+remoting.Application.prototype.disconnect = function() {
+  if (remoting.clientSession) {
+    remoting.clientSession.disconnect(remoting.Error.NONE);
+    console.log('Disconnected.');
+  }
+};
+
 /**
  * Called when a new session has been connected.
  *
@@ -100,12 +124,13 @@ remoting.Application.prototype.start = function() {
  * @return {void} Nothing.
  */
 remoting.Application.prototype.onConnected = function(clientSession) {
-  // TODO(garykac): Make clientSession a member var of Application.
   remoting.clientSession = clientSession;
-  remoting.clientSession.addEventListener('stateChanged', onClientStateChange_);
-
+  this.sessionConnectedHooks_ = new base.Disposables(
+    new base.EventHook(
+      clientSession, 'stateChanged', this.onClientStateChange_.bind(this)),
+    new base.RepeatingTimer(this.updateStatistics_.bind(this), 1000)
+  );
   remoting.clipboard.startSession();
-  updateStatistics_();
 
   this.delegate_.handleConnected(clientSession);
 };
@@ -192,6 +217,48 @@ remoting.Application.prototype.getSessionConnector = function() {
   return this.session_connector_;
 };
 
+/**
+ * Callback function called when the state of the client plugin changes. The
+ * current and previous states are available via the |state| member variable.
+ *
+ * @param {remoting.ClientSession.StateEvent=} state
+ * @private
+ */
+remoting.Application.prototype.onClientStateChange_ = function(state) {
+  switch (state.current) {
+    case remoting.ClientSession.State.CLOSED:
+      console.log('Connection closed by host');
+      this.onDisconnected();
+      break;
+    case remoting.ClientSession.State.FAILED:
+      var error = remoting.clientSession.getError();
+      console.error('Client plugin reported connection failed: ' + error);
+      if (error === null) {
+        error = remoting.Error.UNEXPECTED;
+      }
+      this.onError(error);
+      break;
+
+    default:
+      console.error('Unexpected client plugin state: ' + state.current);
+      // This should only happen if the web-app and client plugin get out of
+      // sync, so MISSING_PLUGIN is a suitable error.
+      this.onError(remoting.Error.MISSING_PLUGIN);
+      break;
+  }
+
+  base.dispose(this.sessionConnectedHooks_);
+  this.sessionConnectedHooks_= null;
+  remoting.clientSession.dispose();
+  remoting.clientSession = null;
+};
+
+/** @private */
+remoting.Application.prototype.updateStatistics_ = function() {
+  var perfstats = remoting.clientSession.getPerfStats();
+  remoting.stats.update(perfstats);
+  remoting.clientSession.logStatistics(perfstats);
+};
 
 /**
  * @interface

remoting/webapp/base/js/base.js

@@ -534,7 +534,7 @@ base.EventSourceImpl.prototype = {
   *
   * @param {base.EventSource} src
   * @param {string} eventName
-  * @param {function(...?)} listener
+  * @param {Function} listener
   *
   * @constructor
   * @implements {base.Disposable}
@@ -553,9 +553,9 @@ base.EventHook.prototype.dispose = function() {
 /**
   * An event hook implementation for DOM Events.
   *
-  * @param {HTMLElement|Element} src
+  * @param {HTMLElement|Element|Window|HTMLDocument} src
   * @param {string} eventName
-  * @param {function(...?)} listener
+  * @param {Function} listener
   * @param {boolean} capture
   *
   * @constructor
@@ -578,7 +578,7 @@ base.DomEventHook.prototype.dispose = function() {
   * An event hook implementation for Chrome Events.
   *
   * @param {chrome.Event} src
-  * @param {function(...?)} listener
+  * @param {Function} listener
   *
   * @constructor
   * @implements {base.Disposable}
@@ -593,6 +593,21 @@ base.ChromeEventHook.prototype.dispose = function() {
   this.src_.removeListener(this.listener_);
 };
 
+/**
+ * A disposable repeating timer.
+ *
+ * @constructor
+ * @implements {base.Disposable}
+ */
+base.RepeatingTimer = function(/** Function */callback, /** number */interval) {
+  /** @private */
+  this.intervalId_ = window.setInterval(callback, interval);
+};
+
+base.RepeatingTimer.prototype.dispose = function() {
+  window.clearInterval(this.intervalId_);
+  this.intervalId_ = null;
+};
 
 /**
   * Converts UTF-8 string to ArrayBuffer.

remoting/webapp/browser_test/browser_test.js

@@ -223,7 +223,7 @@ browserTest.disconnect = function() {
     finishedButton = 'client-finished-it2me-button';
   }
 
-  remoting.disconnect();
+  remoting.app.disconnect();
 
   return browserTest.onUIMode(finishedMode).then(function() {
     browserTest.clickOnControl(finishedButton);

remoting/webapp/crd/js/client_screen.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.
-
-/**
- * @fileoverview
- * Functions related to the 'client screen' for Chromoting.
- */
-
-'use strict';
-
-/** @suppress {duplicate} */
-var remoting = remoting || {};
-
-/**
- * @type {remoting.ClientSession} The client session object, set once the
- *     connector has invoked its onOk callback.
- */
-remoting.clientSession = null;
-
-/**
- * @type {remoting.DesktopConnectedView} The client session object, set once the
- *     connector has invoked its onOk callback.
- */
-remoting.desktopConnectedView = null;
-
-/**
- * Update the remoting client layout in response to a resize event.
- *
- * @return {void} Nothing.
- */
-remoting.onResize = function() {
-  if (remoting.desktopConnectedView) {
-    remoting.desktopConnectedView.onResize();
-  }
-};
-
-/**
- * Handle changes in the visibility of the window, for example by pausing video.
- *
- * @return {void} Nothing.
- */
-remoting.onVisibilityChanged = function() {
-  if (remoting.desktopConnectedView) {
-    remoting.desktopConnectedView.pauseVideo(
-      ('hidden' in document) ? document.hidden : document.webkitHidden);
-  }
-};
-
-/**
- * Disconnect the remoting client.
- *
- * @return {void} Nothing.
- */
-remoting.disconnect = function() {
-  if (!remoting.clientSession) {
-    return;
-  }
-  remoting.clientSession.disconnect(remoting.Error.NONE);
-  console.log('Disconnected.');
-};
-
-/**
- * Callback function called when the state of the client plugin changes. The
- * current and previous states are available via the |state| member variable.
- *
- * @param {remoting.ClientSession.StateEvent=} state
- */
-function onClientStateChange_(state) {
-  switch (state.current) {
-    case remoting.ClientSession.State.CLOSED:
-      console.log('Connection closed by host');
-      if (remoting.desktopConnectedView.getMode() ==
-          remoting.DesktopConnectedView.Mode.IT2ME) {
-        remoting.setMode(remoting.AppMode.CLIENT_SESSION_FINISHED_IT2ME);
-      } else {
-        remoting.setMode(remoting.AppMode.CLIENT_SESSION_FINISHED_ME2ME);
-      }
-      remoting.app.onDisconnected();
-      break;
-
-    case remoting.ClientSession.State.FAILED:
-      var error = remoting.clientSession.getError();
-      console.error('Client plugin reported connection failed: ' + error);
-      if (error == null) {
-        error = remoting.Error.UNEXPECTED;
-      }
-      remoting.app.onError(error);
-      break;
-
-    default:
-      console.error('Unexpected client plugin state: ' + state.current);
-      // This should only happen if the web-app and client plugin get out of
-      // sync, so MISSING_PLUGIN is a suitable error.
-      remoting.app.onError(remoting.Error.MISSING_PLUGIN);
-      break;
-  }
-
-  remoting.clientSession.removeEventListener('stateChanged',
-                                             onClientStateChange_);
-  remoting.clientSession.cleanup();
-  remoting.clientSession = null;
-  remoting.desktopConnectedView = null;
-}
-
-/**
- * Timer callback to update the statistics panel.
- */
-function updateStatistics_() {
-  if (!remoting.clientSession ||
-      remoting.clientSession.getState() !=
-      remoting.ClientSession.State.CONNECTED) {
-    return;
-  }
-  var perfstats = remoting.clientSession.getPerfStats();
-  remoting.stats.update(perfstats);
-  remoting.clientSession.logStatistics(perfstats);
-  // Update the stats once per second.
-  window.setTimeout(updateStatistics_, 1000);
-}

remoting/webapp/crd/js/client_session.js

@@ -42,6 +42,7 @@ remoting.ACCESS_TOKEN_RESEND_INTERVAL_MS = 15 * 60 * 1000;
  *     when the client doesn't define any.
  * @constructor
  * @extends {base.EventSourceImpl}
+ * @implements {base.Disposable}
  */
 remoting.ClientSession = function(host, signalStrategy, credentialsProvider,
                                   container, mode, defaultRemapKeys) {
@@ -311,12 +312,13 @@ remoting.ClientSession.prototype.resetWithError_ = function(error) {
 remoting.ClientSession.prototype.removePlugin = function() {
   this.uiHandler_.removePlugin();
   this.plugin_ = null;
+  remoting.desktopConnectedView = null;
 };
 
 /**
  * Disconnect the current session with a particular |error|.  The session will
  * raise a |stateChanged| event in response to it.  The caller should then call
- * |cleanup| to remove and destroy the <embed> element.
+ * dispose() to remove and destroy the <embed> element.
  *
  * @param {remoting.Error} error The reason for the disconnection.  Use
  *    remoting.Error.NONE if there is no error.
@@ -339,7 +341,7 @@ remoting.ClientSession.prototype.disconnect = function(error) {
  *
  * @return {void} Nothing.
  */
-remoting.ClientSession.prototype.cleanup = function() {
+remoting.ClientSession.prototype.dispose = function() {
   this.sendIq_(
       '<cli:iq ' +
           'to="' + this.host_.jabberId + '" ' +

remoting/webapp/crd/js/desktop_connected_view.js

@@ -60,9 +60,6 @@ remoting.DesktopConnectedView = function(session, container, host, mode,
    */
   this.onInitialized_ = onInitialized;
 
-  /** @type {function(boolean=):void} @private */
-  this.callOnFullScreenChanged_ = this.onFullScreenChanged_.bind(this)
-
   /** @private */
   this.callPluginLostFocus_ = this.pluginLostFocus_.bind(this);
   /** @private */
@@ -88,6 +85,9 @@ remoting.DesktopConnectedView = function(session, container, host, mode,
 
   /** @type {remoting.VideoFrameRecorder} @private */
   this.videoFrameRecorder_ = null;
+
+  /** private {base.Disposable} */
+  this.eventHooks_ = null;
 };
 
 // The mode of this session.
@@ -222,13 +222,22 @@ remoting.DesktopConnectedView.prototype.onPluginInitialized_ = function(
  * This is a callback that gets called when the window is resized.
  *
  * @return {void} Nothing.
+ * @private.
  */
-remoting.DesktopConnectedView.prototype.onResize = function() {
+remoting.DesktopConnectedView.prototype.onResize_ = function() {
   if (this.viewport_) {
     this.viewport_.onResize();
   }
 };
 
+/**
+ * Called when the app window is hidden.
+ * @return {void} Nothing.
+ */
+remoting.DesktopConnectedView.prototype.onVisibilityChanged_ = function() {
+  this.pauseVideo(document.hidden);
+};
+
 /**
  * Callback that the plugin invokes to indicate when the connection is
  * ready.
@@ -269,7 +278,7 @@ remoting.DesktopConnectedView.prototype.removePlugin = function() {
  */
 remoting.DesktopConnectedView.prototype.updateClientSessionUi_ = function(
     clientSession) {
-  if (clientSession == null) {
+  if (clientSession === null) {
     if (remoting.windowFrame) {
       remoting.windowFrame.setDesktopConnectedView(null);
     }
@@ -283,9 +292,8 @@ remoting.DesktopConnectedView.prototype.updateClientSessionUi_ = function(
     document.body.classList.remove('connected');
     this.container_.removeEventListener(
         'mousemove', this.updateMouseCursorPosition_, true);
-    // Stop listening for full-screen events.
-    remoting.fullscreen.removeListener(this.callOnFullScreenChanged_);
-
+    base.dispose(this.eventHooks_);
+    this.eventHooks_ = null;
     base.dispose(this.viewport_);
     this.viewport_ = null;
   } else {
@@ -307,11 +315,15 @@ remoting.DesktopConnectedView.prototype.updateClientSessionUi_ = function(
     document.body.classList.add('connected');
     this.container_.addEventListener(
         'mousemove', this.updateMouseCursorPosition_, true);
-
     // Activate full-screen related UX.
-    remoting.fullscreen.addListener(this.callOnFullScreenChanged_);
-    this.onFullScreenChanged_(remoting.fullscreen.isActive());
     this.setFocusHandlers_();
+    this.eventHooks_ = new base.Disposables(
+      new base.DomEventHook(window, 'resize', this.onResize_.bind(this), false),
+      new base.DomEventHook(document, 'visibilitychange',
+                            this.onVisibilityChanged_.bind(this), false),
+      new remoting.Fullscreen.EventHook(this.onFullScreenChanged_.bind(this))
+    );
+    this.onFullScreenChanged_(remoting.fullscreen.isActive());
   }
 };
 
@@ -356,6 +368,12 @@ remoting.DesktopConnectedView.prototype.setScreenMode =
 remoting.DesktopConnectedView.prototype.onFullScreenChanged_ = function (
     fullscreen) {
   if (this.viewport_) {
+    // When a window goes full-screen, a resize event is triggered, but the
+    // Fullscreen.isActive call is not guaranteed to return true until the
+    // full-screen event is triggered. In apps v2, the size of the window's
+    // client area is calculated differently in full-screen mode, so register
+    // for both events.
+    this.viewport_.onResize();
     this.viewport_.enableBumpScroll(Boolean(fullscreen));
   }
 };
@@ -582,4 +600,4 @@ remoting.DesktopConnectedView.prototype.handleDebugRegion = function(region) {
       this.debugRegionContainer_.appendChild(rect);
     }
   }
-}
+};

remoting/webapp/crd/js/desktop_remoting.js

@@ -97,17 +97,10 @@ remoting.DesktopRemoting.prototype.init = function() {
     remoting.optionsMenu = remoting.toolbar.createOptionsMenu();
 
     window.addEventListener('beforeunload', remoting.promptClose, false);
-    window.addEventListener('unload', remoting.disconnect, false);
+    window.addEventListener('unload',
+                            remoting.app.disconnect.bind(remoting.app), false);
   }
 
-  // When a window goes full-screen, a resize event is triggered, but the
-  // Fullscreen.isActive call is not guaranteed to return true until the
-  // full-screen event is triggered. In apps v2, the size of the window's
-  // client area is calculated differently in full-screen mode, so register
-  // for both events.
-  window.addEventListener('resize', remoting.onResize, false);
-  remoting.fullscreen.addListener(remoting.onResize);
-
   remoting.initHostlist_();
 
   var homeFeedback = new remoting.MenuButton(
@@ -233,6 +226,12 @@ remoting.DesktopRemoting.prototype.handleConnected = function(clientSession) {
  * @return {void} Nothing.
  */
 remoting.DesktopRemoting.prototype.handleDisconnected = function() {
+  if (remoting.desktopConnectedView.getMode() ==
+      remoting.DesktopConnectedView.Mode.IT2ME) {
+    remoting.setMode(remoting.AppMode.CLIENT_SESSION_FINISHED_IT2ME);
+  } else {
+    remoting.setMode(remoting.AppMode.CLIENT_SESSION_FINISHED_ME2ME);
+  }
 };
 
 /**

remoting/webapp/crd/js/fullscreen.js

@@ -50,3 +50,22 @@ remoting.Fullscreen.prototype.removeListener = function(callback) { };
 
 /** @type {remoting.Fullscreen} */
 remoting.fullscreen = null;
+
+
+/**
+ * @constructor
+ * @param {function(boolean=)} listener
+ * @implements {base.Disposable}
+ */
+remoting.Fullscreen.EventHook = function(listener) {
+  /** @private */
+  this.src_ = remoting.fullscreen;
+  /** @private */
+  this.listener_ = listener;
+
+  this.src_.addListener(listener);
+};
+
+remoting.Fullscreen.EventHook.prototype.dispose = function() {
+  this.src_.removeListener(this.listener_);
+};
\ No newline at end of file

remoting/webapp/crd/js/smart_reconnector.js

@@ -68,7 +68,7 @@ remoting.SmartReconnector.kConnectionTimeout = 10000;
 remoting.SmartReconnector.prototype = {
   reconnect_: function() {
     this.cancelPending_();
-    remoting.disconnect();
+    remoting.app.disconnect();
     remoting.setMode(remoting.AppMode.CLIENT_CONNECTING);
     this.connector_.reconnect();
   },

remoting/webapp/crd/js/toolbar.js

@@ -63,9 +63,10 @@ remoting.Toolbar = function(toolbar) {
   window.addEventListener('mousemove', remoting.Toolbar.onMouseMove, false);
   window.addEventListener('resize', this.center.bind(this), false);
 
-  registerEventListener('toolbar-disconnect', 'click', remoting.disconnect);
-  registerEventListener('toolbar-stub', 'click',
-      function() { remoting.toolbar.toggle(); });
+  registerEventListener('toolbar-disconnect', 'click',
+                        remoting.app.disconnect.bind(remoting.app));
+  registerEventListener('toolbar-stub',
+                        'click', function() { remoting.toolbar.toggle(); });
 
   // Prevent the preview canceling if the user is interacting with the tool-bar.
   /** @type {remoting.Toolbar} */

remoting/webapp/crd/js/ui_mode.js

@@ -123,20 +123,9 @@ remoting.setMode = function(mode) {
   if (mode == remoting.AppMode.IN_SESSION) {
     document.removeEventListener('keydown', remoting.ConnectionStats.onKeydown,
                                  false);
-    if ('hidden' in document) {
-      document.addEventListener('visibilitychange',
-                                remoting.onVisibilityChanged, false);
-    } else {
-      document.addEventListener('webkitvisibilitychange',
-                                remoting.onVisibilityChanged, false);
-    }
   } else {
     document.addEventListener('keydown', remoting.ConnectionStats.onKeydown,
                               false);
-    document.removeEventListener('visibilitychange',
-                                 remoting.onVisibilityChanged, false);
-    document.removeEventListener('webkitvisibilitychange',
-                                 remoting.onVisibilityChanged, false);
     // TODO(jamiewalch): crbug.com/252796: Remove this once crbug.com/240772
     // is fixed.
     var scroller = document.getElementById('scroller');

remoting/webapp/crd/js/window_frame.js

@@ -150,7 +150,7 @@ remoting.WindowFrame.prototype.disconnectSession_ = function() {
   if (chrome.app.window.current().isFullscreen()) {
     chrome.app.window.current().restore();
   }
-  remoting.disconnect();
+  remoting.app.disconnect();
 };
 
 /**