Audio player: Throttle concurrent loading of audio metadata.

ui/file_manager/audio_player/js/audio_player.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.
 
 /**
  * Overrided metadata worker's path.
  * @type {string}
  */
 ContentMetadataProvider.WORKER_SCRIPT = '/js/metadata_worker.js';
 
@@ skipping 29 matching lines @@
    * @type {?boolean}
    * @private
    */
   // Initial value is null. It'll be set in load().
   this.isTrackInfoExpanded_ = null;
 
   this.player_ =
     /** @type {AudioPlayerElement} */ (document.querySelector('audio-player'));
   this.player_.tracks = [];
 
+  /**
+   * Queue to throttle concurrent reading of audio file metadata.
+   * @private {AsyncUtil.ConcurrentQueue}
+   */
+  this.loadMetadataQueue_ = new AsyncUtil.ConcurrentQueue(25);

[oka, 2016/10/28 07:10:18]

Could you explain why 25 is chosen?

[fukino, 2016/10/28 09:31:51]

I elaborated it a bit more.

+
   // Restore the saved state from local storage, and update the local storage
   // if the states are changed.
   var STORAGE_PREFIX = 'audioplayer-';
   var KEYS_TO_SAVE_STATES =
       ['shuffle',
        'repeat-mode',
        'volume',
        'playlist-expanded',
        'track-info-expanded'];
   var storageKeys = KEYS_TO_SAVE_STATES.map(a => STORAGE_PREFIX + a);
@@ skipping 149 matching lines @@
     }.bind(this));
   }.bind(this));
 };
 
 /**
  * Loads metadata for a track.
  * @param {number} track Track number.
  * @private
  */
 AudioPlayer.prototype.loadMetadata_ = function(track) {
-  this.fetchMetadata_(
-      this.entries_[track], this.displayMetadata_.bind(this, track));
+  this.loadMetadataQueue_.run(function(trackIndex, callback) {

[oka, 2016/10/28 07:10:18]

fetchMetadata_ looks like more appropriate place to have throttle.

[fukino, 2016/10/28 09:31:51]

I think loadMetadata_ is better.
We want to throttle the initial bulk metadata load, but when a user changed the
current track, loading metadata for the current track should be done before we
completely finish the bulk load.
(For example, when there are 1000 tracks in a directory and the user click
"Prev" button after the audio player is opened, the last song will be selected
and we want to show the metadata asap.)

+    this.fetchMetadata_(this.entries_[trackIndex], function(metadata) {
+      this.displayMetadata_(trackIndex, metadata);
+      callback();
+    }.bind(this));
+  }.bind(this, track));

[oka, 2016/10/28 07:13:43]

I wonder if binding |track| is needed.
Can't you use |track| from the closure?

[fukino, 2016/10/28 09:31:51]

You're right. Thanks!

 };
 
 /**
  * Displays track's metadata.
  * @param {number} track Track number.
  * @param {Object} metadata Metadata object.
  * @param {string=} opt_error Error message.
  * @private
  */
 AudioPlayer.prototype.displayMetadata_ = function(track, metadata, opt_error) {
@@ skipping 377 matching lines @@
   // TODO(yoshiki): Handle error in better way.
   this.title = metadata.mediaTitle || this.getDefaultTitle();
   this.artist = error || metadata.mediaArtist || this.getDefaultArtist();
   this.artworkUrl = metadata.contentThumbnailUrl || "";
 };
 
 // Starts loading the audio player.
 window.addEventListener('DOMContentLoaded', function(e) {
   AudioPlayer.load();
 });