Bug 539908 - Random orange: TEST-UNEXPECTED-FAIL | test_videoAllowedInMail. add plan_for_message_display, debug helper, and improve docs v1. test-only. r=sid0.
authorAndrew Sutherland <asutherland@asutherland.org>
Mon, 18 Jan 2010 00:05:10 -0800
changeset 4705 3224404d8ed990f2ed1607a0282bc8112b661627
parent 4704 3a004d8c38fec49f348b664633b7ec58742f6641
child 4706 7f0ab878abe7496b9ca2eb5ee00af6d2843bef5c
push id3683
push userbugmail@asutherland.org
push dateMon, 18 Jan 2010 08:06:22 +0000
treeherdercomm-central@3224404d8ed9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssid0
bugs539908
Bug 539908 - Random orange: TEST-UNEXPECTED-FAIL | test_videoAllowedInMail. add plan_for_message_display, debug helper, and improve docs v1. test-only. r=sid0.
mail/base/content/messageDisplay.js
mail/test/mozmill/content-policy/test-video-content-policy.js
mail/test/mozmill/folder-display/test-message-window.js
mail/test/mozmill/shared-modules/test-folder-display-helpers.js
mail/test/mozmill/shared-modules/test-window-helpers.js
--- a/mail/base/content/messageDisplay.js
+++ b/mail/base/content/messageDisplay.js
@@ -113,16 +113,30 @@ MessageDisplayWidget.prototype = {
   //@}
 
   /**
    * @name FolderDisplayWidget Notifications
    * @private
    */
   //@{
 
+  /**
+   * Unit testing support variable that tracks whether a message load is in
+   *  process.  This is set to true when |onDisplayingMessage| is invoked and
+   *  cleared by invoking |clearDisplay| or when the message finishes streaming
+   *  and |messageLoaded| is set to true.
+   */
+  messageLoading: false,
+  /**
+   * Unit testing support variable that tracks whether there is currently a
+   *  fully displayed message.  This is cleared when |clearDisplay| is invoked
+   *  or we hear that a new message begins streaming via |onDisplayingMessage|.
+   */
+  messageLoaded: false,
+
   clearDisplay: function MessageDisplayWidget_clearDisplay() {
     this.displayedMessage = null;
     this.messageLoading = false;
     this.messageLoaded = false;
     ClearPendingReadTimer();
     ClearMessagePane();
   },
 
--- a/mail/test/mozmill/content-policy/test-video-content-policy.js
+++ b/mail/test/mozmill/content-policy/test-video-content-policy.js
@@ -189,16 +189,18 @@ function test_videoDeniedInReplyWindow()
     throw new Error("Video has not been blocked in reply window as expected.");
 
   windowHelper.close_window(replyWindow);
 }
 
 function test_videoAllowedInMail() {
   addMsgToFolderAndCheckNoRemoteVideo(folder);
 
+  plan_for_message_display(mc);
+
   // Click on the allow remote content button
   mc.click(new elib.ID(mozmill.getMail3PaneController().window.document, "remoteContentBarButton"));
 
   wait_for_message_display_completion(mc, true);
 
   if (mozmill.getMail3PaneController().window.content.document
              .getElementById("video1").networkState ==
       Components.interfaces.nsIDOMHTMLMediaElement.NETWORK_NO_SOURCE)
--- a/mail/test/mozmill/folder-display/test-message-window.js
+++ b/mail/test/mozmill/folder-display/test-message-window.js
@@ -77,27 +77,29 @@ function test_open_message_window() {
   assert_selected_and_displayed(msgc, curMessage);
 }
 
 /**
  * Use the "f" keyboard accelerator to navigate to the next message,
  * and verify that it is indeed loaded.
  */
 function test_navigate_to_next_message() {
+  plan_for_message_display(msgc);
   msgc.keypress(null, "f", {});
   wait_for_message_display_completion(msgc, true);
   assert_selected_and_displayed(msgc, 1);
 }
 
 /**
  * Delete a single message and verify the next message is loaded. This sets
  * us up for the next test, which is delete on a collapsed thread after
  * the previous message was deleted.
  */
 function test_delete_single_message() {
+  plan_for_message_display(msgc);
   press_delete(msgc);
   wait_for_message_display_completion(msgc, true);
   assert_selected_and_displayed(msgc, 1);
 }
 
 /**
  * Delete the current message, and verify that it only deletes
  * a single message, not the messages in the collapsed thread
--- a/mail/test/mozmill/shared-modules/test-folder-display-helpers.js
+++ b/mail/test/mozmill/shared-modules/test-folder-display-helpers.js
@@ -771,17 +771,17 @@ function assert_folder_mode(aMode) {
  */
 function assert_folder_child_in_view(aChild, aParent) {
   let actualParent = mc.folderTreeView.getParentOfFolder(aChild);
   if (actualParent != aParent)
     throw new Error("Folder " + aChild.URI + " should be the child of " +
                     (aParent && aParent.URI) +
                     ", but is actually the child of " +
                     (actualParent && actualParent.URI));
-      
+
 }
 
 /**
  * Assert that the given folder is in the current folder mode and is visible.
  *
  * @returns The index of the folder, if it is visible.
  */
 function assert_folder_visible(aFolder) {
@@ -1053,27 +1053,56 @@ function wait_for_all_messages_to_load(a
                               FAST_INTERVAL, aController.folderDisplay))
     throw new Error("Messages never finished loading.  Timed Out.");
   // the above may return immediately, meaning the event queue might not get a
   //  chance.  give it a chance now.
   aController.sleep(0);
 }
 
 /**
- * If  a message is in the process of loading, let it finish.  Otherwise we get
- *  horrible assertions like so:
+ * Call this before triggering a message display that you are going to wait for
+ *  using |wait_for_message_display_completion| where you are passing true for
+ *  the aLoadDemanded argument.  This ensures that if a message is already
+ *  displayed for the given controller that state is sufficiently cleaned up
+ *  so it doesn't trick us into thinking that there is no need to wait.
+ *
+ * @param [aController] optional controller, defaulting to |mc|.
+ */
+function plan_for_message_display(aController) {
+  if (aController === undefined)
+    aController = mc;
+  aController.messageDisplay.messageLoaded = false;
+}
+
+/**
+ * If a message is in the process of loading, let it finish; optionally, be sure
+ *  to wait for a load to happen (assuming |plan_for_message_display| is used.)
+ *
+ * This method is used defensively by a lot of other code in this file that is
+ *  realy not sure whether there might be a load in progress or not.  So by
+ *  default we only do something if there is obviously a message display in
+ *  progress.  Since some events may end up getting deferred due to script
+ *  blockers or the like, it is possible the event that triggers the display
+ *  may not have happened by the time you call this.  In that case, you should
+ *  1) pass true for aLoadDemanded and 2) invoke |plan_for_message_display|
+ *  before triggering the event that will induce a message display.  You do not
+ *  need to do #2 if you are opening a new message window and can assume that
+ *  this will be the first message ever displayed in the window.
+ *
+ * If we didn't use this method defensively, we would get horrible assertions
+ *  like so:
  * ###!!! ASSERTION: Overwriting an existing document channel!
  *
- * @param aController optional controller, defaulting to |mc|.
- * @param aLoadDemanded optional indication that we expect and demand that a
- *     message be loaded.  If you call us before the message loading is
- *     initiated, you will need to pass true for this so that we don't see
- *     that a load hasn't started and assume none is required.  Defaults to
- *     false.  This relies on aController.messageDisplay.messageLoaded to
- *     be reliable; make sure it is false when entering this function.
+ *
+ * @param [aController] optional controller, defaulting to |mc|.
+ * @param [aLoadDemanded=false] Should we require that we wait for a message to
+ *     be loaded?  You should use this in conjunction with
+ *     |plan_for_message_display| as per the documentation above.  If you do
+ *     not pass true and there is no message load in process, this method will
+ *     return immediately.
  */
 function wait_for_message_display_completion(aController, aLoadDemanded) {
   if (aController === undefined)
     aController = mc;
   let contentPane = aController.contentPane;
   let oldHref = null;
 
   // There are a couple possible states the universe can be in:
@@ -1083,18 +1112,20 @@ function wait_for_message_display_comple
   // 4) A message load should happen in the near future.
   //
   // We have nothing that needs to be done in cases 1 and 2.  Case 3 is pretty
   //  easy for us.  The question is differentiating between case 4 and (1, 2).
   //  We rely on MessageDisplayWidget.messageLoaded to differentiate this case
   //  for us.
   let isLoadedChecker = function() {
     // If a load is demanded, first require that MessageDisplayWidget think
-    //  that the message is loaded.  Because the notification is imperfect,
-    //  this will strictly happen before the URL finishes running.
+    //  that the message is loaded.  Because the notification that sets the flag
+    //  happens when the message reader code gets told about the last attachment,
+    //  this will actually happen before the URL is finished running.  Luckily
+    //  we have the code below to try and deal with that.
     if (aLoadDemanded && !aController.messageDisplay.messageLoaded)
       return false;
 
     let docShell = contentPane.docShell;
     if (!docShell)
       return false;
     let uri = docShell.currentURI;
     // the URL will tell us if it is running, saves us from potential error
--- a/mail/test/mozmill/shared-modules/test-window-helpers.js
+++ b/mail/test/mozmill/shared-modules/test-window-helpers.js
@@ -631,16 +631,35 @@ var AugmentEverybodyWith = {
       return null;
     },
     /**
      * Wraps a call to a() in an elib.Elem.
      */
     aid: function _get_anon_elementid(aId, aQuery) {
       return new elib.Elem(this.a(aId, aQuery));
     },
+
+    /**
+     * Debug helper that defers a click until the next event loop spin in order
+     *  to create situations that are hard to test in isolation.  In order to
+     *  fashion reliable failures, we currently use a 1-second delay to make
+     *  sure things get sufficiently gummed up.
+     * Only use this for locally reproducing tinderbox failures; do not commit
+     *  code that uses this!
+     *
+     * This gets its own method rather than a generic deferring wrapper so we
+     *  can strap debug on and because it's meant so you can easily just
+     *  prefix on 'defer_' and be done with it.
+     */
+    defer_click: function _augmented_defer_click(aWhatToClick) {
+      let dis = this;
+      dis.window.setTimeout(function() {
+                              dis.click(aWhatToClick);
+                            }, 1000);
+    },
   },
 };
 
 /**
  * Per-windowtype augmentations.  Please use the documentation and general
  *  example of mail:3pane as your example.
  */
 var PerWindowTypeAugmentations = {