Require documentReady for all UI components.

This replaces c01d7eea8675f9a7d84999777e8de72244d687b6.

All UI components require the document to be ready.  So, here the
constructor waits for DomUtils.documentReady().  The diff looks big, but
mainly it's a result of changes in indentation in the constructor and in
hide().

Also, hide() now uses @postMessage() to post a null message.  This
forces hide to use @iframePort.use(), which ensures that hide()s cannot
overtake activate()s.

This continues #2100.

3 files changed, 79 insertions, 80 deletions

diff --git a/content_scripts/hud.coffee b/content_scripts/hud.coffee
index 62bcf03f..7ad13aaf 100644
--- a/content_scripts/hud.coffee
+++ b/content_scripts/hud.coffee
@@ -13,10 +13,8 @@ HUD =
   # it doesn't sit on top of horizontal scrollbars like Chrome's HUD does.
 
   init: ->
-    unless @hudUI?
-      @hudUI = new UIComponent "pages/hud.html", "vimiumHUDFrame", ({data}) =>
-        this[data.name]? data
-      @tween = new Tween "iframe.vimiumHUDFrame.vimiumUIComponentVisible", @hudUI.shadowDOM
+    @hudUI ?= new UIComponent "pages/hud.html", "vimiumHUDFrame", ({data}) => this[data.name]? data
+    @tween ?= new Tween "iframe.vimiumHUDFrame.vimiumUIComponentVisible", @hudUI.shadowDOM
 
   showForDuration: (text, duration) ->
     @show(text)
@@ -46,13 +44,13 @@ HUD =
   # If :updateIndicator is truthy, then we also refresh the mode indicator.  The only time we don't update the
   # mode indicator, is when hide() is called for the mode indicator itself.
   hide: (immediate = false, updateIndicator = true) ->
-    return unless @hudUI?.uiComponentIsReady
-    clearTimeout(@_showForDurationTimerId)
-    @tween.stop()
-    if immediate
-      if updateIndicator then Mode.setIndicator() else @hudUI.hide()
-    else
-      @tween.fade 0, 150, => @hide true, updateIndicator
+    if @hudUI? and @tween?
+      clearTimeout @_showForDurationTimerId
+      @tween.stop()
+      if immediate
+        if updateIndicator then Mode.setIndicator() else @hudUI.hide()
+      else
+        @tween.fade 0, 150, => @hide true, updateIndicator
 
   hideFindMode: (data) ->
     @findMode.checkReturnToViewPort()
diff --git a/content_scripts/ui_component.coffee b/content_scripts/ui_component.coffee
index 0989bbc9..c7038f17 100644
--- a/content_scripts/ui_component.coffee
+++ b/content_scripts/ui_component.coffee
@@ -1,5 +1,4 @@
 class UIComponent
-  uiComponentIsReady: false
   iframeElement: null
   iframePort: null
   showing: false
@@ -13,64 +12,65 @@ class UIComponent
     @iframeElement.classList.add addClass
 
   constructor: (iframeUrl, className, @handleMessage) ->
-    styleSheet = DomUtils.createElement "style"
-    styleSheet.type = "text/css"
-    # Default to everything hidden while the stylesheet loads.
-    styleSheet.innerHTML = "iframe {display: none;}"
+    DomUtils.documentReady =>
+      styleSheet = DomUtils.createElement "style"
+      styleSheet.type = "text/css"
+      # Default to everything hidden while the stylesheet loads.
+      styleSheet.innerHTML = "iframe {display: none;}"
 
-    # Use an XMLHttpRequest, possibly via the background page, to fetch the stylesheet. This allows us to
-    # catch and recover from failures that we could not have caught when using CSS @include (eg. #1817).
-    UIComponent::styleSheetGetter ?= new AsyncDataFetcher @fetchFileContents "content_scripts/vimium.css"
-    @styleSheetGetter.use (styles) -> styleSheet.innerHTML = styles
+      # Use an XMLHttpRequest, possibly via the background page, to fetch the stylesheet. This allows us to
+      # catch and recover from failures that we could not have caught when using CSS @include (eg. #1817).
+      UIComponent::styleSheetGetter ?= new AsyncDataFetcher @fetchFileContents "content_scripts/vimium.css"
+      @styleSheetGetter.use (styles) -> styleSheet.innerHTML = styles
 
-    @iframeElement = DomUtils.createElement "iframe"
-    extend @iframeElement,
-      className: className
-      seamless: "seamless"
-    shadowWrapper = DomUtils.createElement "div"
-    # PhantomJS doesn't support createShadowRoot, so guard against its non-existance.
-    @shadowDOM = shadowWrapper.createShadowRoot?() ? shadowWrapper
-    @shadowDOM.appendChild styleSheet
-    @shadowDOM.appendChild @iframeElement
-    @toggleIframeElementClasses "vimiumUIComponentVisible", "vimiumUIComponentHidden"
+      @iframeElement = DomUtils.createElement "iframe"
+      extend @iframeElement,
+        className: className
+        seamless: "seamless"
+      shadowWrapper = DomUtils.createElement "div"
+      # PhantomJS doesn't support createShadowRoot, so guard against its non-existance.
+      @shadowDOM = shadowWrapper.createShadowRoot?() ? shadowWrapper
+      @shadowDOM.appendChild styleSheet
+      @shadowDOM.appendChild @iframeElement
+      @toggleIframeElementClasses "vimiumUIComponentVisible", "vimiumUIComponentHidden"
 
-    # Open a port and pass it to the iframe via window.postMessage.  We use an AsyncDataFetcher to handle
-    # requests which arrive before the iframe (and its message handlers) have completed initialization.  See
-    # #1679.
-    @iframePort = new AsyncDataFetcher (setIframePort) =>
-      # We set the iframe source and append the new element here (as opposed to 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
-      document.documentElement.appendChild shadowWrapper
+      # Open a port and pass it to the iframe via window.postMessage.  We use an AsyncDataFetcher to handle
+      # requests which arrive before the iframe (and its message handlers) have completed initialization.  See
+      # #1679.
+      @iframePort = new AsyncDataFetcher (setIframePort) =>
+        # We set the iframe source and append the new element here (as opposed to 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
+        document.documentElement.appendChild shadowWrapper
 
-      @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 }) =>
-          { port1, port2 } = new MessageChannel
-          @iframeElement.contentWindow.postMessage vimiumSecret, chrome.runtime.getURL(""), [ port2 ]
-          port1.onmessage = (event) =>
-            switch event?.data?.name ? event?.data
-              when "uiComponentIsReady"
-                # If any other frame receives the focus, then hide the UI component.
-                chrome.runtime.onMessage.addListener ({name, focusFrameId}) =>
-                  if name == "frameFocused" and @options?.focus and focusFrameId not in [frameId, @iframeFrameId]
-                    @hide false
-                  false # We will not be calling sendResponse.
-                # If this frame receives the focus, then hide the UI component.
-                window.addEventListener "focus", (event) =>
-                  if event.target == window and @options?.focus and not @options?.allowBlur
-                    @hide false
-                  true # Continue propagating the event.
-                # Register the UI component as ready and post its iframe port.
-                @uiComponentIsReady = true
-                setIframePort port1
-              when "setIframeFrameId" then @iframeFrameId = event.data.iframeFrameId
-              when "hide" then @hide()
-              else @handleMessage event
+        @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 }) =>
+            { port1, port2 } = new MessageChannel
+            @iframeElement.contentWindow.postMessage vimiumSecret, chrome.runtime.getURL(""), [ port2 ]
+            port1.onmessage = (event) =>
+              switch event?.data?.name ? event?.data
+                when "uiComponentIsReady"
+                  # If any other frame receives the focus, then hide the UI component.
+                  chrome.runtime.onMessage.addListener ({name, focusFrameId}) =>
+                    if name == "frameFocused" and @options?.focus and focusFrameId not in [frameId, @iframeFrameId]
+                      @hide false
+                    false # We will not be calling sendResponse.
+                  # If this frame receives the focus, then hide the UI component.
+                  window.addEventListener "focus", (event) =>
+                    if event.target == window and @options?.focus and not @options?.allowBlur
+                      @hide false
+                    true # Continue propagating the event.
+                  # Set the iframe's port, thereby rendering the UI component ready.
+                  setIframePort port1
+                when "setIframeFrameId" then @iframeFrameId = event.data.iframeFrameId
+                when "hide" then @hide()
+                else @handleMessage event
 
-  # Post a message (if provided), then call continuation (if provided).
+  # Post a message (if provided), then call continuation (if provided).  Because the constructor waits for
+  # documentReady(), @iframePort may not yet be ready.
   postMessage: (message = null, continuation = null) ->
-    @iframePort.use (port) =>
+    @iframePort?.use (port) =>
       port.postMessage message if message?
       continuation?()
 
@@ -81,20 +81,22 @@ class UIComponent
       @showing = true
 
   hide: (shouldRefocusOriginalFrame = true) ->
-    if @showing
-      @showing = false
-      @toggleIframeElementClasses "vimiumUIComponentVisible", "vimiumUIComponentHidden"
-      if @options?.focus
-        @iframeElement.blur()
-        if shouldRefocusOriginalFrame
-          if @options?.sourceFrameId?
-            chrome.runtime.sendMessage
-              handler: "sendMessageToFrames",
-              message: name: "focusFrame", frameId: @options.sourceFrameId, forceFocusThisFrame: true
-          else
-            window.focus()
-      @options = null
-      @postMessage "hidden" # Inform the UI component that it is hidden.
+    # We post a non-message (null) to ensure that hide() requests cannot overtake activate() requests.
+    @postMessage null, =>
+      if @showing
+        @showing = false
+        @toggleIframeElementClasses "vimiumUIComponentVisible", "vimiumUIComponentHidden"
+        if @options?.focus
+          @iframeElement.blur()
+          if shouldRefocusOriginalFrame
+            if @options?.sourceFrameId?
+              chrome.runtime.sendMessage
+                handler: "sendMessageToFrames",
+                message: name: "focusFrame", frameId: @options.sourceFrameId, forceFocusThisFrame: true
+            else
+              window.focus()
+        @options = null
+        @postMessage "hidden" # Inform the UI component that it is hidden.
 
   # Fetch a Vimium file/resource (such as "content_scripts/vimium.css").
   # We try making an XMLHttpRequest request.  That can fail (see #1817), in which case we fetch the
diff --git a/tests/dom_tests/vomnibar_test.coffee b/tests/dom_tests/vomnibar_test.coffee
index c5974a4c..501fd5dd 100644
--- a/tests/dom_tests/vomnibar_test.coffee
+++ b/tests/dom_tests/vomnibar_test.coffee
@@ -20,13 +20,12 @@ context "Keep selection within bounds",
       completer
 
     # Shoulda.js doesn't support async tests, so we have to hack around.
+    stub Vomnibar.vomnibarUI, "hide", ->
     stub Vomnibar.vomnibarUI, "postMessage", (data) ->
       vomnibarFrame.UIComponentServer.handleMessage {data}
     stub vomnibarFrame.UIComponentServer, "postMessage", (data) ->
       UIComponent.handleMessage {data}
 
-    stub Vomnibar.vomnibarUI, "uiComponentIsReady", true
-
   tearDown ->
     Vomnibar.vomnibarUI.hide()