Some WebDriver hooks will need to inform the client that execution has
completed, but without any knowledge of what endpoint was running. Since
there can only ever be a single WebDriver endpoint executing at once, we
can replace the completion callbacks with a single callback.
We currently spin the event loop to wait for the specified element to
become available. As we've seen with other endpoints, this can result
in dead locks if another web component also spins the event loop.
This patch makes the locator implementations asynchronous.
It's currently possible for window size/position updates to hang, as the
underlying IPCs are synchronous. This updates the WebDriver endpoint to
be async, to unblock the WebContent process while the update is ongoing.
The UI process is now responsible for informing WebContent when the
update is complete.
There was a timing issue here where WebDriver would dismiss a dialog,
and then invoke another endpoint before the dialog was actually closed.
This is because the dismissal first has to hop over to the UI process to
close the graphical dialog, which then asynchronously informs WebContent
of the result. It's not until WebContent receives that result that the
dialog is considered closed, thus those subsequent endpoints would abort
due a dialog being "open".
We now wait for dialogs to be fully closed before returning from the
dismissal endpoints.
Similar to commit c2cf65adac, we should
avoid spinning the event loop from the WebContent-side of the WebDriver
connection. This can result in deadlocks if another component in LibWeb
also spins the event loop.
The AO to await navigations has two event loop spinners - waiting for
the navigation to complete and for the document to reach the target
readiness state. We now use NavigationObserver and DocumentObserver to
be notified when these conditions are met. And we use the same async IPC
mechanism as script execution to notify the WebDriver process when all
conditions are met (or timed out).
Previously, tests would intermittently fail because the current session
wasn't yet aware of a newly created window handle.
Co-authored-by: Timothy Flynn <trflynn89@pm.me>
Similar to script execution, this spins the WebDriver process until the
action is complete (rather than spinning the WebContent process, which
we've seen result in deadlocks).
After closing a window, it is the client's job to switch to another
window before executing any other command. Currently, we will crash if
that did not happen when we try to send an IPC to a window handle that
we no longer hold. This patch makes us return a "no such window" error
instead.
The exceptions to this new check are the "Switch to Window" and "Get
Window Handles" commands.
It's difficult to know what we need to implement if we silently ignore
these endpoints. Let's log the endpoints and their parameters, and clean
up the wall of FIXME comments to be easier to grok.
We currently spin the platform event loop while awaiting scripts to
complete. This causes WebContent to hang if another component is also
spinning the event loop. The particular example that instigated this
patch was the navigable's navigation loop (which spins until the fetch
process is complete), triggered by a form submission to an iframe.
So instead of spinning, we now return immediately from the script
executors, after setting up listeners for either the script's promise to
be resolved or for a timeout. The HTTP request to WebDriver must finish
synchronously though, so now the WebDriver process spins its event loop
until WebContent signals that the script completed. This should be ok -
the WebDriver process isn't expected to be doing anything else in the
meantime.
Also, as a consequence of these changes, we now actually handle time
outs. We were previously creating the timeout timer, but not starting
it.
Added the following Routes, IPC definitions, and boilerplates for the
missing endpoints:
- Switch To Frame
- Switch To Parent Frame
- Element Clear
- Element Send Keys
`JsonValue::to_byte_string` has peculiar type-erasure semantics which is
not usually intended. Unfortunately, it also has a very stereotypical
name which does not warn about unexpected behavior. So let's prefix it
with `deprecated_` to make new code use `as_string` if it just wants to
get string value or `serialized<StringBuilder>` if it needs to do proper
serialization.
In a bunch of cases, this actually ends up simplifying the code as
to_number will handle something such as:
```
Optional<I> opt;
if constexpr (IsSigned<I>)
opt = view.to_int<I>();
else
opt = view.to_uint<I>();
```
For us.
The main goal here however is to have a single generic number conversion
API between all of the String classes.
This commit un-deprecates DeprecatedString, and repurposes it as a byte
string.
As the null state has already been removed, there are no other
particularly hairy blockers in repurposing this type as a byte string
(what it _really_ is).
This commit is auto-generated:
$ xs=$(ack -l \bDeprecatedString\b\|deprecated_string AK Userland \
Meta Ports Ladybird Tests Kernel)
$ perl -pie 's/\bDeprecatedString\b/ByteString/g;
s/deprecated_string/byte_string/g' $xs
$ clang-format --style=file -i \
$(git diff --name-only | grep \.cpp\|\.h)
$ gn format $(git ls-files '*.gn' '*.gni')
Previously it was possible to have following sequence of calls
while destroying a session:
1. `WebContentConnection::die()` calls `Client::close_session()`
2. `Client::close_session()` removes a session from active sessions
map which causes session destructor call.
3. Session destructor calls `Client::close_session()` to remove a
session from active sessions.
With `stop()` method inlined into destructor `close_session()` need
to be called just once while destroying a session.
The spec states to only try to close the session *if* it exists. This
situation can occur when closing a session after a Close Window command,
as the session will be closed automatically if it was the last window.
These steps now have more than one caller; specifically, they may be
called from the Delete Session and Close Window endpoints. The session
was only removed from the active session map for the former endpoint.
Instead, let's more accurately handle removing the session where the
spec tells us to, so that all callers properly perform this step.
When some WebDriver spec steps are implemented a bit more literally, we
will end up in a situation where we remove a session from its client's
active session map, but still have more steps to perform. Currently,
when we remove the session, it is immediately destroyed because it is
stored in an OwnPtr. Instead, we can store it as a RefPtr, which will
let the caller to such steps keep the session alive until the subsequent
steps are complete.
While here, this also changes the storage of active sessions to a
HashMap, as all lookups into it are currently a linear search.
With current architecture every window has its own WebContent process
and there is one WebDriver process that is responsible for talking to
all opened windows. It thus make sense to manage open windows from
WebDriver process instead of WebContent process that is not supposed
to know about all other opened WebContent processes.
This mostly reverts 826d5f8f9a but also
adds `web_content_connection` to window structure and window id
generation (currently out of spec).
With these changes `get_window_handles`, `switch_to_window` and
`close_window` start to actually switch, close and returned handles
of currently opened windows.
This changes the parameters parsed from a WebDriver HTTP request to
String for transferring over IPC. Conveniently, most locations these
were ultimately passed to only need a StringView.
This moves the actual launching of browser windows to the WebDriver main
file. This will allow Ladybird to specify its own callback and re-use
Serenity's Session class.
We have a new, improved string type coming up in AK (OOM aware, no null
state), and while it's going to use UTF-8, the name UTF8String is a
mouthful - so let's free up the String name by renaming the existing
class.
Making the old one have an annoying name will hopefully also help with
quick adoption :^)
Still some TODOs here:
* We don't handle all capabilities (e.g. proxy)
* We don't match the capabilities against the running browser
But this will parse the capabilities JSON object received from the
WebDriver client.
