WebMCP: Queue declarative tool registration and schema calculation [chromium/src : main]
Dominic Farolino (Gerrit)
Dominic Farolino voted and added 1 comment![]( "Open in Gerrit")
Votes added by Dominic Farolino
| Commit-Queue | +1 |
1 comment
Found more things I could clean up. As a result of moving to a "queue a task to register/unregister declarative tool" model, I found I could remove all of the `toolchange` task queuing infra in model_context.cc, and move the input schema diffing from there, to the method in HTMLFormElement that performs tool registration.
Related details
- Mason Freed
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Fr (Gerrit)
Fr added 2 comments![]( "Open in Gerrit")
tool_data->RefreshDeclarativeInputSchema();Is this still needed?
navigator.modelContextTesting.ontoolchange = () => {Shall we use addEventListener instead there in case the test also uses navigator.modelContextTesting.ontoolchange which would be overridden IIUC?
Related details
- Dominic Farolino
- Mason Freed
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Philip Pfaffe (Gerrit)
Philip Pfaffe added 1 comment![]( "Open in Gerrit")
Dominic Farolino (Gerrit)
Dominic Farolino voted and added 2 comments![]( "Open in Gerrit")
Votes added by Dominic Farolino
| Commit-Queue | +1 |
2 comments
tool_data->RefreshDeclarativeInputSchema();Dominic FarolinoIs this still needed?
I tried to remove it but some other listTools() tests still rely on it for some subtle reason, and since I'm not focusing on that API as much in this CL, my goal is remove this after without piling onto this CL.
model_context_->NotifyToolChange();OK this crap is ugly! But necessary to get these unit tests to work since there is no browser process to fire `NotifyToolChange()` in unit tests, so we have to "simulate" one here. It's just a hack to get around that until we move all of these unit tests over to WPTs, which I'll do later (I don't want to block merging this PR back to M149 on this).
Related details
- Fr
- Mason Freed
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Dominic Farolino (Gerrit)
Dominic Farolino voted and added 1 comment![]( "Open in Gerrit")
Votes added by Dominic Farolino
| Commit-Queue | +1 |
1 comment
Shall we use addEventListener instead there in case the test also uses navigator.modelContextTesting.ontoolchange which would be overridden IIUC?
Sounds reasonable. Done.
Related details
- Fr
- Mason Freed
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Dominic Farolino (Gerrit)
Dominic Farolino voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
- Fr
- Mason Freed
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Dominic Farolino (Gerrit)
Dominic Farolino voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
- Fr
- Mason Freed
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mason Freed (Gerrit)
Mason Freed added 12 comments![]( "Open in Gerrit")
I think this looks great. Lots of comments, but most are very small.
to make them wait on `toolchange` being fired before accessing the tool.Kudos on the 75 line commit description. 😊
TaskHandle mcp_registration_task_handle_;nit: maybe just `mcp_registration_task_`?
Side note, I kind of hate that we mix and match "webmcp" and "mcp" everywhere. But let's not solve that now I guess.
// regard to `this`, or when a control's attributes are changed. It will only
// queue a task to register a new tool if `this` already has an existingI wonder if it would be possible to CHECK that it's only called if `this` has a tool?
if (!mcp_registration_task_handle_.IsActive()) {This makes me wonder - should we re-schedule it instead of leaving it where it is in the task queue? I can't think of why, but perhaps it's better that you push it out each time we have a new update? (If you can't think of a good reason either, then this is ok as-is.)
if (name == html_names::kToolnameAttr ||
name == html_names::kTooldescriptionAttr) {
if (params.old_value != params.new_value) {
ScheduleDeclarativeWebMCPToolRegistration();
}
}nit:
```
if ((name == html_names::kToolnameAttr ||
name == html_names::kTooldescriptionAttr) &&
(params.old_value != params.new_value)) {
ScheduleDeclarativeWebMCPToolRegistration();
}
```
void HTMLFormElement::ScheduleWebMCPSchemaUpdate() {
if (!RuntimeEnabledFeatures::WebMCPEnabled(GetExecutionContext())) {
return;
}
if (!IsValidWebMCPForm()) {
return;
}
ScheduleDeclarativeWebMCPToolRegistration();
}This is a pretty thin wrapper - perhaps we just remove it and update the few remaining callers to call `ScheduleDeclarativeWebMCPToolRegistration` directly?
model_context_->NotifyToolChange();OK this crap is ugly! But necessary to get these unit tests to work since there is no browser process to fire `NotifyToolChange()` in unit tests, so we have to "simulate" one here. It's just a hack to get around that until we move all of these unit tests over to WPTs, which I'll do later (I don't want to block merging this PR back to M149 on this).
Ack. 😊
async function waitForFormToolSchemaToMatch(expected_schema) {Maybe move this/these to the utility `.js` files?
navigator.modelContextTesting.ontoolchange = check;How come we can't switch these tests to `modelContext` and just use `waitForTool()`?
nit: extra blank lines. I'd get rid of both of them. This comment applies to most tests above.
Also, why not move `assert_true(!!navigator.modelContext);` into `waitForTool()`? Then these are all just one line additions.
Related details
- Dominic Farolino
- Fr
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Dominic Farolino (Gerrit)
Dominic Farolino voted and added 10 comments![]( "Open in Gerrit")
Votes added by Dominic Farolino
| Commit-Queue | +1 |
10 comments
nit: maybe just `mcp_registration_task_`?
Side note, I kind of hate that we mix and match "webmcp" and "mcp" everywhere. But let's not solve that now I guess.
Agreed. I have a personal goal to make everything consistently named in the future.
nit: maybe remove the extra blank line
Done, but I also just removed the last two text lines here anyways, since I think they were superfluous.
// regard to `this`, or when a control's attributes are changed. It will only
// queue a task to register a new tool if `this` already has an existingI wonder if it would be possible to CHECK that it's only called if `this` has a tool?
In the current state, no. That's what [this check protects against](https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/html/forms/html_form_element.cc;l=1632-1634;drc=c065efc537674efbf5ed6292606039bcc6a22f6d). A form control can become associated with a form that doesn't have `toolname`/`tooldescription` attributes, and therefore impact the form's input schema, since there is no tool. Does that make sense?
This makes me wonder - should we re-schedule it instead of leaving it where it is in the task queue? I can't think of why, but perhaps it's better that you push it out each time we have a new update? (If you can't think of a good reason either, then this is ok as-is.)
I can't think of a good reason either where it would *actually* matter, and while I kind of like the intuition of "pushing [the task] out" each time, I implemented and it looked a little weird:
- Either we pull the `.Cancel()` out unconditionally above the `if (!is_valid_mcp_form)` block, or;
- We duplicate it by keeping it right where it is in there, and ALSO adding it on the line that you made this comment, just before we re-queue.
I have thoughts on how to simplify and restructure this whole method, so I'll save those for another CL I think, and continue with the current not-pushing-the-task-out approach we have here. Thanks for the comment.
if (name == html_names::kToolnameAttr ||
name == html_names::kTooldescriptionAttr) {
if (params.old_value != params.new_value) {
ScheduleDeclarativeWebMCPToolRegistration();
}
}nit:
```
if ((name == html_names::kToolnameAttr ||
name == html_names::kTooldescriptionAttr) &&
(params.old_value != params.new_value)) {
ScheduleDeclarativeWebMCPToolRegistration();
}
```
Done
void HTMLFormElement::ScheduleWebMCPSchemaUpdate() {
if (!RuntimeEnabledFeatures::WebMCPEnabled(GetExecutionContext())) {
return;
}
if (!IsValidWebMCPForm()) {
return;
}
ScheduleDeclarativeWebMCPToolRegistration();
}This is a pretty thin wrapper - perhaps we just remove it and update the few remaining callers to call `ScheduleDeclarativeWebMCPToolRegistration` directly?
Hmm, it is a pretty thing wrapper. However, it does do less work, than `ScheduleDeclarativeWebMCPToolRegistration()`, which is why I assume it exists. For associating of every single for control element, regardless of WebMCP usage, this method runs, so it's best to have it stop after a simple pointer-is-null check (which is what `if (!IsValidWebMCPForm()) {` does), rather than entering `ScheduleDeclarativeWebMCPToolRegistration()` and grabbing attributes and running `isConnected()` all to do nothing in 99.9% of forms, where `is_valid_mcp_tool` is null.
But maybe the performance implications of entering that method aren't actually bad (the attribute getter does have "fast" in the name). What do you think? Basically I think this indirection exists for performance reasons, but I'm not sure how serious it is.
model_context_->NotifyToolChange();Mason FreedOK this crap is ugly! But necessary to get these unit tests to work since there is no browser process to fire `NotifyToolChange()` in unit tests, so we have to "simulate" one here. It's just a hack to get around that until we move all of these unit tests over to WPTs, which I'll do later (I don't want to block merging this PR back to M149 on this).
Ack. 😊
Done
async function waitForFormToolSchemaToMatch(expected_schema) {Maybe move this/these to the utility `.js` files?
Created a new `helpers.js` file in `external/wpt/webmcp/resources`, and referenced it from this file. But I think the wpt_internal/ and other tests cannot reference this file, so this is the only test that uses the helper. But once we remove the `modelContextTesting` API and move all the tests here, then others will be able to make use of it.
How come we can't switch these tests to `modelContext` and just use `waitForTool()`?
The only reason is because `modelContext` is gated on secure context, and this file is not HTTPs. But yeah we can rename it and use the new API accordingly. Done.
nit: extra blank lines. I'd get rid of both of them. This comment applies to most tests above.
Also, why not move `assert_true(!!navigator.modelContext);` into `waitForTool()`? Then these are all just one line additions.
Done, and I made it:
```
assert_implements(navigator.modelContextTesting, "modelContext is available");
```
which I think is slightly better.
Related details
- Fr
- Mason Freed
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mason Freed (Gerrit)
Mason Freed voted and added 6 comments![]( "Open in Gerrit")
Votes added by Mason Freed
| Code-Review | +1 |
6 comments
// regard to `this`, or when a control's attributes are changed. It will only
// queue a task to register a new tool if `this` already has an existingDominic FarolinoI wonder if it would be possible to CHECK that it's only called if `this` has a tool?
In the current state, no. That's what [this check protects against](https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/html/forms/html_form_element.cc;l=1632-1634;drc=c065efc537674efbf5ed6292606039bcc6a22f6d). A form control can become associated with a form that doesn't have `toolname`/`tooldescription` attributes, and therefore impact the form's input schema, since there is no tool. Does that make sense?
Yep, makes sense. Thanks.
if (!mcp_registration_task_handle_.IsActive()) {Dominic FarolinoThis makes me wonder - should we re-schedule it instead of leaving it where it is in the task queue? I can't think of why, but perhaps it's better that you push it out each time we have a new update? (If you can't think of a good reason either, then this is ok as-is.)
I can't think of a good reason either where it would *actually* matter, and while I kind of like the intuition of "pushing [the task] out" each time, I implemented and it looked a little weird:
- Either we pull the `.Cancel()` out unconditionally above the `if (!is_valid_mcp_form)` block, or;
- We duplicate it by keeping it right where it is in there, and ALSO adding it on the line that you made this comment, just before we re-queue.I have thoughts on how to simplify and restructure this whole method, so I'll save those for another CL I think, and continue with the current not-pushing-the-task-out approach we have here. Thanks for the comment.
Ok, this is good as-is. Thanks for thinking about it.
void HTMLFormElement::ScheduleWebMCPSchemaUpdate() {
if (!RuntimeEnabledFeatures::WebMCPEnabled(GetExecutionContext())) {
return;
}
if (!IsValidWebMCPForm()) {
return;
}
ScheduleDeclarativeWebMCPToolRegistration();
}Dominic FarolinoThis is a pretty thin wrapper - perhaps we just remove it and update the few remaining callers to call `ScheduleDeclarativeWebMCPToolRegistration` directly?
Hmm, it is a pretty thing wrapper. However, it does do less work, than `ScheduleDeclarativeWebMCPToolRegistration()`, which is why I assume it exists. For associating of every single for control element, regardless of WebMCP usage, this method runs, so it's best to have it stop after a simple pointer-is-null check (which is what `if (!IsValidWebMCPForm()) {` does), rather than entering `ScheduleDeclarativeWebMCPToolRegistration()` and grabbing attributes and running `isConnected()` all to do nothing in 99.9% of forms, where `is_valid_mcp_tool` is null.
But maybe the performance implications of entering that method aren't actually bad (the attribute getter does have "fast" in the name). What do you think? Basically I think this indirection exists for performance reasons, but I'm not sure how serious it is.
Right, I guess I was assuming you'd change the existing (4?) callers of ScheduleWebMCPSchemaUpdate() to something like
```
if (RuntimeEnabledFeatures::WebMCPEnabled(GetExecutionContext()) &&
IsValidWebMCPForm() {
ScheduleDeclarativeWebMCPToolRegistration();
}
```
The feature check is already present in a few cases. Perhaps that's too verbose. I guess at minimum, if you want to leave this, rename it to something like `ScheduleDeclarativeWebMCPToolRegistrationIfActive()` or something? Otherwise, it's a bit confusing to figure out which of the "Schedule*" functions to call at a given location.
async function waitForFormToolSchemaToMatch(expected_schema) {Dominic FarolinoMaybe move this/these to the utility `.js` files?
Created a new `helpers.js` file in `external/wpt/webmcp/resources`, and referenced it from this file. But I think the wpt_internal/ and other tests cannot reference this file, so this is the only test that uses the helper. But once we remove the `modelContextTesting` API and move all the tests here, then others will be able to make use of it.
Lovely
navigator.modelContextTesting.ontoolchange = check;Dominic FarolinoHow come we can't switch these tests to `modelContext` and just use `waitForTool()`?
The only reason is because `modelContext` is gated on secure context, and this file is not HTTPs. But yeah we can rename it and use the new API accordingly. Done.
Nice, thanks. I didn't realize WebMCP required secure context! TIL.
Related details
- Dominic Farolino
- Fr
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Fr (Gerrit)
Fr added 1 comment![]( "Open in Gerrit")
navigator.modelContextTesting.ontoolchange = check;Dominic FarolinoHow come we can't switch these tests to `modelContext` and just use `waitForTool()`?
Mason FreedThe only reason is because `modelContext` is gated on secure context, and this file is not HTTPs. But yeah we can rename it and use the new API accordingly. Done.
Nice, thanks. I didn't realize WebMCP required secure context! TIL.
FWIW, I've filed https://b.corp.google.com/issues/503588985 last month when I've noticed this.
Related details
- Dominic Farolino
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Dominic Farolino (Gerrit)
Dominic Farolino voted and added 1 comment![]( "Open in Gerrit")
Votes added by Dominic Farolino
| Commit-Queue | +2 |
1 comment
void HTMLFormElement::ScheduleWebMCPSchemaUpdate() {
if (!RuntimeEnabledFeatures::WebMCPEnabled(GetExecutionContext())) {
return;
}
if (!IsValidWebMCPForm()) {
return;
}
ScheduleDeclarativeWebMCPToolRegistration();
}Dominic FarolinoThis is a pretty thin wrapper - perhaps we just remove it and update the few remaining callers to call `ScheduleDeclarativeWebMCPToolRegistration` directly?
Mason FreedHmm, it is a pretty thing wrapper. However, it does do less work, than `ScheduleDeclarativeWebMCPToolRegistration()`, which is why I assume it exists. For associating of every single for control element, regardless of WebMCP usage, this method runs, so it's best to have it stop after a simple pointer-is-null check (which is what `if (!IsValidWebMCPForm()) {` does), rather than entering `ScheduleDeclarativeWebMCPToolRegistration()` and grabbing attributes and running `isConnected()` all to do nothing in 99.9% of forms, where `is_valid_mcp_tool` is null.
But maybe the performance implications of entering that method aren't actually bad (the attribute getter does have "fast" in the name). What do you think? Basically I think this indirection exists for performance reasons, but I'm not sure how serious it is.
Right, I guess I was assuming you'd change the existing (4?) callers of ScheduleWebMCPSchemaUpdate() to something like
```
if (RuntimeEnabledFeatures::WebMCPEnabled(GetExecutionContext()) &&
IsValidWebMCPForm() {
ScheduleDeclarativeWebMCPToolRegistration();
}
```The feature check is already present in a few cases. Perhaps that's too verbose. I guess at minimum, if you want to leave this, rename it to something like `ScheduleDeclarativeWebMCPToolRegistrationIfActive()` or something? Otherwise, it's a bit confusing to figure out which of the "Schedule*" functions to call at a given location.
Got it. Allowing `HTMLFormControlElement` to call `IsValidWebMCPForm()` is a little more invasive since we'd have to make that method public, so I slightly prefer renaming this. However, doing that here will wipe the LGTM and I'd like to land this and merge it back this week, so let me take care of that in a follow-up if that's alright. I'll put this on my TODO list.
Related details
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Blink W3C Test Autoroller (Gerrit)
Message from Blink W3C Test Autoroller![]( "Open in Gerrit")
Exportable changes to web-platform-tests were detected in this CL and a pull request in the upstream repo has been made: https://github.com/web-platform-tests/wpt/pull/59889.
When this CL lands, the bot will automatically merge the PR on GitHub if the required GitHub checks pass; otherwise, ecosystem-infra@ team will triage the failures and may contact you.
WPT Export docs:
https://chromium.googlesource.com/chromium/src/+/main/docs/testing/web_platform_tests.md#Automatic-export-process
Related details
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Dominic Farolino (Gerrit)
Dominic Farolino voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
Mason Freed (Gerrit)
Mason Freed added 1 comment![]( "Open in Gerrit")
void HTMLFormElement::ScheduleWebMCPSchemaUpdate() {
if (!RuntimeEnabledFeatures::WebMCPEnabled(GetExecutionContext())) {
return;
}
if (!IsValidWebMCPForm()) {
return;
}
ScheduleDeclarativeWebMCPToolRegistration();
}Dominic FarolinoThis is a pretty thin wrapper - perhaps we just remove it and update the few remaining callers to call `ScheduleDeclarativeWebMCPToolRegistration` directly?
Mason FreedHmm, it is a pretty thing wrapper. However, it does do less work, than `ScheduleDeclarativeWebMCPToolRegistration()`, which is why I assume it exists. For associating of every single for control element, regardless of WebMCP usage, this method runs, so it's best to have it stop after a simple pointer-is-null check (which is what `if (!IsValidWebMCPForm()) {` does), rather than entering `ScheduleDeclarativeWebMCPToolRegistration()` and grabbing attributes and running `isConnected()` all to do nothing in 99.9% of forms, where `is_valid_mcp_tool` is null.
But maybe the performance implications of entering that method aren't actually bad (the attribute getter does have "fast" in the name). What do you think? Basically I think this indirection exists for performance reasons, but I'm not sure how serious it is.
Dominic FarolinoRight, I guess I was assuming you'd change the existing (4?) callers of ScheduleWebMCPSchemaUpdate() to something like
```
if (RuntimeEnabledFeatures::WebMCPEnabled(GetExecutionContext()) &&
IsValidWebMCPForm() {
ScheduleDeclarativeWebMCPToolRegistration();
}
```The feature check is already present in a few cases. Perhaps that's too verbose. I guess at minimum, if you want to leave this, rename it to something like `ScheduleDeclarativeWebMCPToolRegistrationIfActive()` or something? Otherwise, it's a bit confusing to figure out which of the "Schedule*" functions to call at a given location.
Got it. Allowing `HTMLFormControlElement` to call `IsValidWebMCPForm()` is a little more invasive since we'd have to make that method public, so I slightly prefer renaming this. However, doing that here will wipe the LGTM and I'd like to land this and merge it back this week, so let me take care of that in a follow-up if that's alright. I'll put this on my TODO list.
- Dominic Farolino
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
WebMCP: Queue declarative tool registration and schema calculation
Before this CL, declarative WebMCP tools were registered synchronously,
but had their JSON schemas computed lazily, upon (legacy) `listTools()`
invocation, etc. Tool registration looked like this:
1. When a form with `toolname` and `tooldescription` was inserted, it
synchronously registered a tool with `ModelContext` with an empty
schema
2. A task was queued to fire the `toolchange` event at both
modelContext and modelContextTesting.
3. The `RegisterScriptTool()` IPC is sent to the browser;
asynchronously later, it sends `NotifyToolChange()` to tell the
renderer to fire the `toolchange` event (again!)
4. As form control elements become associated or mutate, the same
underlying tool is left alone, but a task is queued to:
1. Fire `toolchange` events
2. Tell DevTools that the tool has been removed/re-added
The two above only happen if the control association/mutation is
deemed to impact the tool's inputSchema.
So declarative tools are never unregistered as they get built up and as
their schemas change, and `toolchange`/DevTools notifications happen
once per batch of changes.
But right now, the browser process doesn't know how to "update" a tool's
schema, so to keep it "fresh" after schema-affecting mutations, we must
unregister/re-register tools accordingly (later we could consider an
"update" path in the browser process).
To accomplish this, the CL does a few things:
- Instead of registering declarative tools immediately with an empty
schema but queueing a `toolchange` event, this CL queues the
registration of the form altogether. That way by registration time,
batched mutations are done, and the form's schema can be computed
once at the end.
- When form controls become associated or mutate, they unregister and
re-register the underlying form tool if it impacts the existing
tool's schema. Before this CL, they just queued a `toolchange` event
if the schema was impacted.
Performing schema diffs at unregistration/registration time allows us to
delete:
- ModelContext::OnToolChange(bool force);
- ModelContext::InvokeToolChangeClosure(bool force);
- ModelContext::MaybeNotifyToolChanged();
This also makes declarative tools play nicely with the new `getTools()`
and `executeTool()` APIs introduced in https://crrev.com/c/7800264 and
https://crrev.com/c/7808168, because with those APIs.
Concretely, this CL:
- Introduces a new TaskHandle for the async task on HTMLFormElement
- Adds an `HTMLFormElement::ChildrenChanged()` handler which queues the
tool registration task, so that once form mutations settle, the
queued task has a full picture of the form and its JSON schema.
- Renames `UpdateMcpDefinitionsIfNeeded()` to
`ScheduleDeclarativeWebMCPToolRegistration()`, which does the
scheduling.
- Introduces `RegisterDeclarativeWebMCPTool()` which does the actual
registration and input schema diffing.
- Updates `ScheduleWebMCPSchemaUpdate()` (called for each new
form control association/attr change) to call
`ScheduleDeclarativeWebMCPToolRegistration()`, if the form has a
registered tool.
To make this work, this CL modifies a bunch of declarative WebMCP tests
to make them wait on `toolchange` being fired before accessing the tool.
R=masonf
- M third_party/blink/renderer/core/html/custom/element_internals.cc
- M third_party/blink/renderer/core/html/forms/html_form_element.cc
- M third_party/blink/renderer/core/html/forms/html_form_element.h
- M third_party/blink/renderer/core/html/forms/html_form_mcp_tool_test.cc
- M third_party/blink/renderer/core/script_tools/model_context.cc
- M third_party/blink/renderer/core/script_tools/model_context.h
- M third_party/blink/renderer/core/script_tools/model_context_test.cc
- A third_party/blink/web_tests/external/wpt/webmcp/declarative/getTools-declarative-schema.https.html
- M third_party/blink/web_tests/external/wpt/webmcp/declarative/toolchange-on-control-add-remove.https.html
- A third_party/blink/web_tests/external/wpt/webmcp/resources/helpers.js
- R third_party/blink/web_tests/fast/webmcp/get_cross_document_script_tool_result.https-expected.txt
- R third_party/blink/web_tests/fast/webmcp/get_cross_document_script_tool_result.https.html
- A third_party/blink/web_tests/fast/webmcp/resources/webmcp-helpers.js
- R third_party/blink/web_tests/fast/webmcp/usecounter.https.html
- M third_party/blink/web_tests/inspector-protocol/webmcp/declarative-tool-multiple-inputs.js
- M third_party/blink/web_tests/inspector-protocol/webmcp/list-tools.js
- M third_party/blink/web_tests/virtual/webmcp/inspector-protocol/webmcp/declarative-tool-multiple-inputs-expected.txt
- M third_party/blink/web_tests/virtual/webmcp/inspector-protocol/webmcp/list-tools-expected.txt
- M third_party/blink/web_tests/wpt_internal/script_tools/form_removal_submit_crash.https.html
- D third_party/blink/web_tests/wpt_internal/webmcp/custom-element-cease-param.html
- A third_party/blink/web_tests/wpt_internal/webmcp/custom-element-cease-param.https.html
- R third_party/blink/web_tests/wpt_internal/webmcp/custom-element-fill.https.html
- R third_party/blink/web_tests/wpt_internal/webmcp/custom-element-invalid-schema.https.html
- R third_party/blink/web_tests/wpt_internal/webmcp/execute_tool_change_event.https.html
- R third_party/blink/web_tests/wpt_internal/webmcp/execute_tool_submit_from_js.https.html
- A third_party/blink/web_tests/wpt_internal/webmcp/resources/webmcp-helpers.js
- R third_party/blink/web_tests/wpt_internal/webmcp/select-multiple-events.https.html
Code-Review: +1 by Mason Freed
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Blink W3C Test Autoroller (Gerrit)
Message from Blink W3C Test Autoroller![]( "Open in Gerrit")
The WPT PR for this CL has been merged upstream! https://github.com/web-platform-tests/wpt/pull/59889
Related details
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |