The inspector view allows modifying the attributes of DOM elements.
However, we lie to the devtools client: While it looks like the
attributes change, the changes are never actually applied to the DOM.
This change fixes that, and also makes it so attribute modifications
from non-inspector sources are shown in the inspector.
Testing: This change adds two tests
---------
Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>
Both the `DevtoolsInstance` and the `BrowsingContextActor` maintain a
list of live connections. When a new global is created, its
`BrowsingContextActor` copies the list of active connections from the
`DevtoolsInstance`. When a client handler thread exits, the
`BrowsingContextActor`s remove the connection from their list of
connections.
This has two implications:
* `BrowsingContextActor`s don't know about connections that are created
after they were constructed, causing them to possibly incorrectly tell
script that it doesn't need to send devtools updates anymore
* the `DevtoolsInstance` never notices when connections are closed and
panics when writing to them.
To fix this, I've removed the list of connections from the
`BrowsingContextActor` and adjusted the logic to notify script when
updates aren't needed anymore accordingly.
For some reason, our tests always open two connections and close the
first one immediately, which is how I found this bug in the first place.
Part of https://github.com/servo/servo/issues/42454 - previously
d5d400c7d6/components/script/dom/globalscope.rs (L240-L242)
was always false during our tests.
---------
Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>
When the servo window is closed while there is an active devtools
connection then the devtools threads reliably panics because it can't
talk to script anymore. We can simply ignore the `Disconnected` error
when shutting down. If the script thread already shut down then there's
nothing meaningful for the devtools to send anyways.
Testing: This kind of interaction is hard to test. I've not written an
automated test.
Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>
This change makes the devtools inspector show any event listeners that
are attached to a node. The primary motivation is making the devtools
more useful for debugging real-world websites.
<img width="536" height="268" alt="image"
src="https://github.com/user-attachments/assets/5ba13e41-14b4-4202-b994-eadff5d1c6b5"
/>
You can also click on the event listener to show some more info, though
most of that (everything except the event type and the event phase) is
currently just placeholder text:
<img width="1360" height="456" alt="image"
src="https://github.com/user-attachments/assets/ec025847-43fc-489c-8b26-46afb6dada64"
/>
Testing: This change adds a new test
---------
Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>
This implements MallocSizeOf for DevtoolsInstance. Major changes:
- Newtype for ActorRegistry because AtomicRefCell<HashMap<String,
Arc<dyn Actor>>> did not like mallocsizeof (even with trait bound on
Actor)
- Implement MallocSizeOf for BTreeSet.
- Implement MallocSizeOf of 0 for AtomicU32 and TcpStream
- Ignore a couple of MallocSizeof for http::Method, http::HeaderMap and
json::Value
Signed-off-by: Narfinger <Narfinger@users.noreply.github.com>
Testing: Compilation is the test.
Fixes: Part of addressing https://github.com/servo/servo/issues/42453
---------
Signed-off-by: Narfinger <Narfinger@users.noreply.github.com>
This allows us to `console.log` objects from JS and have a preview of
them appear in the console.
Interacting with said preview doesn't work yet, because the devtools
object actor is a stub.
<img width="600" height="78" alt="image"
src="https://github.com/user-attachments/assets/d896471f-3982-4406-94d4-b13eebabe337"
/>
Testing: This change adds a test
---------
Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>
There were some `#[serde(untagged)]` and `#[serde(skip*)]` annotations
in the shared devtools types. These are needed for correct JSON
serialization when sending messages to Firefox, but they fail in
multiprocess mode since we are sharing them through the IPC channel with
bincode, which doesn't support certain serde tags.
The workaround is to keep all shared types free from problematic
annotations, and defining wrapping types that are only used in the
devtools crate.
One instance are console messages and page errors. They have been moved
to `console.rs`, with `shared/devtools` only keeping the parts that are
needed for communication between threads.
The other instance are auto margins. Now the final message is built in
`page_style.rs`.
Apologies since both of them were introduced by me, I wasn't aware that
we were limited by bincode in multiprocess mode. A warning has been
added to `shared/devtools` listing the problematic annotations that
shouldn't be used in this file.
Testing: `mach test-devtools` and manual testing using `--multiprocess`.
Fixes: #42170
Signed-off-by: eri <eri@igalia.com>
Add an event listener for `clearBreakpoint` to `debugger.js` and the
necessary glue to access it from the `devtools` crate.
Testing: `./mach test-devtools` and manual testing.
Fixes: Part of: https://github.com/servo/servo/issues/36027
---------
Signed-off-by: atbrakhi <atbrakhi@igalia.com>
Co-authored-by: eri <eri@igalia.com>
Handle `enumProperties`, `enumSymbols` and `prototype` messages.
Create `PropertyIteratorActor` and `SymbolIteratorActor`.
This shows hover information when paused in the debugger, but only for
built-in Javascript objects. Next step would be to fix `evaluateJSAsync`
to take into account the frame / context in which we are evaluating
messages, so that we can do the same for local variables and functions.
Testing: Ran `mach test-tidy` and manual testing.
Part-of: #36027
@atbrakhi
Signed-off-by: eri <eri@igalia.com>
Co-authored-by: atbrakhi <atbrakhi@igalia.com>
Add an event listener for `pause` to `debugger.js` and the necessary
glue to access it from the `devtools` crate. This returns important
information to know where we are paused, such as the source location and
frame state.
Fix frame and object actor encoding into messages. Use information from
`debugger.js` to correctly fill the fields.
Add a new `frames` list to the thread actor and handle the `frames`
message.
Fix `getEnvironment` reply in the frame actor. It is out of form (has a
`type` field but it doesn't require a followup empty message, it already
counts as a reply), so we need to handle it specially.
Note: For now we are focusing on the protocol side of the debugger, and
this patch only shows where the pause would happen. Pausing Servo itself
will happen in a followup.
Testing: `mach test-devtools` and manual testing. No errors (apart from
#42006).
Part of: #36027
---------
Signed-off-by: eri <eri@igalia.com>
Co-authored-by: atbrakhi <atbrakhi@igalia.com>
Co-authored-by: Josh Matthews <josh@joshmatthews.net>
Make the `start` and `end` parameters in
`getBreakpointPositionsCompressed` optional.
Send a resource array even if it is empty.
Testing: Ran `mach test-devtools`.
---------
Signed-off-by: eri <eri@igalia.com>
Listen for `setBreakpoint` on `debugger.js` and add the relevant WebIDLs
and Servo counterparts to trigger this event and notify SpiderMonkey.
Implement `find_source` for `SourceManager` and `find_offset` for
`SourceActor`.
Testing: Manual testing and `./mach test-devtools` (note, the latter
seems to have some failing tests, we are investigating this, but this
patch doesn't add any new failure).
Fixes: Part of #36027
---------
Signed-off-by: eri <eri@igalia.com>
Co-authored-by: atbrakhi <atbrakhi@igalia.com>
Fix caching before the console is opened and stop sending messages
prematurely.
Fix line numbers not showing in console messages because of a missing
`rename_all`.
Remove `getCachedMessages` in favour of sending a list of messages as a
reply to the `watchResources` for `console-message`/`error-message` in
the watcher.
Remove `startListeners` and `stopListeners`. These are legacy methods of
watching properties before the watcher actor. It is preferred to enable
properties in `supported_resources` in the watcher than to use these
messages.
Remove `clearMessagesCache`, only `clearMessagesCacheAsync` seems to be
used now. Add a reply to `clearMessagesCacheAsync`.
Simplify a bit the structs for console messages and prefer serde's
annotations to manual serialization. Merge `handle_console_message` and
`handle_page_error`, and improve the usability of `ConsoleResource`.
Fix some fields in console messages. We are missing `source_id` for now.
This will be easier to add after better support for source actors. We
are also missing stack traces.
| Before | After |
| --- | --- |
| 
| 
|
Testing: Manual testing
Fixes: #26666
---------
Signed-off-by: eri <eri@igalia.com>
Co-authored-by: Martin Robinson <mrobinson@igalia.com>
This change is to make visibility uniformly crate wide across all of the
structs and their props within devtools.
Testing: Tested using `.\mach build -d` `.\mach fmt` `.\mach test-tidy`
all passed
Part of: #41893
---------
Signed-off-by: Seiran <bo646ru@gmail.com>
A warning has been showing up every time the DevTools client was
connected in Firefox Browser Console. Now not only the watcher can watch
resources, the root actor can too. Firefox was complaining about
[`extensions-backgroundscript-status`](https://searchfox.org/firefox-main/source/devtools/server/actors/resources/extensions-backgroundscript-status.js).
We don't have extensions running in the background, but even if we set
it to false, the warning persists. So instead this patch adds
boilerplate to "handle" this watch request.
Testing: Manual testing
---------
Signed-off-by: eri <eri@igalia.com>
Co-authored-by: Martin Robinson <mrobinson@igalia.com>
Replace `RefCell` with `AtomicRefCell` for structs implementing Actor,
making them `Sync`.
Consolidate `register` and `register_later` into a single function,
removing the need to wait for a loop before accessing newly created
actors.
Now `ActorRegsitry` has improved locking. Instead of locking the entire
struct, each member can be locked separately. Additionally, since `find`
now returns `Arc`, we can `find` and `register` multiple actors
depending on each other, since the lock is only needed for the operation
and we can keep the reference after that.
Depends on: #41741, #41744
Testing: Manual testing
---------
Signed-off-by: eri <eri@igalia.com>
Co-authored-by: Martin Robinson <mrobinson@igalia.com>
Remove `find_mut` from `ActorRegistry`. Force that all actors must be
immutable after being inserted in the registry, only allowing changes
through internal mutability.
First step in refactoring `ActorRegistry` to make it more reliable,
easier to use and less error prone.
Depends on #41741. `NetworkEventActor` was more complicated to refactor
and it needed special care so it is split into its own change.
Testing: This patch doesn't change behaviour.
Signed-off-by: eri <eri@igalia.com>
Now `NetworkEventActor`'s methods take a non mutable reference to self
(`add_request`, `add_response` and `add_security_info`). This actor now
has three different `RefCells` inside (`request`, `response` and
`security`), which group pieces of data that are updated separately via
the public methods.
For the actor functionality itself, it now follows Firefox more closely.
In the `resource-available` message it only sends the `*_available`
fields (i.e. `response_headers_available`), deferring the actual data to
the `get*` messages (i.e `getResponseHeaders`). There were also a few
slight changes I noticed along the way that I tried to fix.
Simplify some repeated logic calculating header sizes and listing
cookies.
`resource_updates` is updated to use `Serialize` structs instead of
manually adding values to a map. This is more in line with the rest of
the DevTools code and probably less prone to future errors.
Fix "Resource of network-event is missing a browsingContextID or
innerWindowId attribute" error that shows multiple times each time the
Network tab is open. Updated all messages to use the correct
browsingContextID.
Testing: Manually compared all of the Network tab features to avoid
regressions.
---------
Signed-off-by: eri <eri@igalia.com>
Co-authored-by: Martin Robinson <mrobinson@igalia.com>
Replace the previously hardcoded "name prefix" strings for actors with
[`type_name`](https://doc.rust-lang.org/std/any/fn.type_name.html).
Note that we aren't using the prefix directly as an unique identifier,
and `type_name` wouldn't be suitable for that. The suffix is an
incremental counter shared across all actors, so that alone is enough to
differentiate them.
The purpose of the prefix is to visually inspect the logs and see what
actors are involved. With this change the prefix will be slightly
different: "InspectorActor11" vs "inspector11", but that shouldn't
affect behaviour.
The eventual goal is to remove `new_name` and force the use of
`register/register_later` to create an actor. This is however a bit more
complicated, see #41768.
While I'd love to add `base_name` directly to the `Actor` trait, that
would unfortunately mean that it wouldn't be [dyn
compatible](https://doc.rust-lang.org/reference/items/traits.html#dyn-compatibility).
There are workarounds for this like separating the trait in two and
implementing it automatically, but I feel that would be too convoluted.
Testing: Manual testing, this patch shouldn't change behaviour.
Part of: #41768
Signed-off-by: eri <eri@igalia.com>
Fix a panic when calling `determine_auto_margins` for nodes without
`.style()`. Now it returns an empty `AutoMargins` (all options set to
`None`), which matches the behaviour in Firefox:
```json
"autoMargins": {}
"autoMargins": {"top": "auto", "bottom": "auto", "left": "auto", "right": "auto"}
```
Refactor `GetLayoutReply` and `ComputedNodeLayout` to take advantage of
serde's flatten feature, making the serialization much cleaner.
Testing: Manual testing with the example provided in the issue.
Fixes: #41743
---------
Signed-off-by: eri <eri@igalia.com>
The `ActorRegistry::shareable` field is a copy of the
`Arc<Mutex<ActorRegistry>>` in `DevtoolsInstance`. It seemed to only be
used for the timeline actor. That adds complexity to `ActorRegistry` for
only one actor (which we aren't even using at the moment, the entire
`timeline.rs` is marked as dead code), and it is possible to avoid it by
storing the `Arc<Mutex<..>>` copy directly on the timeline actor when it
is created.
The same goes for `start_stamp`, it is only used in the timeline and it
is ok to have it there instead. Since (if used) the timeline would have
to be created alongside the registry there wouldn't be a noticeable time
difference.
Rename the field in `DevtoolsInstance` from `actors` to `registry` for
consistency with (most) of the other code in the crate.
Testing: Manual testing, but this patch doesn't modify behaviour.
Signed-off-by: eri <eri@igalia.com>
Implement `ActorEncode` for `NetworkEventActor` to be more in line with
the rest of actors
Testing: This change does not modify behaviour
Signed-off-by: eri <eri@igalia.com>
Inspector sub-actors (walker, highlighter and page-style) had an overly
complicated creation mechanism using `RefCell<Option<...>>` and
instantiating these actors afterwards. Since the client always asks for
the three actors and instantiating them isn't that expensive, we aren't
saving anything from delaying this, and it introduces unnecessary
complexity.
For `*Msg`, make inspector sub-actors follow the rest and implement
`ActorEncode` instead of manually creating the struct in
`handle_message`.
Note that this change doesn't fix the issue with navigation/reloading
breaking the inspector. I am planning to send a patch for that, but it
is cleaner to separate this refactor and merge it before.
Testing: Manually test that the inspector still works.
Signed-off-by: eri <eri@igalia.com>
Changed most #[allow]s to #[expect]s, mainly for
clippy::too_many_arguments
Removed unfulfilled expectations
This is my first opensource contribution, so please let me know if
anything should
be done differently.
Testing: Refactor
Part of: #40838
---------
Signed-off-by: Tim van der Lippe <tvanderlippe@gmail.com>
Signed-off-by: TimurBora <timurborisov5561@gmail.com>
Co-authored-by: Tim van der Lippe <TimvdLippe@users.noreply.github.com>
Replace `allow` with `expect` lints for `unused`, `unsafe_code`,
`dead_code`, and `non_upper_case_globals`.
Testing: So far just check it compiled on `x86_64-linux` on NixOS. Need
to use the module `system.fontconfig.enable = true;` I think in my NixOS
config.
Part of: #40383
Searching `allow\(.*\)` for `.rs` files shows the following. for
(total_results:total_files) went from `707:386` to `675:368`, a
reduction of `32:18`.
How many files is too many files per PR? I feel like the 20-30 I have is
too big.
---------
Signed-off-by: Wayne Van Son <waynevanson@gmail.com>
For all upcoming and already-processed deferred fetches, we now
implement a quota. We don't support policy headers yet to configure the
quota, hence we use the default quota. That's why we have some tests
failing again.
It involves computing which document to compute the quota for, as well
as computing total length of requests. Additionally, the DevTools
computation for headers now uses the same logic for headers as deferred
fetches.
Signed-off-by: Tim van der Lippe <tvanderlippe@gmail.com>
These commits allow the frontend to open the devtools settings panel
successfully and allow the network monitor to function when the "Persist
Logs" frontend option is enabled.
Testing: Manually tested. No harness for automated tests for the network
monitor exist yet.
Fixes: #41196
---------
Signed-off-by: Josh Matthews <josh@joshmatthews.net>
First PR for #40232.
Implement the getSecurityInfo protocol message to allow DevTools clients
to inspect TLS connection details for HTTPS requests, including:
- Protocol version (TLS 1.2/1.3)
- Cipher suite
- Key exchange group
- ALPN protocol
- HSTS and ECH status
- Security state
Security state is simplified (secure/insecure only) and certificate
details are stubbed out; both will be completed in follow-up work.
Signed-off-by: jiang1997 <jieke@live.cn>
Update the target spec version from 133 to 145. Solves a few issues with
incorrect or outdated messages, such as an error with `reflow`.
The only meaningful change I could find is that the performance actor is
now called "perf" instead of "performance". It is part of a set of
global actors that the root handles. I made it more explicit and moved
the global actor registration inside of the root actor, it makes more
sense to have it there.
Finally, I ordered the message handling alphabetically. The only changed
entry is `getRoot` because of the global actors.
Testing: I manually tested the functionalities and checked the logs.
Signed-off-by: eri <eri@igalia.com>
Unify various `encodable` and `encode` methods from actors in an
`ActorEncode` trait, making it more consistent.
Add an `ActorRegistry::encode` method that finds an actor by name and
then returns its serialized version.
Add an `ActorMsg` struct to avoid creating repeated structs for simple
serializations.
There are two exceptions: `EncodableConsoleMessage` and `NodeInfo`.
These also have an `encode` method, but they are not actors, so
including them in this trait doesn't make much sense. Besides, they have
special requirements so it is better to keep them separate.
Testing: Manual testing in `about:debugging`.
Signed-off-by: eri <eri@igalia.com>
Switch the devtools part to GenericCallback and GenericSender.
To keep the diff small the names where not changes as a Sender almost
fills the same requirement as a callback.
Testing: As this is mostly type changes, the compilation is the test but
also devtools seem to work fine with these changes. ./mach try run is
here: https://github.com/Narfinger/servo/actions/runs/19931697694
---------
Signed-off-by: Narfinger <Narfinger@users.noreply.github.com>
Some cleanup to avoid having to specify an empty `handle_message` method
if it is not implemented or not necessary. Removed the TODOs from
Environment and Pause since they don't respond to any message.
Testing: This patch doesn't change behaviour.
Signed-off-by: eri <eri@igalia.com>
Small refactor to reduce verbosity when creating actors. Since they
always need to be boxed, do that in `register{_later}` instead. I also
added the `Send` trait to `Actor` directly since it is always needed.
Testing: This patch doesn't change behaviour.
Signed-off-by: eri <eri@igalia.com>
Prequisite for implementing pausing in the debugger. This will be part
of the `ThreadActor` response to `interrupt`. Continuation of #38824.
Depends on #40787 for `EnvironmentActor*`.
Testing: Deferred until the pause is fully implemented.
Fixes: Part of #36027.
---------
Signed-off-by: eri <eri@igalia.com>
Prequisite for implementing pausing in the debugger. The
`EnvironmentActor` will be used in the future from the `FrameActor`, we
need the struct defined to start to implement it.
The `ObjectActorMsg` is not only used by the `EnvoironmentActor`, but
also by the `ThreadActor` when replying to `interrupt`.
Testing: Deferred until the pause is fully implemented.
Fixes: Part of #36027.
---------
Signed-off-by: eri <eri@igalia.com>
Prequisite for implementing pausing in the debugger. This will be part
of the `ThreadActor` response to `interrupt`. I decided to split the
change in multiple patches since it is quite big. Continuation of
#38824.
Testing: Deferred until the pause is fully implemented.
Fixes: Part of #36027.
Signed-off-by: eri <eri@igalia.com>
This implements character set restrictions both for the DOM API and when
getting cookies from http/ws headers. This is a local workaround for
https://github.com/rwf2/cookie-rs/issues/243
We still fail some tests because hyper errors out when parsing headers
with %x1 characters.
This patch also makes a minor change to
'ServoCookie::from_cookie_string()' to avoid some string cloning when
possible.
Testing: wpt tests expectations are updated
Signed-off-by: webbeef <me@webbeef.org>
Adds a wrapper type for vectors that can contain large response bodies
to prevent flooding debug logs with the contents of those bodies.
Testing: Can't test debug log output.
Fixes: #37769
---------
Signed-off-by: Josh Matthews <josh@joshmatthews.net>
