Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add option to make creating a Window synchronous #137

Merged
merged 3 commits into from
Sep 24, 2018
Merged

Add option to make creating a Window synchronous #137

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f03ee91
Add :async option to make `Window()` synchronous!
NHDaly Jul 27, 2018
1d81745
Change Window() async option to pure julia kwarg.
NHDaly Sep 22, 2018
10cc2ba
added test that Window(async=true) is faster than async=false.
NHDaly Sep 22, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions res/blink.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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); }
}
})();
29 changes: 23 additions & 6 deletions src/AtomShell/window.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)...))),
Expand Down
1 change: 1 addition & 0 deletions src/content/main.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
<html>
<head>
<script>var id = {{id}}</script>
<script>{{optional_create_window_callback}}</script>
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be better to put the <script></script> tags in the mustache template as well so that there isn't an empty script node sitting around if the callback isn't being used?
I had it this way originally, because i think it makes the .html file more readable: you know that's going to be a script node, no matter what its contents end up being.

<script src="blink.js"></script>
<link rel="stylesheet" href="reset.css">
<link rel="stylesheet" href="blink.css">
Expand Down
5 changes: 4 additions & 1 deletion src/content/server.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
1 change: 0 additions & 1 deletion src/rpc/rpc.jl
View file
Open in desktop
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

11 changes: 10 additions & 1 deletion test/AtomShell/window.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
8 changes: 4 additions & 4 deletions test/content/api.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) == ""

Expand All @@ -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) {
Expand Down
4 changes: 1 addition & 3 deletions test/runtests.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down