That way, we avoid the racy map access via Browser.executorForTarget. If
a context is attached to a target, the Target field must be non-nil.
The Browser.pages map is still racy, since multiple tabs can be created
concurrently; we'll fix this other data race in another commit.
This vastly speeds up 'go test' on my laptop from ~10s to ~3s, as we
save a lot of time spinning up new Chrome browser processes.
In practice, each tab is a separate process anyway, but there's a lot of
added overhead if we're firing up the entire browser, particularly with
an empty user data dir.
This makes 'go test' racy now, as Browser doesn't support creating tabs
concurrently right now. Follow-up commits will fix that, with the help
of 'go test -race' after this commit.
We broke this in the refactor because of a nil pointer dereference, but
we didn't catch that as none of the tests loaded a page with an iframe.
That is, a page with multiple frames.
Add such a test, and fix the bug by creating an almost-empty frame when
we start receiving events about a new frame before it's navigated to.
Using a smaller viewport speeds up both tests, and lets us know what
dimensions to expect in TestCaptureScreenshot.
For TestScreenshot, we can know what dimensions to expect in advance, as
we have the images in testdata.
'go test -run Screenshot' goes from ~0.9s to ~0.5s on my machine.
Finally, don't run ExampleTitle as part of 'go test', as it's slow.
The Default folder is created asynchronously to Chrome starting to
listen on the debugging protocol port. So we can't expect it to exist.
Instead, base the example on DevToolsActivePort, which we can rely on.
First, collapse Browser.Start with NewBrowser. There's no reason to
split them up.
Second, unexport Browser.userDataDir, since it's only needed for a test.
It's also a bad precedent, as only the ExecAllocator will control the
user data directory.
Third, export Context.Browser, since we were already exporting
Context.Allocator.
Finally, remove the Executor interface, a duplicate of cdp.Executor.
All it did was wait on the entire allocator, which is confusing. From
the user's perspective, this wait method should instead wait for the
resources for its own browser, and not any other browsers sharing the
same allocator.
We haven't decided how to integrate that into our API, so simply replace
it with Allocator.Wait.
Now that they're both faster and more reliable than before the refactor.
On my laptop, 'go test' now consistently takes ~10s, and I haven't found
any flakes in the past couple of days.
The navigate sleeps can be replaced by appropriate wait actions.
Some other tests don't need any sleeps at all. This might be because
work is done synchronously now; I haven't been able to get test flakes
after hundreds of test runs with flags like -parallel=32 -count=200.
We hadn't noticed a few uncaught exceptions being received from the
browser, because the events were ignored. Start printing them via the
error logger.
The ones we were getting were caused by testAllocate running Navigate
actions when the path argument was empty. Navigating to "testdata/"
causes JS exceptions, as it's not a valid page.
Instead, leave the new target pointing at a blank document.
Use a single websocket connection per browser, removing the need for an
extra websocket connection per target.
This is thanks to the Target.sendMessageToTarget command to send
messages to each target, and the Target.receivedMessageFromTarget event
to receive messages back.
The browser handles activity via a single worker goroutine, and the same
technique is used for each target. This means that commands and events
are dealt with in order, and we can do away with some complexity like
mutexes and extra go statements.
First, we want all of the functionality in a single package; this means
collapsing whatever is useful into the root chromedp package.
The runner package is being replaced by the Allocator interface, with a
default implementation which starts browser processes.
The client package doesn't really have a place in the new design. The
context, allocator, and browser types will handle the connection with
each browser.
Finally, the new API is context-based, hence the addition of context.go.
The tests have been modified to build and run against the new API.
This isn't strictly necessary, as one can always build with the earlier
cdproto version specified in go.mod. However, many people still install
chromedp in GOPATH via 'go get -u', so this workaround makes life easier
for a lot of developers.
Fixes#285.
As spotted in #162 by a contributor, if the context is done before the
Selector.run caller has received from the channel, the spawned goroutine
may leak if blocked on a send.
Turns out that these subtests are the only pair which cannot run in
parallel with each other. Undo that change and add a TODO. This should
fix the CI failures.
While at it, remove an unnecessary testAllocate line.
We know we're going to see an error, so don't log it too:
$ go test -run TestAllocatePortInUse
2019/02/21 17:11:34 ERROR: pool could not allocate runner ...
PASS
ok github.com/chromedp/chromedp 0.004s
The subtests were almost all marked as parallel, but that's not enough.
That only makes the subtests run in parallel with other subtests within
the same tests, not with any other test.
Since none of the tests make use of globals nor require the entire
program to themselves, properly run all the tests in parallel.
Speeds up 'go test' on my 8-core laptop from an average of ~130s to an
average of ~50s. Many tests hit timeouts and have sleeps, so we want to
avoid running those sequentially whenever possible.
It supports alternative names for Chrome such as chromium, as well as
extra names to look for like headless-shell.
Also swap the os.Getenv logic, so that we only do the exec.LookPath work
if the env var is unset.
tip is rather unstable, so we shouldn't block PRs if it happens to break
our build or tests.
While at it, run 'go mod tidy' with the latest tip version.
Before the fix, the added test would give a Pool.Allocate error like:
pool could not connect to 9000: timeout waiting for initial target
The actual underlying error, which can only be seen if one inspects
chrome's stderr, is that it failed to bind to the debugging protocol
port if it was already in use.
This is of course an issue with the environment that chromedp is being
run under, since it was given a port range that wasn't available.
However, the confusing error can lead to developers wasting their time
instead of spotting the error quickly.
Unfortunately, there doesn't seem to be a way to have Chrome exit
immediately if it can't bind to the given port. So, instead of relying
on it, check if the current process can bind to the port first.
Add a test too, where we grab the first port in the pool range, and
check that we get an error that's not confusing.
Fixes#253.
