Merge pull request #137 from NHDaly/synchronous_Window

Add option to make creating a Window synchronous: `Window(async=false)`
JuliaGizmos · Sep 24, 2018 · 93aeb16 · 93aeb16
2 parents 7e654de + 10cc2ba
commit 93aeb16
Show file tree

Hide file tree

Showing 8 changed files with 47 additions and 16 deletions.
diff --git a/res/blink.js b/res/blink.js
@@ -147,4 +147,8 @@
 
   Blink.click = click;
 
+  // Window creation callback: Mark this window as done loading.
+  if (typeof callback_id !== 'undefined') {
+    Blink.sock.onopen = ()=>{ cb(callback_id, true); }
+  }
 })();
diff --git a/src/AtomShell/window.jl b/src/AtomShell/window.jl
@@ -24,18 +24,35 @@ const window_defaults = @d(:url => "about:blank",
 
 raw_window(a::Electron, opts) = @js a createWindow($(merge(window_defaults, opts)))
 
-function Window(a::Shell, opts::AbstractDict = Dict())
+function Window(a::Shell, opts::AbstractDict = Dict(); async=true)
+  # TODO: Custom urls don't support async b/c don't load Blink.js. (Same as https://github.com/JunoLab/Blink.jl/issues/150)
   return haskey(opts, :url) ?
     Window(raw_window(a, opts), a, nothing) :
-    Window(a, Page(), opts)
+    Window(a, Page(), opts, async=async)
 end
 
-function Window(a::Shell, content::Page, opts::AbstractDict = Dict())
-  opts = merge(opts, Dict(:url => Blink.localurl(content)))
-  return Window(raw_window(a, opts), a, content)
+function Window(a::Shell, content::Page, opts::AbstractDict = Dict(); async=true)
+  url = Blink.localurl(content)
+  if !async
+      # Send the callback id as a query param in the url.
+      id, cond = Blink.callback!()
+      url *= "?callback=$id"
+  end
+  # Create the window.
+  opts = merge(opts, Dict(:url => url))
+  w = Window(raw_window(a, opts), a, content)
+  # If callback is requested, wait until the window has finished loading.
+  if !async
+      val = wait(cond)
+      if isa(val, AbstractDict) && get(val, "type", "") == "error"
+          err = JSError(get(val, "name", "unknown"), get(val, "message", "blank"))
+          throw(err)
+      end
+  end
+  return w
 end
 
-Window(args...) = Window(shell(), args...)
+Window(args...; kwargs...) = Window(shell(), args...; kwargs...)
 
 dot(a::Electron, win::Integer, code; callback = true) =
   js(a, :(withwin($(win), $(jsstring(code)...))),

diff --git a/src/content/main.html b/src/content/main.html
@@ -2,6 +2,7 @@
 <html>
   <head>
     <script>var id = {{id}}</script>
+    <script>{{optional_create_window_callback}}</script>
     <script src="blink.js"></script>
     <link rel="stylesheet" href="reset.css">
     <link rel="stylesheet" href="blink.css">

diff --git a/src/content/server.jl b/src/content/server.jl
@@ -20,7 +20,10 @@ function page_handler(req)
   haskey(pool, id) || @goto fail
   active(pool[id].value) && @goto fail
 
-  return render(maintp, d("id"=>id))
+  callback_id = try split(req[:query], "=")[2] catch e nothing end
+  callback_script = callback_id != nothing ? """var callback_id = $callback_id""" : ""
+  return render(maintp, d("id"=>id,
+                          "optional_create_window_callback"=>callback_script))
 
   @label fail
   return d(:body => "Not found",

diff --git a/src/rpc/rpc.jl b/src/rpc/rpc.jl
@@ -51,4 +51,3 @@ end
 macro js_(o, ex)
     :(js($(esc(o)), $(Expr(:quote, ex)), callback=false))
 end
-
diff --git a/test/AtomShell/window.jl b/test/AtomShell/window.jl
@@ -2,9 +2,18 @@ using Blink
 using Test
 
 @testset "size Tests" begin
-    w = Window(Blink.@d(:show => false, :width=>150, :height=>100)) ; sleep(5.0);
+    w = Window(Blink.@d(:show => false, :width=>150, :height=>100), async=false);
     @test size(w) == [150,100]
 
     size(w, 200,200)
     @test size(w) == [200,200]
 end
+
+@testset "async" begin
+    # Test that async Window() creation is faster than synchronous creation.
+    # (Repeat the test a few times, just to be sure it's consistent.)
+    for _ in 1:5
+        (@timed Window(Blink.@d(:show => false), async=true))[2] <
+         (@timed Window(Blink.@d(:show => false), async=false))[2]
+    end
+end
diff --git a/test/content/api.jl b/test/content/api.jl
@@ -2,7 +2,7 @@ using Blink
 using Test
 
 @testset "content! Tests" begin
-    w = Window(Blink.@d(:show => false)); sleep(5.0)
+    w = Window(Blink.@d(:show => false), async=false);
     body!(w, "", async=false);
     @test (@js w document.querySelector("body").innerHTML) == ""
 
@@ -20,22 +20,22 @@ using Test
     fadeTestHtml = """<script>var testJS = "test";</script><div id="d">hi world</div>"""
     @testset "Fade True" begin
         # Must create a new window to ensure javascript is reset.
-        w = Window(Blink.@d(:show => false)); sleep(5.0)
+        w = Window(Blink.@d(:show => false), async=false);
 
         body!(w, fadeTestHtml; fade=true, async=false);
         @test (@js w testJS) == "test"
     end
     @testset "Fade False" begin
         # Must create a new window to ensure javascript is reset.
-        w = Window(Blink.@d(:show => false)); sleep(5.0)
+        w = Window(Blink.@d(:show => false), async=false);
 
         body!(w, fadeTestHtml; fade=false, async=false);
         @test (@js w testJS) == "test"
     end
 end
 
 @testset "Sync/Async content reload tests" begin
-    w = Window(Blink.@d(:show => false)); sleep(5.0)
+    w = Window(Blink.@d(:show => false), async=false);
     sleep_content(seconds) = """
         <script>
             function spinsleep(ms) {

diff --git a/test/runtests.jl b/test/runtests.jl
@@ -8,9 +8,7 @@ cleanup = !AtomShell.isinstalled()
 cleanup && AtomShell.install()
 
 # open window and wait for it to initialize
-# TODO: can we remove the sleep(10) when the Window()
-# constructor is made synchronous?
-w = Window(Blink.@d(:show => false)); sleep(10.0)
+w = Window(Blink.@d(:show => false), async=false);
 
 # make sure the window is really active
 @test @js(w, Math.log(10)) ≈ log(10)
Original file line number	Diff line number	Diff line change
Expand Up		@@ -51,4 +51,3 @@ end
		macro js_(o, ex)
		:(js($(esc(o)), $(Expr(:quote, ex)), callback=false))
		end