[Video Player] Re-factoring the code

ui/file_manager/video_player/js/video_player.js

Index: ui/file_manager/video_player/js/video_player.js
diff --git a/ui/file_manager/video_player/js/video_player.js b/ui/file_manager/video_player/js/video_player.js
index f472cdff9a133e10f74c335c6a7eba52f8ce4dea..7a0363ce0eb94be1cd0854b72ec7490bb19d7231 100644
--- a/ui/file_manager/video_player/js/video_player.js
+++ b/ui/file_manager/video_player/js/video_player.js
@@ -5,41 +5,6 @@
 'use strict';
 
 /**
- * Displays error message.
- * @param {string} message Message id.
- */
-function showErrorMessage(message) {
-  var errorBanner = document.querySelector('#error');
-  errorBanner.textContent =
-      loadTimeData.getString(message);
-  errorBanner.setAttribute('visible', 'true');
-
-  // The window is hidden if the video has not loaded yet.
-  chrome.app.window.current().show();
-}
-
-/**
- * Handles playback (decoder) errors.
- */
-function onPlaybackError() {
-  showErrorMessage('GALLERY_VIDEO_DECODING_ERROR');
-  decodeErrorOccured = true;
-
-  // Disable inactivity watcher, and disable the ui, by hiding tools manually.
-  controls.inactivityWatcher.disabled = true;
-  document.querySelector('#video-player').setAttribute('disabled', 'true');
-
-  // Detach the video element, since it may be unreliable and reset stored
-  // current playback time.
-  controls.cleanup();
-  controls.clearState();
-
-  // Avoid reusing a video element.
-  video.parentNode.removeChild(video);
-  video = null;
-}
-
-/**
  * @param {Element} playerContainer Main container.
  * @param {Element} videoContainer Container for the video element.
  * @param {Element} controlsContainer Container for video controls.
@@ -49,16 +14,16 @@ function FullWindowVideoControls(
     playerContainer, videoContainer, controlsContainer) {
   VideoControls.call(this,
       controlsContainer,
-      onPlaybackError,
+      this.onPlaybackError_.wrap(this),
       loadTimeData.getString.wrap(loadTimeData),
       this.toggleFullScreen_.wrap(this),
       videoContainer);
 
   this.playerContainer_ = playerContainer;
+  this.decodeErrorOccured = false;
 
   this.updateStyle();
   window.addEventListener('resize', this.updateStyle.wrap(this));
-
   document.addEventListener('keydown', function(e) {
     if (e.keyIdentifier == 'U+0020') {  // Space
       this.togglePlayStateWithFeedback();
@@ -94,6 +59,40 @@ function FullWindowVideoControls(
 FullWindowVideoControls.prototype = { __proto__: VideoControls.prototype };
 
 /**
+ * Displays error message.
+ * @param {string} message Message id.

[mtomasz, 2014/05/29 13:44:12]

nit: Can we make it private?

[yoshiki, 2014/05/29 14:45:19]

Done.

+ */
+FullWindowVideoControls.prototype.showErrorMessage = function(message) {
+  var errorBanner = document.querySelector('#error');
+  errorBanner.textContent =
+      loadTimeData.getString(message);
+  errorBanner.setAttribute('visible', 'true');
+
+  // The window is hidden if the video has not loaded yet.
+  chrome.app.window.current().show();
+}

[mtomasz, 2014/05/29 13:44:12]

nit: ; missing.

[yoshiki, 2014/05/29 14:45:19]

Done.

+
+/**
+ * Handles playback (decoder) errors.

[mtomasz, 2014/05/29 13:44:12]

nit: @private missing.

[yoshiki, 2014/05/29 14:45:19]

Done.

+ */
+FullWindowVideoControls.prototype.onPlaybackError_ = function() {
+  this.showErrorMessage('GALLERY_VIDEO_DECODING_ERROR');
+  this.decodeErrorOccured = true;
+
+  // Disable inactivity watcher, and disable the ui, by hiding tools manually.
+  this.inactivityWatcher.disabled = true;
+  document.querySelector('#video-player').setAttribute('disabled', 'true');
+
+  // Detach the video element, since it may be unreliable and reset stored
+  // current playback time.
+  this.cleanup();
+  this.clearState();
+
+  // Avoid reusing a video element.
+  player.unloadVideo();
+};
+
+/**
  * Toggles the full screen mode.
  * @private
  */
@@ -103,113 +102,193 @@ FullWindowVideoControls.prototype.toggleFullScreen_ = function() {
 };
 
 // TODO(mtomasz): Convert it to class members: crbug.com/171191.
-var decodeErrorOccured;
-var video;
-var controls;
+var player = new VideoPlayer();
 
 /**
- * Initializes the video player window.
+ * @constructor
  */
-function loadVideoPlayer() {
+function VideoPlayer() {
+  this.controls = null;

[mtomasz, 2014/05/29 13:44:12]

nit: Can we make them private?

[yoshiki, 2014/05/29 14:45:19]

Done.

+  this.videoElement = null;
+  this.videos = null;
+
+  Object.seal(this);
+}
+
+/**
+ * Initializes the video player window. This method must be called after DOM
+ * initialization.
+ * @param {Array.<Object.<string, Object>>} videos List of video to play.

[mtomasz, 2014/05/29 13:44:12]

nit: ... -> List of videos.

[yoshiki, 2014/05/29 14:45:19]

Done.

+ */
+VideoPlayer.prototype.prepare = function(videos) {
   document.ondragstart = function(e) { e.preventDefault() };
 
-  chrome.fileBrowserPrivate.getStrings(function(strings) {
-    loadTimeData.data = strings;
+  this.videos = videos;
 
-    var url = window.videoUrl;
-    document.title = window.videoTitle;
-
-    controls = new FullWindowVideoControls(
-        document.querySelector('#video-player'),
-        document.querySelector('#video-container'),
-        document.querySelector('#controls'));
-
-    var reloadVideo = function(e) {
-      if (decodeErrorOccured &&
-          // Ignore shortcut keys
-          !e.ctrlKey && !e.altKey && !e.shiftKey && !e.metaKey) {
-        loadVideo(url);
-        e.preventDefault();
-      }
-    };
-
-    loadVideo(url);
-    document.addEventListener('keydown', reloadVideo, true);
-    document.addEventListener('click', reloadVideo, true);
-  });
-}
+  this.controls = new FullWindowVideoControls(
+      document.querySelector('#video-player'),
+      document.querySelector('#video-container'),
+      document.querySelector('#controls'));
+
+  var reloadVideo = function(e) {
+    if (this.controls.decodeErrorOccured &&
+        // Ignore shortcut keys
+        !e.ctrlKey && !e.altKey && !e.shiftKey && !e.metaKey) {
+      this.playVideo_();
+      e.preventDefault();
+    }
+  }.wrap(this);
+
+  document.addEventListener('keydown', reloadVideo, true);
+  document.addEventListener('click', reloadVideo, true);
+};
 
 /**
  * Unloads the player.
  */
 function unload() {
-  if (!controls.getMedia())
+  if (!player.controls || !player.controls.getMedia())
     return;
 
-  controls.savePosition(true /* exiting */);
-  controls.cleanup();
+  player.controls.savePosition(true /* exiting */);
+  player.controls.cleanup();
 }
 
 /**
- * Reloads the player.
+ * Loads the video file.
  * @param {string} url URL of the video file.
+ * @param {string} title Title of the video file.
+ * @param {function(number, number)=} callback Callback

[mtomasz, 2014/05/29 13:44:12]

nit: callback -> opt_callback

[mtomasz, 2014/05/29 13:44:12]

nit: Callback -> Completion callback.

[yoshiki, 2014/05/29 14:45:19]

Done.

[yoshiki, 2014/05/29 14:45:19]

Done.

+ * @private
  */
-function loadVideo(url) {
+VideoPlayer.prototype.loadVideo_ = function(url, title, opt_callback) {
+  this.unloadVideo();
+
+  document.title = title;
+
   // Re-enable ui and hide error message if already displayed.
   document.querySelector('#video-player').removeAttribute('disabled');
   document.querySelector('#error').removeAttribute('visible');
-  controls.inactivityWatcher.disabled = false;
-  decodeErrorOccured = false;
+  this.controls.inactivityWatcher.disabled = false;
+  this.controls.decodeErrorOccured = false;
+
+  this.videoElement = document.createElement('video');
+  document.querySelector('#video-container').appendChild(this.videoElement);
+  this.controls.attachMedia(this.videoElement);
+
+  this.videoElement.src = url;
+  this.videoElement.load();
+  if (opt_callback)
+    this.videoElement.addEventListener('loadedmetadata', opt_callback);
+};
+
+/**
+ * Plays the video.
+ * @private
+ */
+VideoPlayer.prototype.playVideo_ = function() {
+  var currentVideo = this.videos[0];
+  this.loadVideo_(currentVideo.fileUrl,
+                  currentVideo.entry.name,
+                  this.onVideoReady_.wrap(this));
+};
 
+/**
+ * Unlaods the current video.

[mtomasz, 2014/05/29 13:44:12]

typo: Unloads.

[yoshiki, 2014/05/29 14:45:19]

Done.

+ * @private
+ */
+VideoPlayer.prototype.unloadVideo = function() {
   // Detach the previous video element, if exists.
-  if (video)
-    video.parentNode.removeChild(video);
-
-  video = document.createElement('video');
-  document.querySelector('#video-container').appendChild(video);
-  controls.attachMedia(video);
-
-  video.src = url;
-  video.load();
-  video.addEventListener('loadedmetadata', function() {
-    // TODO: chrome.app.window soon will be able to resize the content area.
-    // Until then use approximate title bar height.
-    var TITLE_HEIGHT = 33;
-
-    var aspect = video.videoWidth / video.videoHeight;
-    var newWidth = video.videoWidth;
-    var newHeight = video.videoHeight + TITLE_HEIGHT;
-
-    var shrinkX = newWidth / window.screen.availWidth;
-    var shrinkY = newHeight / window.screen.availHeight;
-    if (shrinkX > 1 || shrinkY > 1) {
-      if (shrinkY > shrinkX) {
-        newHeight = newHeight / shrinkY;
-        newWidth = (newHeight - TITLE_HEIGHT) * aspect;
-      } else {
-        newWidth = newWidth / shrinkX;
-        newHeight = newWidth / aspect + TITLE_HEIGHT;
-      }
+  if (this.videoElement)
+    this.videoElement.parentNode.removeChild(this.videoElement);
+  this.videoElement = null;
+};
+
+/**
+ * Called when the video is ready after starting to load.
+ * @private
+ */
+VideoPlayer.prototype.onVideoReady_ = function() {
+  // TODO: chrome.app.window soon will be able to resize the content area.

[mtomasz, 2014/05/29 13:44:12]

FYI: From M36, we can finally do that.

https://developer.chrome.com/apps/app_window#type-AppWindow -> innerBounds.

[yoshiki, 2014/05/29 14:45:19]

Thanks for guidance. I'll change it in a separated patch.

+  // Until then use approximate title bar height.
+  var TITLE_HEIGHT = 33;
+
+  var videoWidth = this.videoElement.videoWidth;
+  var videoHeight = this.videoElement.videoHeight;
+
+  var aspect = videoWidth / videoHeight;
+  var newWidth = videoWidth;
+  var newHeight = videoHeight + TITLE_HEIGHT;
+
+  var shrinkX = newWidth / window.screen.availWidth;
+  var shrinkY = newHeight / window.screen.availHeight;
+  if (shrinkX > 1 || shrinkY > 1) {
+    if (shrinkY > shrinkX) {
+      newHeight = newHeight / shrinkY;
+      newWidth = (newHeight - TITLE_HEIGHT) * aspect;
+    } else {
+      newWidth = newWidth / shrinkX;
+      newHeight = newWidth / aspect + TITLE_HEIGHT;
     }
+  }
 
-    var oldLeft = window.screenX;
-    var oldTop = window.screenY;
-    var oldWidth = window.outerWidth;
-    var oldHeight = window.outerHeight;
+  var oldLeft = window.screenX;
+  var oldTop = window.screenY;
+  var oldWidth = window.outerWidth;
+  var oldHeight = window.outerHeight;
 
-    if (!oldWidth && !oldHeight) {
-      oldLeft = window.screen.availWidth / 2;
-      oldTop = window.screen.availHeight / 2;
-    }
+  if (!oldWidth && !oldHeight) {
+    oldLeft = window.screen.availWidth / 2;
+    oldTop = window.screen.availHeight / 2;
+  }
+
+  var appWindow = chrome.app.window.current();
+  appWindow.resizeTo(newWidth, newHeight);
+  appWindow.moveTo(oldLeft - (newWidth - oldWidth) / 2,
+                   oldTop - (newHeight - oldHeight) / 2);
+  appWindow.show();
 
-    var appWindow = chrome.app.window.current();
-    appWindow.resizeTo(newWidth, newHeight);
-    appWindow.moveTo(oldLeft - (newWidth - oldWidth) / 2,
-                     oldTop - (newHeight - oldHeight) / 2);
-    appWindow.show();
+  this.videoElement.play();
+};
+
+/**
+ * Initialize the list of videos.
+ * @param {function(Array.<Object>)} callback Called with the video list when
+ *     it is ready.
+ **/
+function initVideos(callback) {
+  if (window.videos) {
+    var videos = window.videos;
+    window.videos = null;
+    callback(videos);
+    return;
+  }
+
+  chrome.runtime.onMessage.addListener(
+      function(request, sender, sendResponse) {
+        var videos = window.videos;
+        window.videos = null;
+        callback(videos);
+      });
+}
 
-    video.play();
+/**
+ * Initializes the strings.
+ * @param {function()} callback Called when the sting data is ready.
+ **/
+function initStrings(callback) {
+  chrome.fileBrowserPrivate.getStrings(function(strings) {
+    loadTimeData.data = strings;
+    callback();
   });
 }
 
-util.addPageLoadHandler(loadVideoPlayer);
+var initPhase1 = Promise.all([new Promise(initVideos.wrap(null)),

[mtomasz, 2014/05/29 13:44:12]

nit: initPhase1 and initPhase2 naming is not helpful. How about either:

1. Chaining it:
Promise.all(...).then()

2. Or renaming initPhase1 to initPromise, and removing initPhase2, since it is
not used?

[yoshiki, 2014/05/29 14:45:19]

Done #2. Thanks.

+                          new Promise(initStrings.wrap(null)),
+                          new Promise(util.addPageLoadHandler.wrap(null))]);
+
+var initPhase2 = initPhase1.then(function(results) {
+  var videos = results;
+  player.prepare(videos);
+  return new Promise(player.playVideo_.wrap(player));
+});