Refactor to avoid potential race condition.

I haven't seen this happen, but...

It could be possible for the iframe's contents to load before this
callback is called (on nextTick), in which case the "load" callback
wouldn't be called.  So we delay setting the iframe's source until
nextTick has elapsed.

2 files changed, 21 insertions, 14 deletions

diff --git a/content_scripts/ui_component.coffee b/content_scripts/ui_component.coffee
index 24982fa7..4e26f26a 100644
--- a/content_scripts/ui_component.coffee
+++ b/content_scripts/ui_component.coffee
@@ -15,7 +15,6 @@ class UIComponent
     extend @iframeElement,
       className: className
       seamless: "seamless"
-      src: chrome.runtime.getURL iframeUrl
     shadowWrapper = document.createElement "div"
     # PhantomJS doesn't support createShadowRoot, so guard against its non-existance.
     @shadowDOM = shadowWrapper.createShadowRoot?() ? shadowWrapper
@@ -28,9 +27,12 @@ class UIComponent
     @hide false
 
     # Open a port and pass it to the iframe via window.postMessage.  We use an AsyncDataFetcher to handle
-    # requests which arrive before the frame (and its message handlers) have completed initialization.  See
+    # requests which arrive before the iframe (and its message handlers) have completed initialization.  See
     # #1679.
     @iframePort = new AsyncDataFetcher (setIframePort) =>
+      # We set the iframe source here (and not above) to avoid a potential race condition vis-a-vis the "load"
+      # event (because this callback runs on "nextTick").
+      @iframeElement.src = chrome.runtime.getURL iframeUrl
       @iframeElement.addEventListener "load", =>
         # Get vimiumSecret so the iframe can determine that our message isn't the page impersonating us.
         chrome.storage.local.get "vimiumSecret", ({ vimiumSecret }) =>
diff --git a/tests/dom_tests/phantom_runner.coffee b/tests/dom_tests/phantom_runner.coffee
index 93218724..e0382a35 100644
--- a/tests/dom_tests/phantom_runner.coffee
+++ b/tests/dom_tests/phantom_runner.coffee
@@ -37,15 +37,20 @@ page.open testfile, (status) ->
     console.log 'Unable to load tests.'
     phantom.exit 1
 
-  testsFailed = page.evaluate ->
-    Tests.run()
-    return Tests.testsFailed
-
-  if system.args[1] == '--coverage'
-    data = page.evaluate -> JSON.stringify _$jscoverage
-    fs.write dirname + 'dom_tests_coverage.json', data, 'w'
-
-  if testsFailed > 0
-    phantom.exit 1
-  else
-    phantom.exit 0
+  runTests = ->
+    testsFailed = page.evaluate ->
+      Tests.run()
+      return Tests.testsFailed
+
+    if system.args[1] == '--coverage'
+      data = page.evaluate -> JSON.stringify _$jscoverage
+      fs.write dirname + 'dom_tests_coverage.json', data, 'w'
+
+    if testsFailed > 0
+      phantom.exit 1
+    else
+      phantom.exit 0
+
+  # We add a short delay to allow asynchronous initialization (that is, initialization which happens on
+  # "nextTick") to complete.
+  setTimeout runTests, 10