Previously, trying to access a non-readable file would cause a
connection reset in the browser; trying to access a non-executable
directory would show a completely empty directory listing.
After moving to navigables, we started reusing the code that populates
session history entries with the srcdoc attribute value from iframes
in `Page::load_html()` for loading HTML.
This change addresses a crash in `determine_the_origin` which occurred
because this method expected the URL to be `about:srcdoc` if we also
provided HTML content (previously, it was the URL passed along with the
HTML content into `load_html()`).
Fixes regression introduced in b4fe118dff
The `WebContentConsoleClient` needs to be created not just once, but
for every new document. Although the JS Console window allows
communication only with the active document associated with the
top-level browsing context, we still need a console client for each
iframe's document to ensure their console logs are printed.
In the future, we might consider adding the ability to switch which
document the JS Console window communicates with.
Fixes https://github.com/SerenityOS/serenity/issues/21117
The Kernel/API directory in general shouldn't include userspace code,
but structure definitions that both are shared between the Kernel and
userspace.
All users of the ioctl API obviously use LibC so LibC is the most common
and shared library for the affected programs.
Previously, we used `on_load_finish` to determine when the text test
was completed. This method did not allow testing of async functions
because there was no way to indicate that the runner should wait for
the async call to end.
This change introduces a function in the `internals` object that is
intended to be called when the text test execution is completed. The
text test runner will now ignore `on_load_finish` which means a test
will timeout if this new function is never called.
`test(f)` function in `include.js` has been modified to automatically
terminate a test once `load` event is fired on `window`.
new `asyncTest(f)` function has been introduces. `f` receives function
that will terminate a test as a first argument.
Every test is expected to call either `test()` or `asyncTest()` to
complete. If not, it will remain hanging until a timeout occurs.
This attempts to load the URL of the first `<link rel="match" href=""/>`
it finds. If that tag is missing, we load an error page to make sure
the ref-test fails. (And to provide some feedback if someone looks at
the screenshot somehow.) Wrong URLs will instead end up loading the
default 404 error page.
Userspace initially didn't have any sort of mechanism to handle
device hotplug (either removing or inserting a device).
This meant that after a short term of scanning all known devices, by
fetching device events (DeviceEvent packets) from /dev/devctl, we
basically never try to read it again after SystemServer initialization
code.
To accommodate hotplug needs, we change SystemServer by ensuring it will
generate a known set of device nodes at their location during the its
main initialization code. This includes devices like /dev/mem, /dev/zero
and /dev/full, etc.
The actual responsible userspace program to handle hotplug events is a
new userspace program called DeviceMapper, with following key points:
- Its current task is to to constantly read the /dev/devctl device node.
Because we already created generic devices, we only handle devices
that are dynamically-generated in nature, like storage devices, audio
channels, etc.
- Since dynamically-generated device nodes could have an infinite minor
numbers, but major numbers are decoded to a device type, we create an
internal registry based on two structures - DeviceNodeFamily, and
RegisteredDeviceNode. DeviceNodeFamily objects are attached in the
main logic code, when handling a DeviceEvent device insertion packet.
A DeviceNodeFamily object has an internal HashTable to hold objects of
RegisteredDeviceNode class.
- Because some device nodes could still share the same major number (TTY
and serial TTY devices), we have two modes of allocation - limited
allocation (so a range is defined for a major number), or infinite
range. Therefore, two (or more) separate DeviceNodeFamily objects can
can exist albeit sharing the same major number, but they are required
to allocate from a different minor numbers' range to ensure there are
no collisions.
- As for KCOV, we handle this device differently. In case the user
compiled the kernel with such support - this happens to be a singular
device node that we usually don't need, so it's dynamically-generated
too, and because it has only one instance, we don't register it in our
internal registry to not make it complicated needlessly.
The Kernel code is modified to allow proper blocking in case of no
events in the DeviceControlDevice class, because otherwise we will need
to poll periodically the device to check if a new event is available,
which would waste CPU time for no good reason.
This change ensures that code in the Service class doesn't try to check
the g_system_mode variable, but instead is asked on whether it supports
a given system mode string value.
Also, don't assume that we should create sockets for any new Service
instance, but instead do that only if the Service should run in the
current system mode.
Just a small cleanup to ensure we can get these pieces of code out to
other files and still have the main.cpp file organized.
The populate_devtmpfs_char_devices_based_on_sysfs() method is removed
because we can simply create the /dev/devctl device node without looking
at the SysFS. This assumed-to-exist device node will be used later on in
an event loop to handle hotplug events.
Before page_did_create_main_document() only initialized ConsoleClient
for top-level browsing context which means that nested browsing context
could not print into the console.
With this change, ConsoleClient is initialized for documents created
for nested browsing context too. One ConsoleClient is shared between
all browsing contexts within the same page.
The Hurd has sin_len, just like the BSDs.
This happened to hit a clang-format bug, and we have been advised
to disable clang-format for this block of code for now.
The behavior of the Serenity `PlaybackStream` implementation should
match the `AudioCodecPluginSerenity` class removed by this commit. Any
inconsistencies should be fixable without needing feature additions to
the underlying implementation.
The name for this directory is a bit awkward. Also, the distinction of
constant information is not really valuable as I thought it would be, so
let's bring that information back into the /sys/kernel directory.
The name "variables" is a bit awkward and what the directory entries are
really about is kernel configuration so let's make it clear with the new
name.
On Serenity and on my Linux machine, we have a 1:1 CSS-to-device pixel
ratio. On macOS, we have a 2:1 ratio. This did not affect the Qt chrome,
because we are ignoring this position and placing the context menu at
the globally-accessible mouse position. The AppKit chrome will be using
this position, though.
This might be what caused the issue worked around by commit 8177ecb.
Having all 4 of these methods together makes it more immediately obvious
that the 2 moved here are not correct (we are not converting the CSS
pixel point to a device pixel point).
Stop worrying about tiny OOMs. Work towards #20449.
While going through these, I also changed the function signature in many
places where returning ThrowCompletionOr<T> is no longer necessary.
We already do this for headless-browser. There's no need to open any URL
other than about:blank when starting a WebDriver session. We should also
do this from WebDriver code, rather than in special logic in Browser's
main.cpp.
This patch checks for visible items to determine the menu height. Now
the last visible item is used to determine the height of the menu.
Before this patch that menu height could be wrong e.g. if the last item
was not visible.
LibTLS still can't access many parts of the web, so let's hide this
behind a flag (with all the plumbing that entails).
Hopefully this can encourage folks to improve LibTLS's algorithm support
:^).
In order to follow spec text to achieve this, we need to change the
underlying representation of a host in AK::URL to deserialized format.
Before this, we were parsing the host and then immediately serializing
it again.
Making that change resulted in a whole bunch of fallout.
After this change, callers can access the serialized data through
this concept-host-serializer. The functional end result of this
change is that IPv6 hosts are now correctly serialized to be
surrounded with '[' and ']'.
Previously, all files and folders copied with FileManager would be
created with the default mode bits set and would have the same uid
and gid as the currently logged in user.
Importantly, we now only consider overflow from descendants with
explicltly visible overflow, and only from descendants that have the
measured box as their containing block.
Also, we now measure scrollable overflow for all boxes, not just scroll
containers. This will allow us to fix a long-standing paint problem in
the next commit.
This prevents fd leaks when the user of the API forgets to pass
CloseAfterSending to IPC::File. Since we are calling leak_fd in the
constructor, we want it to also take care of closing.
The previous iteration of this API was somewhat odd and rough in random
places, which degraded usability and made less than perfect sense.
This commit reworks the API to be a little closer to more
conventional promise APIs (a la javascript promises).
Also adds a test to ensure the class even works.
The inspector widget now has a new ARIA tab which displays an
individual element's ARIA properties and state. The view itself
is pretty basic for now, just being a table- there is definitely room
for some better UX here but it's enough for a first cut.
We used to not care about stopping an audio output stream for Intel HDA
since AudioServer would continuously send new buffers to play. Since
707f5ac150ef858760eb9faa52b9ba80c50c4262 however, that has changed.
Intel HDA now uses interrupts to detect when each buffer was completed
by the device, and uses a simple heuristic to detect whether a buffer
underrun has occurred so it can stop the output stream.
This was tested on Qemu's Intel HDA (Linux x86_64) and a bare metal MSI
Starship/Matisse HD Audio Controller.
This change was a long time in the making ever since we obtained sample
rate awareness in the system. Now, each client has its own sample rate,
accessible via new IPC APIs, and the device sample rate is only
accessible via the management interface. AudioServer takes care of
resampling client streams into the device sample rate. Therefore, the
main improvement introduced with this commit is full responsiveness to
sample rate changes; all open audio programs will continue to play at
correct speed with the audio resampled to the new device rate.
The immediate benefits are manifold:
- Gets rid of the legacy hardware sample rate IPC message in the
non-managing client
- Removes duplicate resampling and sample index rescaling code
everywhere
- Avoids potential sample index scaling bugs in SoundPlayer (which have
happened many times before) and fixes a sample index scaling bug in
aplay
- Removes several FIXMEs
- Reduces amount of sample copying in all applications (especially
Piano, where this is critical), improving performance
- Reduces number of resampling users, making future API changes (which
will need to happen for correct resampling to be implemented) easier
I also threw in a simple race condition fix for Piano's audio player
loop.
The immutability of the string is not relevant here, since the string
we're given was allocated in the IPC serialization layer and will be
destroyed shortly afterwards. Additionally, noone relies on
DeprecatedString-specific functionality. This will make it easier to
convert the IPC layer itself to String later on.
This is a sensible separation of concerns that mirrors the WindowServer
IPC split. On the one hand, there is the "normal" audio interface, used
for clients that play audio, which is the primary service of
AudioServer. On the other hand, there is the management interface,
which, like the WindowManager endpoint, provides higher-level control
over clients and the server itself.
The reasoning for this split are manifold, as mentioned we are mirroring
the WindowServer split. Another indication to the sensibility of the
split is that no single audio client used the APIs of both interfaces.
Also, useless audio queues are no longer created for managing clients
(since those don't even exist, just like there's no window backing
bitmap for window managing clients), eliminating any bugs that may occur
there as they have in the past.
Implementation-wise, we just move all the APIs and implementations from
the old AudioServer into the AudioManagerServer (and respective clients,
of course). There is one point of duplication, namely the hardware
sample rate. This will be fixed in combination with per-client sample
rate, eliminating client-side resampling and the related update bugs.
For now, we keep one legacy API to simplify the transition.
The new AudioManagerServer also gains a hardware sample rate change
callback to have exact symmetry on the main server parameters (getter,
setter, and callback).
This facility was added in 15a1d9a, but isn't being used for anything.
It wasn't even hooked up to LibGUI for applications to use.
Relevant use-cases, such as the most prominent one in `AnalogClock`, use
`GUI::Window::set_frameless()` instead.
From what I can tell, this facility was added to WSWindow/GWindow in
2019 in 9b71307. I only found a single place in the codebase still using
this facility: `WindowServer::Menu::start_activation_animation()`. A
subtle fade-out animation that happens when a menu item is selected, and
the menu disappears.
I think our compositing facilities have improved enough to make this
facility redundant. The remaining use mentioned above was ported to just
directly blit the fade-out animation instead of requesting it from
WindowServer.
It's currently possible to seek to the total sample count of an audio
loader. We must limit seeking to one less than that count.
This mistake was duplicated in both AudioCodecPluginSerenity/Ladybird,
so the computation was moved to a helper in the base AudioCodecPlugin.
Specifying port 0 on the command line causes WebServer to select a
random available port. We now show the port WebServer is actually
using rather than assuming it is the same as the command line argument.
The main thread in the WebContent process is often busy with layout and
running JavaScript. This can cause audio to sound jittery and crack. To
avoid this behavior, we now drive audio on a secondary thread.
Note: Browser on Serenity uses AudioServer, the connection for which is
already handled on a secondary thread within LibAudio. So this only
applies to Lagom.
Rather than using LibThreading, our hands are tied to QThread for now.
Internally, the Qt media objects use a QTimer, which is forbidden from
running on a thread that is not a QThread (the debug console is spammed
with messages pointing this out). Ideally, in the future AudioServer
will be able to run for non-Serenity platforms, and most of this can be
aligned with the Serenity implementation.
This commit changes the variables used to represent the size and
progress of downloads from u32 to u64. This allows `pro` and
`Browser` to report the total size and progress of a download
correctly for downloads larger than 4GiB.
The data we want to send out of the WebContent process is identical for
audio and video elements. Rather than just duplicating all of this for
audio, generalize the names used for this IPC for all media elements.
This also encapsulates that data into a struct. This makes adding new
fields to be sent much easier (such as an upcoming field for muting the
element).
Both `Database` and `Heap` were allowed to be opened twice. Prevent
this, and change SQLServer to only open databases that are not already
opened.
This fixes a Ladybird crash where opening the application twice would
erroneously duplicate free heap block indices.
We used to call `did_exit()` directly with the status returned from
`waitpid` but the function expected an exit code. We now use several
of `wait`-related macros to deduce the correct information.
This creates (and installs upon WebContent startup) a platform plugin to
play audio data.
On Serenity, we use AudioServer to play audio over IPC. Unfortunately,
AudioServer is currently coupled with Serenity's audio devices, and thus
cannot be used in Ladybird on Lagom. Instead, we use a Qt audio device
to play the audio, which requires the Qt multimedia package.
While we use Qt to play the audio, note that we can still use LibAudio
to decode the audio data and retrieve samples - we simply send Qt the
raw PCM signals.
Although DistinctNumeric, which is supposed to abstract the underlying
type, was used to represent CSSPixels, we have a whole bunch of places
in the layout code that assume CSSPixels::value() returns a
floating-point type. This assumption makes it difficult to replace the
underlying type in CSSPixels with a non-floating type.
To make it easier to transition CSSPixels to fixed-point math, one step
we can take is to prevent access to the underlying type using value()
and instead use explicit conversions with the to_float(), to_double(),
and to_int() methods.
If we try to launch a lazily-spawned service and the SystemServer as a
(running --user) session leader is running with root permissions, then
if it is instructed to drop the root permissions for a the new service
then it will make sense to abort the entire spawn procedure if dropping
of privileges failed.
For other users, trying to change UID/GID to something else doesn't make
sense (and will always actually fail) as we are already running in non
root permissions, hence we don't attempt to do this anymore.
It should be noted that if an explicit User configuration was actually
specified for a Service to be used with, we would still try to login
with the requested User option value, which would fail when running as
non-root user.
This is useful for example when trying to run the pro utility with pls
to elevate to root permissions, but the session leader is still the same
so trying to "drop" privileges to UID 0 doesn't make sense.
This makes it possible to set a pseudo-element as the inspected node
using Document::set_inspected_node(), Document then provides
inspected_layout_node() for the painting related functions.
Core::File's new `DontCreate` open mode removes the need for these
capabilities on SpiceAgent. We shouldn't have to create this file,
as if it doesn't exist, QEMU never initiated a spice connection!
If an image had a valid header and valid metadata, but decoding the
image frame data failed, the renderer used to crash.
The crash only happened in SerenityOS, because there
ImageCodecPluginSerenity returned nullptr bitmaps. Instead, return
{} like ImageCodecPluginLadybird already does if there's a nullptr
frame.
Fixes#19141.
Loading #19141 in the browser satisfyingly also serves as a manual
test for the bug. (No automated test since we don't run layout
tests within SerenityOS on the bots.)
Before this commit, notifications would appear in the top left of the
screen when created, then move to the top right once hovered by the
mouse. This happened because the first notification would use its own
default-constructed position of 0,0 as a point of reference.
The server cannot use these values anywhere, because this method always
sets 'prompt = ShouldPrompt::No'. This saves a bunch of roundtrips for
all clients that use FSAS to read abritrary files.
This is a clear sign that they want to use a UnixDateTime instead.
This also adds support for placing durations and date times into SQL
databases via their millisecond offset to UTC.
That's what this class really is; in fact that's what the first line of
the comment says it is.
This commit does not rename the main files, since those will contain
other time-related classes in a little bit.
This fixes a plethora of rounding problems on many websites.
In the future, we may want to replace this with fixed-point arithmetic
(bug #18566) for performance (and consistency with other engines),
but in the meantime this makes the web look a bit better. :^)
There's a lot more things that could be converted to doubles, which
would reduce the amount of casting necessary in this patch.
We can do that incrementally, however.
The spice server will ignore any clipboard-related messages if we don't
have the appropriate capabilities, but I think it's better for us to
do less CPU churning whenever the user copies something to their
clipboard.
It also stops the spice server from warning in the console
about a clipboard grab message being recieved when the capability was
never announced.
Previously, calling `.right()` on a `Gfx::Rect` would return the last
column's coordinate still inside the rectangle, or `left + width - 1`.
This is called 'endpoint inclusive' and does not make a lot of sense for
`Gfx::Rect<float>` where a rectangle of width 5 at position (0, 0) would
return 4 as its right side. This same problem exists for `.bottom()`.
This changes `Gfx::Rect` to be endpoint exclusive, which gives us the
nice property that `width = right - left` and `height = bottom - top`.
It enables us to treat `Gfx::Rect<int>` and `Gfx::Rect<float>` exactly
the same.
All users of `Gfx::Rect` have been updated accordingly.
Now, we write the data recieved to a file when the user drags a file
onto the Spice Viewer window. Once complete, the FileExplorer will open
with the copied file highlighted.
The old message system was very dependent on syscalls, and the overall
structure made it hard to implement new features.
The new message system is pretty expandible, where each message has its
own dedicated class. As well as this, we now use Core::File and
AK::Stream for reading and writing messages.
Using AK::Stream also allows us to change the actual data source
(in this case, Core::File) without having to update a whole lot of code
in the future.
It occurred to me that when trying to running "pls pro SOME_URL" with a
subsequent failure (which will be fixed in a future patch), that a small
error message was printed to the debug log about "Failed to drop
privileges (GID=0, UID=0)".
To actually understand where it failed, I added the actual errno to
printed message which helped me with further debugging, but this could
easily help others in similar scenarios so let's print the actual error.
Instead of showing an ambiguous "Unknown error" when FSAS approval is
denied, let's affirm the user's action wasn't permitted if they
reject the prompt.
These 2 are an actual separate types of syscalls, so let's stop using
special flags for bind mounting or re-mounting and instead let userspace
calling directly for this kind of actions.
The resolved property sets are stored with the element in a
per-pseudo-element array (same as for pseudo element layout nodes).
Longer term, we should stop storing this with elements entirely and make
it temporary state in StyleComputer somehow, so we don't waste memory
keeping all the resolved properties around.
This makes various gradients show up on https://shopify.com/ :^)
This allows for the browser process to control the play/pause state,
whether we paint user agent controls on the video, and whether the video
loops when it finishes playing.
This just sets up the IPC to notify the browser process of context menu
requests on video elements. The IPC contains a few pieces of information
about the state of the video element.
Previously it was possible for a window to register as a parentless
blocking modal then add itself to a stealable parent's modal chain,
bypassing a mode misbehavior check in create_window()
Also relaxes reciprocity for blockers with the same parent. This
scenario is usually created by simultaneous MessageBoxes. It's not
an ideal UX to cascade these, but there's no need to crash over it.
Creates two new gatekept helpers for FilePicker and MessageBox to be
used by FSAS to replace the "dummy window" approach to centering
Dialogs. There was a slight delay in creating two windows, one a
transparent intermediary hidden behind the second, to display FSAS
Dialogs. Now we only need to make the window we actually see.
When the host clears the clipboard (e.g. by running `pbcopy </dev/null`)
the Spice server sends an empty string to us. Previously, we would crash
as `AnonymousBuffer::create_with_size` doesn't accept a size of 0. Fix
this by passing `{}` to `async_set_clipboard_data` in this case.
Note that previously, the only check was that at least one byte was read
from /dev/devctl. This is incorrect, as potentially not the entire
struct was read. In practice, this probably never happened, but the new
code at least detects this case and aborts.
Resolves#18624
Switching to and from fullscreen produces a behaviour where window
content too big in relation to window size.
This patch fixes sent resize event to contain current
window size.
The pattern to construct `Application` was to use the `try_create`
method from the `C_OBJECT` macro. While being safe from an OOM
perspective, this method doesn't propagate errors from the constructor.
This patch make `Application` use the `C_OBJECT_ABSTRACT` and manually
define a `create` method that can bubble up errors from the
construction stage.
This commit also removes the ability to use `argc` and `argv` to
create an `Application`, only `Main`'s `Arguments` can be used.
From a user point of view, the patch renames `try_create` => `create`,
hence the huge number of modified files.
Previously we would exit the dequeuing loop after just one buffer
had been dequeued due to some bogus logic. This would manifest
when stopping and starting a track in SoundPlayer, where a few
miliseconds of 'old' audio would play when restarting the playback.
This commit makes sure we clear the entire queue.
ChessEngine and the chess GUI will no longer crash on error while
reading UCI commands. ChessEngine will print a message to the standard
output, while the GUI will ignore unknown commands. Both will print
the error to the debug log if the UCI_DEBUG flag is enabled.
Trailing and preceding whitespace is now stripped from commands before
they are parsed. Commands which are just whitespace no longer produce
errors.
Previously, Frames could set both these properties along with a
thickness to confusing effect: Most shapes of the same shadowing only
differentiated at a thickness >= 2, and some not at all. This led
to a lot of creative but ultimately superfluous choices in the code.
Instead let's streamline our options, automate thickness, and get
the right look without so much guesswork.
Plain shadowing has been consolidated into a single Plain style,
and 0 thickness can be had by setting style to NoFrame.
This program has never lived up to its original idea, and has been
broken for years (property editing, etc). It's also unmaintained and
off-by-default since forever.
At this point, Inspector is more of a maintenance burden than a feature,
so this commit removes it from the system, along with the mechanism in
Core::EventLoop that enables it.
If we decide we want the feature again in the future, it can be
reimplemented better. :^)
Not a single client of this API actually used the event mask feature to
listen for readability AND writability.
Let's simplify the API and have only one hook: on_activation.
We never clear content filters on either end of the Browser-WebContent
IPC connection. So when the filters change, we re-append all filters to
the Vector holding them. This incidentally makes it impossible to remove
a filter.
Change both sides to clear their filter lists when receiving a new set
of filters.
It returns a PaintableBox (a PaintableWithLines, to be specific), not a
'PaintBox'. paintable_box() without the cast is already available
through BlockContainer's Box base class, we don't need to shadow it.
The spec defines a Permissions Policy to control some browser behaviors
on a per-origin basis. Management of these permissions live in their own
spec: https://w3c.github.io/webappsec-permissions-policy/
This implements a somewhat ad-hoc Permissions Policy for autoplaying
media elements. We will need to implement the entire policy spec for
this to be more general.
This now defaults to serializing the path with percent decoded segments
(which is what all callers expect), but has an option not to. This fixes
`file://` URLs with spaces in their paths.
The name has been changed to serialize_path() path to make it more clear
that this method will generate a new string each call (except for the
cannot_be_a_base_url() case). A few callers have then been updated to
avoid repeatedly calling this function.
Some keymaps will bind key presses with the alt modifier to characters
other than the unmodified one, in which case you couldn't activate the
alt shortcuts in the menu bar before.
We now ask the current keymap for the code point that is mapped to the
pressed (unmodified) key instead.
While the window geometry overlay is centered inside the tile overlay
neither the text nor the location change, so there is no need to
re-render and update it every time the window moves.
Keep track of areas that overlays were rendered to when we recompute
occlusions. This allows us to then easily figure out areas where
overlays were moved from or removed from.
Some of these are allocated upon initialization of the intrinsics, and
some lazily, but in neither case the getters actually return a nullptr.
This saves us a whole bunch of pointer dereferences (as NonnullGCPtr has
an `operator T&()`), and also has the interesting side effect of forcing
us to explicitly use the FunctionObject& overload of call(), as passing
a NonnullGCPtr is ambigous - it could implicitly be turned into a Value
_or_ a FunctionObject& (so we have to dereference manually).
This ports MouseEvent, UIEvent, WheelEvent, and Event to new String.
They all had a dependency to T::create() in
WebDriverConnection::fire_an_event() and therefore had to be ported in
the same commit.
This fixes an issue found on Linus's hosted WebServer. Now, if WebServer
is hosted at a non-root URL. (eg, `example.com/webserver` instead of
`example.com`) the links will correctly go to
`example.com/webserver/foo` instead of `example.com/foo`.
The spec states that creating new windows "must be done without invoking
the focusing steps for the created browsing context". It also states we
should do so by "running the window open steps", but nowhere in those
steps is there an option to invoke or skip any focusing steps. So we do
so with a custom WebContent IPC parameter.
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.
Currently we have `m_should_block_pop_ups` set to true by default
which means `choose_a_browsing_context` will early return if new
top-level browsing context is requested and write `Pop-up blocked!`
in console. It is good but when WebDriver is connected we want it
to be able to actually open a new window if one is requested.
