Skip to content

libservo: Introduce SiteDataManager and NetworkManager - #41255

Merged
mrobinson merged 3 commits into
servo:mainfrom
janvarga:initial-site-data-manager
Dec 16, 2025
Merged

mrobinson merged 3 commits into
servo:mainfrom
janvarga:initial-site-data-manager

Conversation

@janvarga

@janvarga janvarga commented Dec 14, 2025 •

Copy link
Copy Markdown
Member

This PR introduces an initial SiteDataManager abstraction intended
primarily for embedders to inspect and manage site data.

At this stage, SiteDataManager supports clearing network cache and
cookies. Cookie related functionality has been moved here from the
former CookieManager, which has been renamed to NetworkManager to
better reflect its scope.

Additional site data APIs will be added in follow-up PRs.

Testing: Added a new integration test with a TODO comment.

@janvarga
janvarga force-pushed the initial-site-data-manager branch from 79a0610 to dfb788a Compare December 15, 2025 09:41
Comment thread components/servo/site_data_manager.rs
Comment thread components/servo/site_data_manager.rs
@janvarga
janvarga marked this pull request as ready for review December 15, 2025 09:47
@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Dec 15, 2025

@mrobinson mrobinson left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a question: does it makes sense for the CookieManager to become a NetworkManager data structure or to combine it into the SiteDataManager. I'm starting to be convinced that cookies are also site data as well. What do you think?

Would you be willing to combine the two data structures into a single SiteDataManager?

@janvarga

Copy link
Copy Markdown
Member Author

Just a question: does it makes sense for the CookieManager to become a NetworkManager data structure or to combine it into the SiteDataManager. I'm starting to be convinced that cookies are also site data as well. What do you think?

Would you be willing to combine the two data structures into a single SiteDataManager?

Yeah, now that I think about it again, it probably makes more sense to have just one manager. I was likely too influenced by WPE WebKit. That said, the planned protocol handlers probably do deserve a separate NetworkManager.

How about a new follow-up PR where I rename CookieManager to NetworkManager and move clear_cookies to SiteDataManager?

@janvarga

Copy link
Copy Markdown
Member Author

How about a new follow-up PR where I rename CookieManager to NetworkManager and move clear_cookies to SiteDataManager?

I can also do that in this PR, if that's preferred.

@mrobinson

Copy link
Copy Markdown
Member

I can also do that in this PR, if that's preferred.

Maybe it would indeed be better to do it all in one go here to avoid code churn. I think you could also combine the cookie.rs test into the new test file as well.

Thanks!

@janvarga
janvarga marked this pull request as draft December 15, 2025 18:03
@janvarga
janvarga force-pushed the initial-site-data-manager branch from d7ad04f to 4802a91 Compare December 15, 2025 18:30
@janvarga
janvarga marked this pull request as ready for review December 15, 2025 18:36
@janvarga

Copy link
Copy Markdown
Member Author

Ok, added a new commit for that.

@mrobinson mrobinson left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

One nit to address before landing:

Let's name the Rust files after the data structure they expose, so site_data.rs would be site_data_manager.rs and net.rs would be network_manager.rs. This is just to be consistent with the API Rust files we already have.

@servo-highfive servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Dec 16, 2025
@janvarga
janvarga force-pushed the initial-site-data-manager branch from 4802a91 to 0e6b40f Compare December 16, 2025 11:51
@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Dec 16, 2025
@janvarga
janvarga force-pushed the initial-site-data-manager branch from 0e6b40f to c4aaa93 Compare December 16, 2025 11:51
@janvarga

Copy link
Copy Markdown
Member Author

Let's name the Rust files after the data structure they expose, so site_data.rs would be site_data_manager.rs and net.rs would be network_manager.rs. This is just to be consistent with the API Rust files we already have.

Done (in a new commit)

@janvarga
janvarga force-pushed the initial-site-data-manager branch from c4aaa93 to 669786b Compare December 16, 2025 11:58
This patch introduces an initial `SiteDataManager` abstraction intended
primarily for embedders to inspect and manage site data.

At this stage, only clearing of network cache is supported. Additional
functionality will be added in follow-up PRs.

Signed-off-by: Jan Varga <jvarga@igalia.com>
…cookies` to `SiteDataManager`

This patch renames `CookieManager` to `NetworkManager` to better reflect its
future scope. It also moves `clear_cookies` to `SiteDataManager`, aligning
cookie clearing with other planned site data APIs.

No functional changes are intended.

Signed-off-by: Jan Varga <jvarga@igalia.com>
Signed-off-by: Jan Varga <jvarga@igalia.com>
@janvarga
janvarga force-pushed the initial-site-data-manager branch from 669786b to 5cd07af Compare December 16, 2025 11:58
@mrobinson mrobinson changed the title libservo: Introduce SiteDataManager for embedders libservo: Introduce SiteDataManager and NetworkManager Dec 16, 2025
@servo-highfive servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Dec 16, 2025
@mrobinson
mrobinson added this pull request to the merge queue Dec 16, 2025
@servo-highfive servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Dec 16, 2025
Merged via the queue into servo:main with commit 220bf6c Dec 16, 2025
30 checks passed
@servo-highfive servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Dec 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants