Skip to content

libservo: Expand the UserContentManager API.#42288

Merged
mukilan merged 1 commit intoservo:mainfrom
mukilan:user-content-api-enhancements
Feb 5, 2026
Merged

libservo: Expand the UserContentManager API.#42288
mukilan merged 1 commit intoservo:mainfrom
mukilan:user-content-api-enhancements

Conversation

@mukilan
Copy link
Copy Markdown
Member

@mukilan mukilan commented Feb 2, 2026

The patch adds the following functionality to the per-WebView UserContentManager API.

  • Removing a UserScript at that was previously added.
  • Adding a new UserStyleSheet representing a user-origin style sheet. allow removing user script
  • Removing a previously added UserStyleSheet.

There might be scope for some improvements in the API:

  • UserScript and UserStyleSheet have different ways of representing the source location - a PathBuf and Url respectively. This is due to how those values are used by the underlying evaluation APIs in script and stylo. More investigation is needed here and could be addressed in future patches.

Testing: New unit tests are added for the user stylesheet APIs. Existing tests have been updated to test the removal of user scripts.

@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Feb 2, 2026
@mukilan mukilan force-pushed the user-content-api-enhancements branch from 7ae9516 to c7b8b4c Compare February 4, 2026 12:13
Copy link
Copy Markdown
Member

@mrobinson mrobinson left a comment

Choose a reason for hiding this comment

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

Looks good with a couple small things:

static USER_STYLE_SHEET_ID: AtomicU32 = AtomicU32::new(1);

#[derive(Clone, Copy, Debug, Deserialize, Eq, Hash, MallocSizeOf, PartialEq, Serialize)]
pub struct UserStyleSheetId(u32);
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.

I think make this private now as it's no longer necessary to expose it.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This type is referenced in the UserContentManagerAction enum that is used in the embedder->constellation message, so this needs to be public.

Comment on lines +65 to +69
/// Returns the `UserStyleSheetId` of this `UserStyleSheet`. This can be used to remove the
/// user stylesheet from the `UserContentManager`.
pub fn id(&self) -> UserStyleSheetId {
self.id
}
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.

I would remove this as the embedder doesn't need to know about the id (at least yet).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

True, but it is currently used by constellation to filter out the removed UserScripts and UserStyleSheets from the Constellation's state when it receives the UserContentManagerAction message due to a mutation.

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.

Ah, right. Maybe just use #[doc(hidden)] for this then.

static USER_SCRIPT_ID: AtomicU32 = AtomicU32::new(1);

#[derive(Clone, Copy, Debug, Deserialize, Eq, Hash, MallocSizeOf, PartialEq, Serialize)]
pub struct UserScriptId(u32);
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.

Let's make this private.

@servo-highfive servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Feb 5, 2026
@mukilan mukilan force-pushed the user-content-api-enhancements branch from c7b8b4c to 5383c66 Compare February 5, 2026 09:57
@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Feb 5, 2026
@mukilan mukilan enabled auto-merge February 5, 2026 10:06
@mukilan
Copy link
Copy Markdown
Member Author

mukilan commented Feb 5, 2026

Thanks for the review @mrobinson! I've addressed the recent review comments and pushed the changes, but looks like you might need to approve the new changes again, as the PR doesn't seem to get added to the merge queue automatically.

@mukilan mukilan added this pull request to the merge queue Feb 5, 2026
@servo-highfive servo-highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Feb 5, 2026
github-merge-queue bot pushed a commit that referenced this pull request Feb 5, 2026
The patch adds the following functionality to the per-WebView
`UserContentManager` API.
- Removing a `UserScript` at that was previously added.
- Adding a new `UserStyleSheet` representing a user-origin style sheet.
allow removing user script
- Removing a previously added `UserStyleSheet`.

There might be scope for some improvements in the API:
- `UserScript` and `UserStyleSheet` have different ways of representing
the source location - a `PathBuf` and `Url` respectively. This is due to
how those values are used by the underlying evaluation APIs in script
and stylo. More investigation is needed here and could be addressed in
future patches.

Testing: New unit tests are added for the user stylesheet APIs. Existing
tests have been updated to test the removal of user scripts.
@mrobinson
Copy link
Copy Markdown
Member

@mukilan Very odd. That wasn't a requirement before. This seems like it's a change to GitHub, a GitHub bug, or a change in our project configuration that I didn't know about. In any case, it's in the MQ now.

@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 5, 2026
@servo-highfive servo-highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Feb 5, 2026
@mukilan mukilan force-pushed the user-content-api-enhancements branch from 5383c66 to b13e136 Compare February 5, 2026 13:16
@servo-highfive servo-highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-tests-failed The changes caused existing tests to fail. labels Feb 5, 2026
@mukilan mukilan enabled auto-merge February 5, 2026 13:16
The patch adds the following functionality to the per-WebView
`UserContentManager` API.
- Removing a `UserScript` at that was previously added.
- Adding a new `UserStyleSheet` representing a user-origin style sheet.
allow removing user script
- Removing a previously added `UserStyleSheet`.

There might be scope for some improvements in the API:
- `UserScript` and `UserStyleSheet` have different ways of representing
  the source location - a `PathBuf` and `Url` respectively. This is due
  to how those values are used by the underlying evaluation APIs in
  script and stylo. More investigation is needed here and could be
  addressed in future patches.

Testing: New unit tests are added for the user stylesheet APIs.
Exisiting tests have been updated to test the removal of user scripts.

Signed-off-by: Mukilan Thiyagarajan <[email protected]>

review comments
@mukilan mukilan force-pushed the user-content-api-enhancements branch from b13e136 to b4f6e09 Compare February 5, 2026 13:17
@mukilan mukilan disabled auto-merge February 5, 2026 13:17
@mukilan mukilan enabled auto-merge February 5, 2026 13:17
@mukilan mukilan added this pull request to the merge queue Feb 5, 2026
@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 Feb 5, 2026
Merged via the queue into servo:main with commit 79ed814 Feb 5, 2026
29 checks passed
@mukilan mukilan deleted the user-content-api-enhancements branch February 5, 2026 14:12
@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 Feb 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-awaiting-review There is new code that needs to be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants