Commit Graph

236 Commits

Author SHA1 Message Date
atbrakhi
3ca83edb56 devtools: Implement PropertyIteratorActor boilerplate (#43319)
Implement `PropertyIteratorActor` boilerplate

This is prerequisite for implementing object property enumeration in the
debugger. It is part of popup preview work in debugger.

Testing: Should not change behavior 
Fixes: part of #36027

Signed-off-by: atbrakhi <atbrakhi@igalia.com>
2026-03-17 10:05:41 +00:00
Brent Schroeter
890ef58052 devtools: Use active pipeline and script channel in inspector actors (#43153)
The `InspectorActor` and its children now query the
`BrowsingContextActor` for the active pipeline ID and script sender
instead of storing their own copies, which become outdated on
navigation. Previously, the inspector remained permanently stuck to the
pipeline that was active upon launch. Now, connecting a fresh devtools
client session after navigation allows inspection of each
`BrowsingContext`'s active DOM.

Navigation within a continuous remote debugging session still breaks the
Firefox inspector panel. This is due to multiple compatibility issues
with the protocol implementation, which will be addressed in separate
PRs.

This PR does not touch `TimelineActor` or `FramerateActor`. These are
theoretically affected by the same issue as the `InspectorActor`, but in
practice neither is ever instantiated. It seems both have been
effectively unreachable code since 1aab10f2 and will require more
extensive work and testing that is beyond the scope of this change.

Testing: Updates to the `WalkerActor` and `BrowsingContextActor` are
unit tested via the new `test_walker_observes_new_dom_after_nav` case in
`devtools_tests.py`. There are no existing unit tests to reference for
the highlighter and style actors; these have been tested manually but
this PR does not introduce further automated tests. This PR also
improves handling of `tabNavigated` messages to simplify the unit test
written for 4c69d85.

---------

Signed-off-by: Brent Schroeter <contact@brentsch.com>
2026-03-13 10:13:55 +00:00
Simon Wülker
c0ff7c1fc9 Everywhere: Remove instances of clippy::redundant-clone (#43212)
This change fixes all instances where
[`clippy::redundant-clone`](https://rust-lang.github.io/rust-clippy/master/index.html?groups=complexity%2Ccorrectness%2Cnursery%2Csuspicious&levels=allow#redundant_clone)
would trigger. It's allowed by default

I've also changed the lint to warn-by-default for servo.

Testing: Covered by WPT

---------

Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>
2026-03-12 13:28:21 +00:00
eri
948859b2fe devtools: Allow registering environment actor from debugger.js (#43166)
This functionality will be used in a follow up patch to implement the
getEnvironment message.

Testing: Check `mach test-devtools` and manual test
Part of: #36027

Signed-off-by: eri <eri@igalia.com>
Co-authored-by: atbrakhi <atbrakhi@igalia.com>
2026-03-11 15:45:35 +00:00
atbrakhi
9a915ece22 devtools: Pass ListFrames back to devtools (#43039)
Pass `ListFrames` back to devtools

Testing: existing test passes
Fixes: part of #36027

Signed-off-by: atbrakhi <atbrakhi@igalia.com>
Co-authored-by: eri <eri@igalia.com>
2026-03-06 08:47:15 +00:00
atbrakhi
1b7ec96fb6 devtools: implement listing all frames (#43015)
This change implements listing all frames from youngest to oldest. This
is in order to send correct frame message from thread actor

This is needed for our upcoming work!

Testing: Current tests are passing + manual test
Fixes: #36027

Signed-off-by: atbrakhi <atbrakhi@igalia.com>
Co-authored-by: eri <eri@igalia.com>
2026-03-05 14:58:30 +00:00
eri
b2b27d857b devtools: Avoid creating unnecessary new frame actors (#42906)
Before we were creating a new frame actor each time we paused, even if
the frame object in debugger.js was the same. Now we avoid this by
reusing the same frame actor id.

This helps our upcoming work on onStep, onPop, and onEnterFrame hooks.

Testing: Ran `mach test-devtools` and manually check that it works
Part of: #36027

Signed-off-by: eri <eri@igalia.com>
Co-authored-by: atbrakhi <atbrakhi@igalia.com>
2026-02-27 14:34:18 +00:00
atbrakhi
433d0acd0e devtools: Make why attribute use PauseReasons (#42878)
devtools: Make `why` attribute use `PauseReasons`

Currently we have why attribute hardcoded as per the event. In this PR
we introduce `PAUSE_REASON` and make sure `why` attribute is using
`PAUSE_REASONS`.

This is in order to support the upcoming work on `onStep`, `onPop`, and
`onEnterFrame` hook.

Testing: all existing tests are passing
Fixes: Part of #36027

Signed-off-by: atbrakhi <atbrakhi@igalia.com>
Co-authored-by: eri <eri@igalia.com>
2026-02-27 09:22:33 +00:00
eri
a2f1c55676 devtools: Register frame actor before pause (#42844)
We need a map between frame actor ids (from devtools) and frame objects
(from debugger.js) to implement stepping hooks in the future.

To achieve this, we register the frame actor with a call to the devtools
before entering the event loop when the debugger is paused. Instead of
creating the frame actor in `handle_debugger_pause`, we create it
before.

Testing: It passes existing devtools tests
Part of: #36027

Signed-off-by: eri <eri@igalia.com>
Co-authored-by: atbrakhi <atbrakhi@igalia.com>
2026-02-26 09:24:04 +00:00
eri
92259af702 devtools: Unify pause handling for breakpoints and interrupt (#42599)
The pause debugger screen should be shown for both pausing manually
(interrupt) and hitting a breakpoint.

Reuse the logic for pausing breakpoints to pause the debugger when the
user manually clicks the pause button.

Rename the pause event to interrupt to match the language of the
DevTools client and to avoid confusion with paused frames, which can
happen on interrupt or on a breakpoint.


https://github.com/user-attachments/assets/ceb0007d-0e57-44d6-a159-55980ff8b517

Testing: New DevTools test and manual testing.
Part of: #36027

cc @atbrakhi

---------

Signed-off-by: eri <eri@igalia.com>
Co-authored-by: atbrakhi <atbrakhi@igalia.com>
2026-02-23 09:29:59 +00:00
Narfinger
00fa5d3088 devtools: Allow specifiying an address to listen to and default to localhost (#42502)
Previously we listened to 0.0.0.0. which means any connection coming to
a specific port. That seems a bit ill advised as not everybody has a
good firewall setup. Now we default listen only on 127.0.0.1 but
optionally can describe a full SocketAddr such as "192.168.1.23:1234".

Side note: Cleaned up the ipc-channel syntax.

Signed-off-by: Narfinger <Narfinger@users.noreply.github.com>

Testing: Currently we don't have an automatic way to test this. Manually
run devtools and it connects.

---------

Signed-off-by: Narfinger <Narfinger@users.noreply.github.com>
2026-02-19 03:24:56 +00:00
atbrakhi
3c8c46d32f devtools: implement pause and resume in debugger (#42580)
When a breakpoint is hit, the script thread now pauses execution and
notifies devtools clients with a "paused" event. The script
thread enters a loop that processes devtools messages until
a Resume command is received.

This change does not implement manual pause

Testing: Added new test
Fixes: part of https://github.com/servo/servo/issues/36027




https://github.com/user-attachments/assets/c619db20-4579-4f77-aa60-0e43e6e7e575

Signed-off-by: atbrakhi <atbrakhi@igalia.com>
2026-02-18 10:08:46 +00:00
Simon Wülker
a60f9370c7 devtools: Apply attribute modifications in the inspector to the DOM tree (#42601)
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>
2026-02-17 13:05:41 +00:00
Simon Wülker
53ffdda2a1 devtools: Properly handle opening and closing client connections (#42583)
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>
2026-02-13 14:39:47 +00:00
Narfinger
2b6a260125 devtools: Plugin to about:memory (#42480)
This implements the final glue to have memory reporting for devtools.

Testing: Run some webpages and connected to devtools and looked at
about:memory
Fixes: https://github.com/servo/servo/issues/42453

---------

Signed-off-by: Narfinger <Narfinger@users.noreply.github.com>
2026-02-11 13:23:05 +00:00
Narfinger
870576f948 devtools: Implement MallocSizeOf for DevtoolsInstance (#42478)
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>
2026-02-10 17:32:47 +00:00
Simon Wülker
60c8b8fad5 devtools: Support sending js objects to firefox devtools from console.log (#42296)
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>
2026-02-10 10:31:17 +00:00
eri
6e781aaf74 devtools: Remove unsupported serde annotations from shared/devtools.rs (#42219)
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>
2026-01-28 15:48:59 +00:00
eri
151074e9a1 devtools: Handle pause in the debugger (#42007)
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.

![Debugger showing the line where execution would be
paused](https://github.com/user-attachments/assets/c007f205-0ccd-47f1-ad0b-81b7415e8211)

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>
2026-01-19 19:27:52 +00:00
eri
8cf4955731 devtools: Correctly cache console messages (#41895)
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 |
| --- | --- |
| ![Console message list starting at 5 and counting up, showing the file
location but not the line
number](https://github.com/user-attachments/assets/b21159b4-1a95-46aa-9337-0004a922837c)
| ![Console message list starting at 1, showing both the file location
and line number. It says "Connected here" at message number
4](https://github.com/user-attachments/assets/f8ea1a7c-9262-4fa2-a882-cad35af9c2dc)
|

Testing: Manual testing
Fixes: #26666

---------

Signed-off-by: eri <eri@igalia.com>
Co-authored-by: Martin Robinson <mrobinson@igalia.com>
2026-01-15 21:41:07 +00:00
SeiRan
375c4f722b devtools: Restrict visibility of actors in devtools (#41935)
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>
2026-01-15 21:39:17 +00:00
atbrakhi
0112ffa812 devtools: Register workers in workers list, not tabs (#41929)
Devtools tests are failing on `./mach test-devtools`. Regression from
#41744

Testing: Manually tested

Signed-off-by: atbrakhi <atbrakhi@igalia.com>
2026-01-15 11:41:32 +00:00
eri
41db8d9111 devtools: Consolidate register/register_later (#41796)
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>
2026-01-13 19:24:30 +00:00
eri
e95afbf7cf devtools: Remove find_mut and make all actors immutable (#41744)
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>
2026-01-13 08:58:34 +00:00
eri
d61b7c934a devtools: Compute Actor base name per type (#41769)
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>
2026-01-09 15:26:00 +00:00
eri
f0233c3f0e devtools: Implement Default for ActorRegistry (#41795)
Small tweak to use `Default` instead of new for `ActorRegistry`.

Testing: This patch doesn't change behaviour

Signed-off-by: eri <eri@igalia.com>
2026-01-09 10:29:11 +00:00
eri
ffb254c5d5 devtools: Remove ActorRegistry shareable and start_stamp (#41767)
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>
2026-01-09 01:14:30 +00:00
atbrakhi
ae6f56f194 Separate console.clear() from ConsoleLogLevel (#41495)
Separate `console.clear()` from `ConsoleLogLevel`. See
https://github.com/servo/servo/pull/41311#discussion_r2622642820

Testing: Tested locally. Unfortunately in DevTools tests we do not cover
console tab

Signed-off-by: atbrakhi <atbrakhi@igalia.com>
2025-12-23 14:27:04 +00:00
atbrakhi
f3488b25a8 Move devtools_traits::LogLevel to embedder_traits (#41311)
This PR moves devtools_traits::LogLevel to embedder_traits in
preparation of upcoming work on creating a ServoDelegate and
WebViewDelegate method for Console API output. Also see
https://github.com/servo/servo/issues/41145

Testing: only moves code, no functional change
Fixes: part of https://github.com/servo/servo/issues/41145

Signed-off-by: atbrakhi <atbrakhi@igalia.com>
2025-12-16 10:38:30 +00:00
jiang1997
f60735086d net/devtools: Expose TLS security info to DevTools network panel (#40567)
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>
2025-12-11 10:27:17 +00:00
eri
b5fa10050f devtools: Update to Firefox 145 (#41087)
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>
2025-12-10 05:41:04 +00:00
eri
deb172492e devtools: ActorEncode trait (#41139)
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>
2025-12-08 14:05:01 +00:00
Narfinger
6f62269c8c Devtools: Switch to GenericChannel and GenericCallback (#41051)
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>
2025-12-08 11:03:45 +00:00
eri
0ce97201fd devtools: Box the actor inside register (#41074)
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>
2025-12-05 12:53:17 +00:00
eri
879a69da6d devtools: Frame actor boilerplate (#40791)
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>
2025-11-24 01:16:00 +00:00
eri
4ea0f57a40 devtools: Add Environment actor boilerplate and Object actor encode (#40787)
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>
2025-11-22 09:28:14 +00:00
eri
fdd49e6da3 devtools: Pause actor boilerplate (#40786)
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>
2025-11-22 09:23:45 +00:00
Martin Robinson
c776475b3b Remove Servo's wrapper around rand (#39979)
This wrapper was added in order to eliminate the number of file
descriptors used accessing `/dev/urandom`, but these days `osrandom` and
by proxy `rand` will try to use `getrandom` on Linux and similar system
APIs on other platforms [^1].

This is a trial balloon for removing the wrapper, since almost all
modern Linux systems have `getrandom`  (available since Linux
3.17).

[^1]: https://docs.rs/getrandom/0.3.4/getrandom/#supported-targets

Testing: Should not change observable behavior (only in random ways), so
should
be covered by WPT tests.

Signed-off-by: Martin Robinson <mrobinson@igalia.com>
2025-10-18 13:54:20 +00:00
Narfinger
84465e7768 Removed FnvHash and transformed the rest to FxHashmap (#39233)
This should be the final PR for the Hash Function series that is
trivial.

Of note: I decided to transform `HashMapTracedValues<Atom,..>` to use
FxBuildHasher. This is likely not going to improve performance as Atom's
already have a unique u32 that is used as the Hash but it safes a few
bytes for the RandomState that is normally in the HashMap.

Signed-off-by: Narfinger <Narfinger@users.noreply.github.com>

Testing: Hash function changes should not change functionality, we
slightly decrease the size and unit tests still work.

Signed-off-by: Narfinger <Narfinger@users.noreply.github.com>
2025-09-10 13:34:54 +00:00
Jonathan Schwender
66d9f957e6 EmbedderMsg: port reply channels to GenericChannel (#39018)
This change ports all `EmbedderMsg` reply channels that don't use the
`ROUTER` to GenericChannel.
The remaining reply channels that use the router are blocked until
#38973 is merged.
This is a breaking change in the API between libservo and embedders.

Future work: A lot of the reply channels in this PR look like they
conceptually should be oneshot ipc channels. It might make sense to
provide a `OneshotGenericChannel` abstraction that encodes this.

Testing: No functional changes - covered by existing tests. None of the
channels changed here uses the Router
Part of #38912

---------

Signed-off-by: Jonathan Schwender <schwenderjonathan@gmail.com>
2025-08-29 12:44:21 +00:00
Usman Yahaya Baba
f2294db95b Split devtools network event logic for creating/retrieving network event actors (#38409)
`DevtoolsInstance::find_network_event_actor` silently creates a new
actor if there is not one already known for a given ID. This is
confusing; this PR separates logic for handling network requests (create
a new actor) and network responses (retrieve an existing actor).

Fixes: (https://github.com/servo/servo/issues/37841)

---------

Signed-off-by: uthmaniv <uthmanyahayababa@gmail.com>
2025-08-19 19:13:54 +00:00
Martin Robinson
8743a11ba4 tidy: Add a rule ensuring that // comments are followed by a space in Rust (#38698)
This shows up sometimes in code reviews, so it makes sense that tidy
enforces it. `rustfmt` supports this via comment normalization, but it
does many other things and is still an unstable feature (with bugs).

Testing: There are new tidy tests for this change.

Signed-off-by: Martin Robinson <mrobinson@igalia.com>
2025-08-18 12:09:09 +00:00
Usman Yahaya Baba
02dca0fb21 net: Send ResponseContentObj to Devtools (#38625)
Currently, the response tab for a request's detail in devtools does not
show the available data, this was due to how the content is being
structured (not the way firefox's devtools client expects it) and also
the body being discarded and not stored in the actor.
This PR stores the body in the actor , which is then retrieved in
`getResponseContent` and then use it to instantiate the new struct
`ResponseContentObj` which matches the format firefox's expects

Fixes: https://github.com/servo/servo/issues/38128

---------

Signed-off-by: uthmaniv <uthmanyahayababa@gmail.com>
Signed-off-by: Josh Matthews <josh@joshmatthews.net>
Co-authored-by: Josh Matthews <josh@joshmatthews.net>
2025-08-15 08:26:24 +00:00
shuppy
f5b631e270 devtools: Show clients where they can set breakpoints (#37667)
devtools clients query source actors to determine where the user can set
breakpoints in a source. there are two relevant requests here:
`getBreakableLines` controls which line numbers can be clicked in the
margin, and once a line number is clicked,
`getBreakpointPositionsCompressed` controls where to show breakpoint
buttons within that line.

this patch handles those requests by querying the [SpiderMonkey Debugger
API](https://firefox-source-docs.mozilla.org/js/Debugger/) for that
information:
- devtools sends its script thread a GetPossibleBreakpoints message for
the source’s
[`spidermonkey_id`](https://firefox-source-docs.mozilla.org/js/Debugger/Debugger.Source.html#id)
- the script thread fires a `getPossibleBreakpoints` event into its
debugger global
- the debugger script looks up the
[root](https://firefox-source-docs.mozilla.org/js/Debugger/Debugger.html#onnewscript-script-global)
[Debugger.Script](https://firefox-source-docs.mozilla.org/js/Debugger/Debugger.Script.html#getpossiblebreakpoints-query)
for that source, calls
[getPossibleBreakpoints()](https://firefox-source-docs.mozilla.org/js/Debugger/Debugger.Script.html#getpossiblebreakpoints-query),
and returns the result via
DebuggerGlobalScope#getPossibleBreakpointsResult()
- that method takes the pending result sender, and sends the result back
to devtools
- devtools massages the result into the format required by the request,
and replies to the client

as a result, users of the Firefox devtools client can now set
breakpoints, though they don’t have any effect.

Testing: this patch adds new devtools tests
Fixes: part of #36027

<img width="1433" height="1328" alt="image"
src="https://github.com/user-attachments/assets/f0cd31e0-742f-44d3-8c5d-ceedd9a2706d"
/>

---------

Signed-off-by: Delan Azabani <dazabani@igalia.com>
Co-authored-by: atbrakhi <atbrakhi@igalia.com>
2025-08-12 04:53:53 +00:00
shuppy
4784668fa9 devtools: Create source actors from Debugger API notifications (#38334)
currently our devtools impl creates source actors in script, when
executing scripts in HTMLScriptElement or DedicatedWorkerGlobalScope.
this approach is cumbersome, and it means that many pathways to running
scripts are missed, such as imported ES modules.

with the [SpiderMonkey Debugger
API](https://firefox-source-docs.mozilla.org/js/Debugger/), we can pick
up all of the scripts and all of their sources without any extra code,
as long as we tell it about every global we create (#38333, #38551).
this patch adds a [Debugger#onNewScript()
hook](https://firefox-source-docs.mozilla.org/js/Debugger/Debugger.html#onnewscript-script-global)
to the debugger script, which calls
DebuggerGlobalScope#notifyNewSource() to notify our script system when a
new script runs. if the source is relevant to the file tree in the
Sources tab, script tells devtools to create a source actor.

Testing: adds several new automated devtools tests
Fixes: part of #36027

Signed-off-by: Delan Azabani <dazabani@igalia.com>
Co-authored-by: atbrakhi <atbrakhi@igalia.com>
2025-08-11 06:04:51 +00:00
shuppy
c9541f2906 devtools: Expose introductionType to devtools clients (#38541)
in the devtools protocol, [source
forms](https://firefox-source-docs.mozilla.org/devtools/backend/protocol.html#loading-script-sources)
announced in `resources-available-array` messages can include the
`introductionType`, which more or less mirrors the field of the same
name in SpiderMonkey’s CompileOptions.

this patch exposes `introductionType` accordingly, allowing us to check
for the correct values in automated tests.

Testing: new coverage in devtools tests
Fixes: part of #36027

---------

Signed-off-by: Delan Azabani <dazabani@igalia.com>
Co-authored-by: atbrakhi <atbrakhi@igalia.com>
2025-08-08 12:20:30 +00:00
shuppy
a671c50bc9 devtools: Fix source contents tests and fix a race (#38359)
test_sources_list() relied on <https://servo.org/js/load-table.js> to
test scripts loaded from multiple origins, but that file was deleted a
couple of weeks ago. this patch adds a second internal web server, then
replaces that load with scripts loaded from two distinct origins:
<http://127.0.0.1:10000> and <http://127.0.0.1:10001>.

fixing that test revealed an impl bug where inline module scripts
containing `import` statements may never get their source contents
populated. this is because the logic for populating source contents for
inline scripts only applied to source actors created *before* we
finished parsing the page. we assumed that inline scripts always block
the parser, but this is not the case. this patch ensures that inline
source contents can be populated for source actors created after parse.

Testing: adds a new test,
test_source_content_with_inline_module_import_external()
Fixes: part of #36325

Signed-off-by: Delan Azabani <dazabani@igalia.com>
Co-authored-by: atbrakhi <atbrakhi@igalia.com>
2025-07-30 11:29:24 +00:00
atbrakhi
4ef15ec42f devtools: Make contentType optional in source actor source responses (#38330)
This is allowed by the [protocol
docs](https://firefox-source-docs.mozilla.org/devtools/backend/protocol.html#loading-script-sources),
and eliminates an unwrap().

Testing: does not get exercised yet, but will be used in #37667 
Fixes: part of #36027

Signed-off-by: Delan Azabani <dazabani@igalia.com>
Co-authored-by: Delan Azabani <dazabani@igalia.com>
2025-07-29 11:16:59 +00:00
Usman Yahaya Baba
ff02fdad6d Send early DevToolsHttpRequest and relocate response reporting to main_fetch (#37906)
This change refactors how we notify DevTools about network activity so
that all fetches (even those served from cache) appear correctly in the
Network panel, and so that DevTools sees request metadata as soon as
possible rather than waiting until the end of a full HTTP cycle.
- Before, we only send DevTools events inside http_network_fetch, so
cached responses (which skip that path) never show up. By emitting a
minimal HttpRequest event at the very start of main_fetch (with URL,
method, pipeline and browsing IDs), we guarantee every fetch shows up
immediately.
- Then, by moving HttpResponse notifications out of http_network_fetch
into main_fetch (right after process_response and process_response_eof),
we ensure DevTools gets status, header, and completion events for both
network and cache hits. Leveraging nullable fields in NetworkEventActor
lets us incrementally fill in timing, header, and body data later,
improving DevTools’ visibility.
Testing: Ran servo with `--devtools=6080` flag, cached responses now
appear in the network panel
Fixes: https://github.com/servo/servo/issues/37869

---------

Signed-off-by: Uthman Yahaya Baba <uthmanyahayababa@gmail.com>
2025-07-15 05:41:11 +00:00
atbrakhi
71d97bd935 Devtools: send error replies instead of ignoring messages (#37686)
Client messages, which are always requests, are dispatched to Actor
instances one at a time via Actor::handle_message. Each request must be
paired with exactly one reply from the same actor the request was sent
to, where a reply is a message with no type (if a message from the
server has a type, it’s a notification, not a reply).

Failing to reply to a request will almost always permanently break that
actor, because either the client gets stuck waiting for a reply, or the
client receives the reply for a subsequent request as if it was the
reply for the current request. If an actor fails to reply to a request,
we want the dispatcher (ActorRegistry::handle_message) to send an error
of type `unrecognizedPacketType`, to keep the conversation for that
actor in sync. Since replies come in all shapes and sizes, we want to
allow Actor types to send replies without having to return them to the
dispatcher.

This patch adds a wrapper type around a client stream that guarantees
request/reply invariants. It allows the dispatcher to check if a valid
reply was sent, and guarantees that if the actor tries to send a reply,
it’s actually a valid reply (see ClientRequest::is_valid_reply). It does
not currently guarantee anything about messages sent via the TcpStream
released via ClientRequest::try_clone_stream or the return value of
ClientRequest::reply. We also send `unrecognizedPacketType`,
`missingParameter`, `badParameterType`, and `noSuchActor` messages per
the
[protocol](https://firefox-source-docs.mozilla.org/devtools/backend/protocol.html#error-packets)
[docs](https://firefox-source-docs.mozilla.org/devtools/backend/protocol.html#packets).

Testing: automated tests all pass, and manual testing looks ok
Fixes: #37683 and at least six bugs, plus one with a different root
cause, plus three with zero impact

---------

Signed-off-by: atbrakhi <atbrakhi@igalia.com>
Signed-off-by: Delan Azabani <dazabani@igalia.com>
Co-authored-by: delan azabani <dazabani@igalia.com>
Co-authored-by: Simon Wülker <simon.wuelker@arcor.de>
Co-authored-by: the6p4c <me@doggirl.gay>
2025-07-07 12:40:44 +00:00