CRD Viewport Management refactor

remoting/webapp/browser_test/bump_scroll_browser_test.js

Index: remoting/webapp/browser_test/bump_scroll_browser_test.js
diff --git a/remoting/webapp/browser_test/bump_scroll_browser_test.js b/remoting/webapp/browser_test/bump_scroll_browser_test.js
index 71545b412ec7a8d84aa6909104de908475784977..68ca67d26e8f41bfb580319448490a78a70fca88 100644
--- a/remoting/webapp/browser_test/bump_scroll_browser_test.js
+++ b/remoting/webapp/browser_test/bump_scroll_browser_test.js
@@ -13,21 +13,30 @@
 'use strict';
 
 /** @constructor */
-browserTest.FakeDesktopConnectedView = function() {
+browserTest.FakeDesktopViewport = function() {
   this.pluginPosition = {
     top: 0,
     left: 0
   };
-  this.defineEvents(Object.keys(remoting.DesktopConnectedView.Events));
+  this.bumpScroller = new base.EventSourceImpl();
+  this.bumpScroller.defineEvents(Object.keys(remoting.BumpScroller.Events));
 };
 
-base.extend(browserTest.FakeDesktopConnectedView, base.EventSourceImpl);
-
-browserTest.FakeDesktopConnectedView.prototype.getPluginPositionForTesting =
+browserTest.FakeDesktopViewport.prototype.getPluginPositionForTesting =
     function() {
   return this.pluginPosition;
 };
 
+browserTest.FakeDesktopViewport.prototype.getBumpScrollerForTesting =
+    function() {
+  return this.bumpScroller;
+};
+
+browserTest.FakeDesktopViewport.prototype.raiseEvent =
+    function() {
+  return this.bumpScroller.raiseEvent.apply(this.bumpScroller, arguments);
+};
+
 
 /** @constructor */
 browserTest.Bump_Scroll = function() {
@@ -79,18 +88,20 @@ browserTest.Bump_Scroll.prototype.run = function(data) {
 };
 
 browserTest.Bump_Scroll.prototype.noScrollWindowed = function() {
-  remoting.desktopConnectedView.pluginWidthForBumpScrollTesting =
+  var viewport = remoting.desktopConnectedView.getViewportForTesting();
+  viewport.pluginWidthForBumpScrollTesting =
       window.innerWidth + this.kHostDesktopSizeDelta;
-  remoting.desktopConnectedView.pluginHeightForBumpScrollTesting =
+  viewport.pluginHeightForBumpScrollTesting =
       window.innerHeight + this.kHostDesktopSizeDelta;
   this.moveMouseTo(0, 0);
   return this.verifyScroll(undefined, undefined);
 };
 
 browserTest.Bump_Scroll.prototype.noScrollSmaller = function() {
-  remoting.desktopConnectedView.pluginWidthForBumpScrollTesting =
+  var viewport = remoting.desktopConnectedView.getViewportForTesting();
+  viewport.pluginWidthForBumpScrollTesting =
       window.innerWidth - this.kHostDesktopSizeDelta;
-  remoting.desktopConnectedView.pluginHeightForBumpScrollTesting =
+  viewport.pluginHeightForBumpScrollTesting =
       window.innerHeight - this.kHostDesktopSizeDelta;
   this.moveMouseTo(0, 0);
   return this.verifyScroll(undefined, undefined);
@@ -98,9 +109,10 @@ browserTest.Bump_Scroll.prototype.noScrollSmaller = function() {
 
 browserTest.Bump_Scroll.prototype.scrollDirection =
     function(widthFraction, heightFraction) {
-  remoting.desktopConnectedView.pluginWidthForBumpScrollTesting =
+  var viewport = remoting.desktopConnectedView.getViewportForTesting();
+  viewport.pluginWidthForBumpScrollTesting =
       screen.width + this.kHostDesktopSizeDelta;
-  remoting.desktopConnectedView.pluginHeightForBumpScrollTesting =
+  viewport.pluginHeightForBumpScrollTesting =
       screen.height + this.kHostDesktopSizeDelta;
   var expectedTop = heightFraction == 0.0 ? 0 :
                     heightFraction == 1.0 ? -this.kHostDesktopSizeDelta :
@@ -156,52 +168,52 @@ browserTest.Bump_Scroll.prototype.moveMouseTo = function(x, y) {
 
 // verifyScroll is complicated enough to warrant a test
 browserTest.Bump_Scroll.prototype.testVerifyScroll = function() {
-  var STARTED = remoting.DesktopConnectedView.Events.bumpScrollStarted;
-  var STOPPED = remoting.DesktopConnectedView.Events.bumpScrollStopped;
-  var fakeSession = new browserTest.FakeDesktopConnectedView;
+  var STARTED = remoting.BumpScroller.Events.bumpScrollStarted;
+  var STOPPED = remoting.BumpScroller.Events.bumpScrollStopped;
+  var fakeViewport = new browserTest.FakeDesktopViewport;
   var that = this;
 
   // No events raised (e.g. windowed mode).
-  var result = this.verifyScroll(undefined, undefined, fakeSession)
+  var result = this.verifyScroll(undefined, undefined, fakeViewport)
 
   .then(function() {
     // Start and end events raised, but no scrolling (e.g. full-screen mode
     // with host desktop <= window size).
-    fakeSession = new browserTest.FakeDesktopConnectedView;
-    var result = that.verifyScroll(undefined, undefined, fakeSession);
-    fakeSession.raiseEvent(STARTED, {});
-    fakeSession.raiseEvent(STOPPED, {});
+    fakeViewport = new browserTest.FakeDesktopViewport;
+    var result = that.verifyScroll(undefined, undefined, fakeViewport);
+    fakeViewport.raiseEvent(STARTED, {});
+    fakeViewport.raiseEvent(STOPPED, {});
     return result;
 
   }).then(function() {
     // Start and end events raised, with incorrect scrolling.
-    fakeSession = new browserTest.FakeDesktopConnectedView;
+    fakeViewport = new browserTest.FakeDesktopViewport;
     var result = base.Promise.negate(
-        that.verifyScroll(2, 2, fakeSession));
-    fakeSession.raiseEvent(STARTED, {});
-    fakeSession.pluginPosition.top = 1;
-    fakeSession.pluginPosition.left = 1;
-    fakeSession.raiseEvent(STOPPED, {});
+        that.verifyScroll(2, 2, fakeViewport));
+    fakeViewport.raiseEvent(STARTED, {});
+    fakeViewport.pluginPosition.top = 1;
+    fakeViewport.pluginPosition.left = 1;
+    fakeViewport.raiseEvent(STOPPED, {});
     return result;
 
   }).then(function() {
     // Start event raised, but not end event.
-    fakeSession = new browserTest.FakeDesktopConnectedView;
+    fakeViewport = new browserTest.FakeDesktopViewport;
     var result = base.Promise.negate(
-        that.verifyScroll(2, 2, fakeSession));
-    fakeSession.raiseEvent(STARTED, {});
-    fakeSession.pluginPosition.top = 2;
-    fakeSession.pluginPosition.left = 2;
+        that.verifyScroll(2, 2, fakeViewport));
+    fakeViewport.raiseEvent(STARTED, {});
+    fakeViewport.pluginPosition.top = 2;
+    fakeViewport.pluginPosition.left = 2;
     return result;
 
   }).then(function() {
     // Start and end events raised, with correct scrolling.
-    fakeSession = new browserTest.FakeDesktopConnectedView;
-    var result = that.verifyScroll(2, 2, fakeSession);
-    fakeSession.raiseEvent(STARTED, {});
-    fakeSession.pluginPosition.top = 2;
-    fakeSession.pluginPosition.left = 2;
-    fakeSession.raiseEvent(STOPPED, {});
+    fakeViewport = new browserTest.FakeDesktopViewport;
+    var result = that.verifyScroll(2, 2, fakeViewport);
+    fakeViewport.raiseEvent(STARTED, {});
+    fakeViewport.pluginPosition.top = 2;
+    fakeViewport.pluginPosition.left = 2;
+    fakeViewport.raiseEvent(STOPPED, {});
     return result;
   });
 
@@ -215,23 +227,23 @@ browserTest.Bump_Scroll.prototype.testVerifyScroll = function() {
  *    plugin, or undefined if it is not expected to change.
  * @param {number|undefined} expectedLeft The expected horizontal position of
  *    the plugin, or undefined if it is not expected to change.
- * @param {browserTest.FakeDesktopConnectedView=} opt_desktopConnectedView
- *     DesktopConnectedView fake, for testing.
+ * @param {browserTest.FakeDesktopViewport=} opt_desktopViewport
+ *     DesktopViewport fake, for testing.
  */
 browserTest.Bump_Scroll.prototype.verifyScroll =
-    function (expectedTop, expectedLeft, opt_desktopConnectedView) {
-  var desktopConnectedView = opt_desktopConnectedView ||
-      remoting.desktopConnectedView;
-  base.debug.assert(desktopConnectedView != null);
-  var STARTED = remoting.DesktopConnectedView.Events.bumpScrollStarted;
-  var STOPPED = remoting.DesktopConnectedView.Events.bumpScrollStopped;
+    function (expectedTop, expectedLeft, opt_desktopViewport) {
+  var desktopViewport = opt_desktopViewport ||
+      remoting.desktopConnectedView.getViewportForTesting();
+  base.debug.assert(desktopViewport != null);
+  var STARTED = remoting.BumpScroller.Events.bumpScrollStarted;
+  var STOPPED = remoting.BumpScroller.Events.bumpScrollStopped;
 
-  var initialPosition = desktopConnectedView.getPluginPositionForTesting();
+  var initialPosition = desktopViewport.getPluginPositionForTesting();
   var initialTop = initialPosition.top;
   var initialLeft = initialPosition.left;
 
   var verifyPluginPosition = function() {
-    var position = desktopConnectedView.getPluginPositionForTesting();
+    var position = desktopViewport.getPluginPositionForTesting();
     if (expectedLeft === undefined) {
       expectedLeft = initialLeft;
     }
@@ -248,16 +260,15 @@ browserTest.Bump_Scroll.prototype.verifyScroll =
     }
   };
 
-  var started = browserTest.expectEvent(desktopConnectedView, STARTED, 1000);
-  var stopped = browserTest.expectEvent(desktopConnectedView, STOPPED, 5000);
-  return started.then(function() {
-    return stopped.then(function() {
-      return verifyPluginPosition();
-    });
+  var bumpScroller = desktopViewport.getBumpScrollerForTesting();
+  var started = browserTest.expectEvent(bumpScroller, STARTED, 1000);
+  var stopped = browserTest.expectEvent(bumpScroller, STOPPED, 5000);
+  return Promise.all([started, stopped]).then(function() {

[Jamie, 2015/02/18 00:33:10]

Promise.all is not order-dependent, right? So I think this change introduces the
possibility of false-positive if STOPPED fires before STARTED. Since it doesn't
simplify the code significantly, I'd prefer to leave it as it was.

[kelvinp, 2015/02/20 23:24:16]

Done.

+    return verifyPluginPosition();
   }, function() {
     // If no started event is raised, the test might still pass if it asserted
     // no scrolling.
-    if (expectedTop == undefined && expectedLeft == undefined) {
+    if (expectedTop === undefined && expectedLeft === undefined) {
       return Promise.resolve();
     } else {
       return Promise.reject(