[Files.app] Initial implementation of new audio player

chrome/browser/resources/file_manager/background/js/background.js

Index: chrome/browser/resources/file_manager/background/js/background.js
diff --git a/chrome/browser/resources/file_manager/background/js/background.js b/chrome/browser/resources/file_manager/background/js/background.js
index 94e850e3bf99bafc8b34d45769f9f7f0acc5f346..5bdbac71f61e77b2d172cbfeff3076474a689f60 100644
--- a/chrome/browser/resources/file_manager/background/js/background.js
+++ b/chrome/browser/resources/file_manager/background/js/background.js
@@ -666,29 +666,42 @@ Background.prototype.onExecute_ = function(action, details) {
   }
 };
 
-/**
- * Audio player window create options.
- * @type {Object}
- * @const
- */
-var AUDIO_PLAYER_CREATE_OPTIONS = Object.freeze({
-  type: 'panel',
-  hidden: true,
-  minHeight: 35 + 58,
-  minWidth: 280,
-  height: 35 + 58,
-  width: 280
-});
+var audioPlayer = null;
+var audioPlayerLaunchHandler = new AsyncUtil.Group();

[mtomasz, 2014/01/23 08:05:09]

Please add comments, what's the purpose of it.

[yoshiki, 2014/01/24 15:34:19]

Added a comment and changed the name of the variable. It prevents the race.

 
-var audioPlayer = new SingletonAppWindowWrapper('mediaplayer.html',
-                                                AUDIO_PLAYER_CREATE_OPTIONS);
+chrome.commandLinePrivate.hasSwitch(

[mtomasz, 2014/01/23 08:05:09]

I think this is racy. launchAudioPlayer() may be called, before hasSwitch
finishes.

[yoshiki, 2014/01/24 15:34:19]

It won't be happened. In the case that the 'audioPlayer' object is not
initializad, the code is stored into 'audioPlayerLaunchHandler' as a closure
instead.

+    'enable-new-audio-player', function(newAudioPlayerEnabled) {
+  var audioPlayerHTML =
+      newAudioPlayerEnabled ? 'audio_player.html' : 'mediaplayer.html';
+
+  /**
+   * Audio player window create options.
+   * @type {Object}
+   */
+  var audioPlayerCreateOptions = Object.freeze({
+    type: 'panel',
+    hidden: true,
+    minHeight: newAudioPlayerEnabled ? 116 : (35 + 58),
+    minWidth: newAudioPlayerEnabled ? 292 : 280,
+    height: newAudioPlayerEnabled ? 356 : (35 + 58),
+    width: newAudioPlayerEnabled ? 292 : 280,
+  });
+
+  audioPlayer = new SingletonAppWindowWrapper(audioPlayerHTML,
+                                              audioPlayerCreateOptions);
+  audioPlayerLaunchHandler.run();

[mtomasz, 2014/01/23 08:05:09]

Please add comments, this is not easy to understand.

+  audioPlayerLaunchHandler = null;
+});
 
 /**
  * Launch the audio player.
  * @param {Object} playlist Playlist.
  */
 function launchAudioPlayer(playlist) {
-  audioPlayer.launch(playlist);
+  if (audioPlayer)
+    audioPlayer.launch(playlist);
+  else
+    audioPlayerLaunchHandler.add(launchAudioPlayer.bind(null, playlist));

[mtomasz, 2014/01/23 08:05:09]

What's the purpose of this group? Is it needed?

 }
 
 var videoPlayer = new SingletonAppWindowWrapper('video_player.html',
@@ -745,8 +758,13 @@ Background.prototype.onRestarted_ = function() {
     }
   });
 
-  // Reopen sub-applications.
-  audioPlayer.reopen();
+  // Reopen audio player.
+  if (audioPlayer)
+    audioPlayer.reopen();
+  else
+    audioPlayerLaunchHandler.add(function() { audioPlayer.reopen(); });
+
+  // Reopen video player.
   videoPlayer.reopen();
 };