Major revision of page cycler UI.

chrome/common/extensions/docs/examples/apps/cycler/cycler.js

Index: chrome/common/extensions/docs/examples/apps/cycler/cycler.js
diff --git a/chrome/common/extensions/docs/examples/apps/cycler/cycler.js b/chrome/common/extensions/docs/examples/apps/cycler/cycler.js
new file mode 100644
index 0000000000000000000000000000000000000000..1a57bbddb31a446b28f70885d43662235e765768
--- /dev/null
+++ b/chrome/common/extensions/docs/examples/apps/cycler/cycler.js
@@ -0,0 +1,383 @@
+// 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.
+
+function $(criterion) {
+  return document.querySelector(criterion);
+}
+
+var cyclerUI = new (function () {

[Jeffrey Yasskin, 2012/08/09 19:27:04]

Do you ever use 'cyclerUI'? I'm not a JS expert, but would it be cleaner to just
use a closure with local variables instead of an object?

[clintstaley, 2012/08/10 00:33:13]

I'm fairly new to it myself, and am just imitating what I've seen in other
cases.  The style I've seen is to use closure-locals where "static" data would
be normal, and to use prototype and normal objects for what would be "member"
data.

+
+  /**
+   * Note to reviewers: This is a (perhaps excessively?) fancy

[Jeffrey Yasskin, 2012/08/09 19:27:04]

Grammar-o: "This is a fancy these functions into ..."?

[clintstaley, 2012/08/10 00:33:13]

Done.

+   * these functions into the cyclerUI object wouldn't
+   * group them so nicely, but would also avoid a lot of 'bind' calls. I'm
+   * still sorting out JS design, and would value your opinion on this.

[Jeffrey Yasskin, 2012/08/09 19:27:04]

I'm not at all an expert on JS design, but it seems wrong to have a bunch of
functions where "this" needs to be re-bound to refer to something other than the
containing object. I think you should either have the functions take an argument
or make them methods of cyclerUI.

[clintstaley, 2012/08/10 00:33:13]

Yeah, this is what I was worried about, too.  If you don't mind, I'd like to
ping Aaron on this one, since I hear he's done a bit of JS in his time :).  My
$20 says he'll agree with you, but a side bet says he'll say "Oh, Clint's enum
is a dumb way to do that, instead try <insert clever third solution neither of
us have considered>".

+   *
+   * Enum for different UI states, showing variously a capture page,
+   * a page for choosing a playback, one for showing an already-chosen
+   * playback, and one for indicating that no playback choices yet exist.
+   * @enum {{set: !function()}}
+   * @private

[Jeffrey Yasskin, 2012/08/09 19:27:04]

I think being a local variable in the object closure makes this automatically
private. Also, wouldn't this have to go through the Closure Compiler to make
@private meaningful?

[clintstaley, 2012/08/10 00:33:13]

It surely does.  I was thinking of the @private as just documentation, and
adding it to be thorough.  Can drop it if you want.  Got dinged in earlier CLs
for not putting all the dots and crosses in the JSDocs, tho.

+   */
+  var EnableState_ = {
+    capture: {
+      set:
+        function() {
+          if (this.enableState != EnableState_.capture) {
+            this.enableState = EnableState_.capture;
+            this.enableTab_(this.captureTabLabel, this.captureTab);
+            this.captureTab.hidden = false;
+            this.playbackTab.hidden = true;
+          }
+        }
+    },
+    choosePlayback: {
+      set:
+        function() {
+          if (this.enableState != EnableState_.choosePlayback) {
+            this.enableState = EnableState_.choosePlayback;
+            this.enableTab_(this.playbackTabLabel, this.playbackTab);
+            this.captureTab.hidden = true;
+            this.playbackTab.hidden = false;
+            this.noCaptures.hidden = true;
+            this.yesCaptures.hidden = false;
+            this.playbackDetails.hidden = true;
+          }
+        }
+    },
+    showPlayback: {
+      set:
+        function() {
+          if (this.enableState != EnableState_.showPlayback) {
+            this.enableState = EnableState_.showPlayback;
+            this.enableTab_(this.playbackTabLabel, this.playbackTab);
+            this.captureTab.hidden = true;
+            this.playbackTab.hidden = false;
+            this.noCaptures.hidden = true;
+            this.yesCaptures.hidden = false;
+            this.playbackDetails.hidden = false;
+          }
+        }
+    },
+    showNoCaptures: {
+      set:
+        function() {
+          if (this.enableState != EnableState_.showNoCaptures) {
+            this.enableState = EnableState_.showNoCaptures;
+            this.enableTab_(this.playbackTabLabel, this.playbackTab);
+            this.captureTab.hidden = true;
+            this.playbackTab.hidden = false;
+            this.noCaptures.hidden = false;
+            this.yesCaptures.hidden = true;
+          }
+        }
+    }
+  };
+
+  // Members for all UI elements subject to programmatic adjustment.
+  this.captureTab = $('#capture-tab');
+  this.captureTabLabel = $('#capture-tab-label');
+  this.captureName = $('#capture-name');
+  this.captureURLs = $('#capture-urls');
+  this.doCaptureButton = $('#do-capture');
+
+  this.playbackTab = $('#playback-tab');
+  this.noCaptures = $('#no-captures');
+  this.yesCaptures = $('#yes-captures');
+
+  this.playbackTabLabel = $('#playback-tab-label');
+  this.playbackName = $('#playback-name');
+  this.playbackDetails = $('#playback-details');
+  this.playbackURLs = $('#playback-urls');
+  this.playbackRepeats = $('#playback-repeats');
+  this.playbackExtension = $('#playback-extension');
+  this.doPlaybackButton = $('#do-playback');
+  this.doDeleteButton = $('#do-delete');
+
+  this.popupDialog = $('#popup');
+  this.popupContent = $('#popup-content');
+  this.doPopupDismiss = $('#do-popup-dismiss');
+
+  // currentCaptureName_ is most recently done capture, or most recently
+  // selected stored capture.
+  this.currentCaptureName_ = null;
+
+  /**
+   * Enable the tab indciated by |tabLabel| and |tab|, the HTML elements

[Jeffrey Yasskin, 2012/08/09 19:27:04]

sp: indciated

[clintstaley, 2012/08/10 00:33:13]

Done.

+   * showing the selection-label for the tab, and the tab itself, respectively.
+   * @param {!HTMLDivElement} tabLabel The <div> for the label of the
+   *     tab to enable.
+   * @param {!HTMLDivElement} tab The <div> for the tab itself.
+   * @private
+   */
+  this.enableTab_ = function(tabLabel, tab) {
+    var tabList = document.querySelectorAll('.tab');
+    var tabLabelList = document.querySelectorAll('.tab-label');
+
+    for (var i = 0; i < tabList.length; i++)
+      tabList[i].hidden = tabList[i] != tab;
+
+    for (var i = 0; i < tabLabelList.length; i++)
+      if (tabLabelList[i] == tabLabel) {
+        tabLabelList[i].classList.add('select');
+      } else {
+        tabLabelList[i].classList.remove('select');
+      }
+  }
+
+  /**
+   * Show an overlay with a message, a dismiss button with configurable
+   * label, and an action to call upon dismissal.
+   * @param {!string} content The message to display.
+   * @param {!string} dismissLabel The label on the dismiss button.
+   * @param {function()} action Additional action to take, if any, upon
+   *     dismissal.
+   * @private
+   */
+  this.showMessage_ = function(content, dismissLabel, action) {
+    this.popupContent.innerText = content;
+    this.doPopupDismiss.innerText = dismissLabel;
+    this.popupDialog.hidden = false;
+    if (action != null)
+      doPopupDismiss.addEventListener('click', action);
+  }
+
+  /**
+   * Default action for popup dismissal button, performed in addition to
+   * any other actions that may be specified in showMessage_ call.
+   * @private
+   */
+  this.clearMessage_ = function() {
+    this.popupDialog.hidden = true;
+  }
+
+  /* Mockup of capture saving system. Sets up a simple array of known
+   * captures, and adjusts that array. This code will all be replaced
+   * in the next CL.
+   */
+  this.currentCaptures = ['alpha', 'beta', 'gamma'];
+
+  /**
+   * Save a capture from a specified cache directory on the browser side,
+   * storing it in HTML side FileSystem storage. Update the capture choices
+   * HTML select element.
+   * @param {!string} name The name of the capture.
+   * @param {!Array.<!string>} urlList List of URLs to capture.
+   * @param {!string} directory The cache directory name.
+   */
+  this.saveCapture = function(name, urlList, directory) {
+    console.log('Saving capture ' + name + ' on ' + urlList +
+        ' from browserside directory ' + directory);
+    this.currentCaptures.push(name);
+  }
+
+  /**
+   * Return a list of currently stored captures in the local FileSystem.
+   * @return {Array.<!string>} Names of all the current captures.
+   */
+  this.getCaptures = function() {
+    return this.currentCaptures;
+  }
+
+  /**
+   * Delete capture |name| from the local FileSystem, and update the

[Jeffrey Yasskin, 2012/08/09 19:27:04]

Does this actually delete from the filesystem?

[clintstaley, 2012/08/10 00:33:13]

No, like the comment says above, it's a mockup for now.  Next revision will
include much new FileSystem stuff.

+   * capture choices HTML select element.
+   * @param {!string} name The name of the capture to delete.
+   */
+  this.deleteCapture = function(name) {
+    this.currentCaptures.splice(this.currentCaptures.indexOf(name), 1);
+    this.updatePlaybackChoices_();
+  }
+
+  /* End capture saving mockup */
+
+  /**
+   * Do a capture using current data from the capture tab. Post an error
+   * dialog if said data is incorrect or incomplete. Otherwise pass
+   * control to the browser side.
+   * @private
+   */
+  this.doCapture_ = function() {
+    var errors = [];
+    var captureName = this.captureName.value.trim();
+    var urlList;
+
+    urlList = this.captureURLs.value.split('\n');
+    if (captureName.length == 0)
+      errors.push('Must give a capture name');
+    if (urlList.length == 0)
+      errors.push('Must give at least one URL');
+
+    if (errors.length > 0) {
+      this.showMessage_(errors.join('\n'), 'Ok');
+    } else {
+      this.doCaptureButton.disabled = true;
+      chrome.experimental.record.captureURLs(captureName, urlList,

[Jeffrey Yasskin, 2012/08/09 19:27:04]

Can captureURLs throw exceptions that need to be exposed to the user somehow?

[clintstaley, 2012/08/10 00:33:13]

Not across an extension API boundary that I know of.  They instead show up in
"errors" parameters passed to the onCaptureDone callback.  Given it's an
asynchronous call, I'm not sure there is a way to do exception throwing.

But, if you're asking more generally about error returns, yes, things like bad
URLs, etc. are returned via |errors| and posted for the user to see.

+          this.onCaptureDone.bind(this));
+    }
+  }
+
+  /**
+   * Delete the capture with name |currentCaptureName_|. For this method
+   * to be callable, there must be a selected currentCaptureName_. Change
+   * the displayed HTML to show no captures if none exist now, or to show
+   * none selected (since we just deleted the selected one).
+   * @private
+   */
+  this.doDelete_ = function() {
+    this.deleteCapture(this.currentCaptureName_);
+    this.currentCaptureName_ = null;
+    this.updatePlaybackChoices_();
+    if (this.getCaptures().length == 0) {
+      EnableState_.showNoCaptures.set.bind(this)();
+    } else {
+      EnableState_.choosePlayback.set.bind(this)();
+    }
+  }
+
+  /**
+   * Callback for completed (or possibly failed) capture. Post a message
+   * box, either with errors or "Success!" message. Save the capture cache
+   * directory in the event of success.
+   * @param {!Array.<string>} errors List of errors that occured
+   *     during capture, if any.
+   * @param {!string} cacheDirectory Cache directory name for successful
+   *     capture.
+   * @private
+   */
+  this.onCaptureDone = function(errors, cacheDirectory) {
+    this.doCaptureButton.disabled = false;
+
+    if (errors.length > 0) {
+      this.showMessage_(errors.join('\n'), 'Ok');
+    } else {
+      this.showMessage_('Success!', 'Ok');
+      this.currentCaptureName_ = this.captureName.value.trim();
+      this.saveCapture(this.currentCaptureName_, cacheDirectory);
+      this.updatePlaybackChoices_();
+    }
+  }
+
+  /**
+   * Display the playback tab, showing either its "no captures", "choose
+   * a capture" or "display chosen capture" form, depending on the state
+   * of existing captures and |currentCaptureName_|.
+   * @private
+   */
+  this.displayPlayback = function() {
+    if (this.getCaptures().length == 0) {
+      EnableState_.showNoCaptures.set.bind(this)();
+    } else if (this.currentCaptureName_ == null) {
+      EnableState_.choosePlayback.set.bind(this)();
+    } else {
+      EnableState_.showPlayback.set.bind(this)();
+    }
+  }
+
+  /**
+   * Utility function to refresh the selection list of captures that may
+   * be chosen for playback. Show all current captures, and also a
+   * "Choose a capture" default text if no capture is currently selected.
+   * @private
+   */
+  this.updatePlaybackChoices_ = function() {
+    var captureNames = this.getCaptures();
+    var options = this.playbackName.options;
+    var nextIndex = 0;
+
+    options.length = 0;
+
+    if (this.currentCaptureName_ == null) {
+      options.add(new Option('Choose a capture', null));
+      options.selectedIndex = nextIndex++;
+    }
+    for (var i = 0; i < captureNames.length; i++) {
+      options.add(new Option(captureNames[i], captureNames[i]));
+      if (captureNames[i] == this.currentCaptureName_) {
+        options.selectedIndex = nextIndex;
+      }
+      nextIndex++;
+    }
+  }
+
+  /**
+   * Event callback for selection of a capture to play back. Save the
+   * choice in |currentCaptureName_|. Update the selection list because the
+   * default reminder message is no longer needed when a capture is chosen.
+   * Change playback tab to show-chosen-capture mode.
+   */
+  this.selectPlaybackName = function() {
+    this.currentCaptureName_ = this.playbackName.value;
+    this.updatePlaybackChoices_();
+    EnableState_.showPlayback.set.bind(this)();
+  }
+
+  /**
+   * Event callback for pressing the playback button, which button is only
+   * enabled if a capture has been chosen for playback. Check for errors
+   * on playback page, and either display a message with such errors, or
+   * call the extenion api to initiate a playback.
+   */
+  this.doPlayback_ = function() {
+    var extensionPath = this.playbackExtension.value;
+    var repeatCount = parseInt(this.playbackRepeats.value);
+    var errors = [];
+
+    // Check local errors
+    if (isNaN(repeatCount)) {
+      errors.push('Enter a number for repeat count');
+    } else if (repeatCount < 1 || repeatCount > 100) {
+      errors.push('Repeat count must be between 1 and 100');
+    }
+
+    if (errors.length > 0) {
+      this.showMessage_(errors.join('\n'), 'Ok');
+    } else {
+      this.doPlaybackButton.disabled = true;
+      chrome.experimental.record.replayURLs(
+          this.currentCaptureName_,
+          repeatCount,
+          {'extensionPath': extensionPath},
+          this.onPlaybackDone.bind(this));
+    }
+  }
+
+  /**
+   * Extension API calls this back when a playback is done.
+   * @param {!number} runtime Number of ms it took to run the playback.
+   * @param {!string} stats Statistics from the playback.
+   * @param {!Array.<!string>} errors List of error messages, if any,
+   *     resuting from the playback.
+   */
+  this.onPlaybackDone = function(runTime, stats, errors) {
+    this.doPlaybackButton.disabled = false;
+
+    if (errors.length > 0) {
+      this.showMessage_(errors.join('\n'), 'Ok');
+    } else {
+      this.showMessage_('Test took ' + runTime + 'mS :\n' + stats, 'Ok');

[Jeffrey Yasskin, 2012/08/09 19:27:04]

I understand this is just the initial version, but performance measurements
aren't very useful without a presentation of the inherent jitter and
uncertainty. Have you thought about whether the current API supports good
statistics on time measurements? Does it support a warmup phase and multiple
measurements of the same loads? Intentionally-cold starts done by flushing
caches? Etc?

[clintstaley, 2012/08/10 00:33:13]

The design of the C++ side of this is a whole nother story, but the short
version (longer one is in the cycler design docs) is that we are starting a
sub-browser instance, and running it entirely from a preloaded cache that was
set up by the initial capture run, so we don't have network latency variations
to pollute the timing.  This obviates any cache-clear testing.  The repeat count
option is meant to "average out" any computational timing variations, though how
well it does that is worth discussing.  Getting a more polished set of stats,
even to the point of nice diagrams, is part of this new effort, just not part of
the current CL :)

+    }
+  }
+
+  // Set up listeners on all buttons.
+  this.doCaptureButton.addEventListener('click', this.doCapture_.bind(this));
+  this.doPlaybackButton.addEventListener('click', this.doPlayback_.bind(this));
+  this.doDeleteButton.addEventListener('click', this.doDelete_.bind(this));
+  this.doPopupDismiss.addEventListener('click', this.clearMessage_.bind(this));
+
+  // Set up listeners on tab labels.
+  this.captureTabLabel.addEventListener('click',
+      EnableState_.capture.set.bind(this));
+  this.playbackTabLabel.addEventListener('click',
+      this.displayPlayback.bind(this));
+
+  // Set up initial selection list for existing captures, and selection
+  // event listener.
+  this.updatePlaybackChoices_();
+  this.playbackName.addEventListener('change',
+      this.selectPlaybackName.bind(this));
+
+  // Start with capture tab displayed.
+  EnableState_.capture.set.bind(this)();
+})();