Add WebMCP methods to Script Tools API [chromium/src : main]
David Bokan (Gerrit)
David Bokan added 7 comments![]( "Open in Gerrit")
lgtm, thanks!
I think you also have to update https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/web_tests/webexposed/global-interface-listing-expected.txt;l=374;drc=e9442c3f9a41aebeaafccecb132709a05201fa50 (the one you updated is specific to android webview)
const mcp = window.mcp;supernit: seems unneeded? i.e. `window.mcp.provideContext`?
type: "object", Please fix this WARNING reported by Check Contents: Please remove the trailing whitespace.
To rerun the analyzer locally, run: `ali...
Please remove the trailing whitespace.
To rerun the analyzer locally, run: `alint -- -c CheckContents`
For more information, see go/alint.
const mcp = window.mcp;Ah, this is existing, feel free to remove use of variable.
Please fix this WARNING reported by Check Contents: Please remove the trailing whitespace.
To rerun the analyzer locally, run: `ali...
Please remove the trailing whitespace.
To rerun the analyzer locally, run: `alint -- -c CheckContents`
For more information, see go/alint.
static ModelContextProvider* GetProviderIfExists(LocalDOMWindow&);nit: since the name already has to be qualified with the class, how about `GetIfExists`?
name: "",No need to resolve here but we should probably have some restrictions on the name string (i.e. I think right now a single space would be valid?) Also limits on length?
Unfortunately I don't see anything about this in the MCP spec other than it being unique.
Related details
- Brandon Walderman
- Dave Tapuska
- Khushal Sagar
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
David Bokan (Gerrit)
David Bokan voted Code-Review+1![]( "Open in Gerrit")
| Code-Review | +1 |
Related details
- Brandon Walderman
- Dave Tapuska
- Khushal Sagar
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Khushal Sagar (Gerrit)
Khushal Sagar added 1 comment![]( "Open in Gerrit")
Given that we couldn't get a resolution on this today (see https://github.com/webmachinelearning/webmcp/issues/24), I'd prefer not landing the rename for now. There's a few demos built on this API, better if we have devs update it once?
I'm good with landing the new provideContext/clearContext APIs since we had a resolution on that: https://github.com/webmachinelearning/webmcp/issues/15. Could we have this patch do just that for now?
Related details
- Brandon Walderman
- Dave Tapuska
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Dave Tapuska (Gerrit)
Dave Tapuska voted and added 2 comments![]( "Open in Gerrit")
Votes added by Dave Tapuska
| Code-Review | +1 |
2 comments
interface ModelContextProviderNot sorted correctly. That is why the bot is failing
Related details
- Brandon Walderman
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
David Bokan (Gerrit)
David Bokan added 2 comments![]( "Open in Gerrit")
lgtm, thanks!
I think you also have to update https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/web_tests/webexposed/global-interface-listing-expected.txt;l=374;drc=e9442c3f9a41aebeaafccecb132709a05201fa50 (the one you updated is specific to android webview)
Oops - missed that you updated both but Dave points out it's a sort error.
Given that we couldn't get a resolution on this today (see https://github.com/webmachinelearning/webmcp/issues/24), I'd prefer not landing the rename for now. There's a few demos built on this API, better if we have devs update it once?
I'm good with landing the new provideContext/clearContext APIs since we had a resolution on that: https://github.com/webmachinelearning/webmcp/issues/15. Could we have this patch do just that for now?
Given there's only a hand full of demos now and assuming we have some confidence `mcp` can stick for at least the near term (~months), now is probably the easiest it'll ever be to rename.
Sorry I had to miss the meeting today - do you have a sense of if we just want another week or two of discussion time or if this is something that'll take longer to resolve? If the former I'm fine to wait but if latter I think we should rename to a more amenable name sooner rather than later.
Related details
- Brandon Walderman
- Khushal Sagar
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Brandon Walderman (Gerrit)
Brandon Walderman added 5 comments![]( "Open in Gerrit")
supernit: seems unneeded? i.e. `window.mcp.provideContext`?
Done
Please fix this WARNING reported by Check Contents: Please remove the trailing whitespace.
To rerun the analyzer locally, run: `ali...
Please remove the trailing whitespace.
To rerun the analyzer locally, run: `alint -- -c CheckContents`
For more information, see go/alint.
Fixed
Ah, this is existing, feel free to remove use of variable.
Done
Please fix this WARNING reported by Check Contents: Please remove the trailing whitespace.
To rerun the analyzer locally, run: `ali...
Please remove the trailing whitespace.
To rerun the analyzer locally, run: `alint -- -c CheckContents`
For more information, see go/alint.
Fixed
static ModelContextProvider* GetProviderIfExists(LocalDOMWindow&);nit: since the name already has to be qualified with the class, how about `GetIfExists`?
- Dave Tapuska
- David Bokan
- Khushal Sagar
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Brandon Walderman (Gerrit)
Brandon Walderman added 3 comments![]( "Open in Gerrit")
David BokanGiven that we couldn't get a resolution on this today (see https://github.com/webmachinelearning/webmcp/issues/24), I'd prefer not landing the rename for now. There's a few demos built on this API, better if we have devs update it once?
I'm good with landing the new provideContext/clearContext APIs since we had a resolution on that: https://github.com/webmachinelearning/webmcp/issues/15. Could we have this patch do just that for now?
Given there's only a hand full of demos now and assuming we have some confidence `mcp` can stick for at least the near term (~months), now is probably the easiest it'll ever be to rename.
Sorry I had to miss the meeting today - do you have a sense of if we just want another week or two of discussion time or if this is something that'll take longer to resolve? If the former I'm fine to wait but if latter I think we should rename to a more amenable name sooner rather than later.
Leaving the object rename out of this CL and will hopefully follow up shortly with another once we have a resolution on that.
interface ModelContextProviderBrandon Waldermansorting
Done
Not sorted correctly. That is why the bot is failing
Done
Related details
- Dave Tapuska
- David Bokan
- Khushal Sagar
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Dave Tapuska (Gerrit)
Dave Tapuska added 2 comments![]( "Open in Gerrit")
[CallWith=ScriptState, RaisesException] undefined registerTool(ToolRegistrationParams params);If we are having end users experiment with both approaches should we have a MeasureAs for these?
Related details
- Brandon Walderman
- David Bokan
- Khushal Sagar
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Dave Tapuska (Gerrit)
Dave Tapuska voted Code-Review+1![]( "Open in Gerrit")
| Code-Review | +1 |
Dave Tapuska (Gerrit)
Dave Tapuska added 1 comment![]( "Open in Gerrit")
[CallWith=ScriptState, RaisesException] undefined registerTool(ToolRegistrationParams params);If we are having end users experiment with both approaches should we have a MeasureAs for these?
We should be able to just add the Measure (not MeasureAs) to these extended attributes and update the use counters appropriately.
Khushal Sagar (Gerrit)
Khushal Sagar voted and added 3 comments![]( "Open in Gerrit")
Votes added by Khushal Sagar
| Code-Review | +1 |
3 comments
tool_map_ = std::move(prev_tool_map);Not sure if it's better to keep the existing state when there's an error or discard it anyway. Can discuss on an issue.
[CallWith=ScriptState, RaisesException] undefined registerTool(ToolRegistrationParams params);Dave TapuskaIf we are having end users experiment with both approaches should we have a MeasureAs for these?
We should be able to just add the Measure (not MeasureAs) to these extended attributes and update the use counters appropriately.
We're far off from end user experiments. :) For now, we just wanted to add both for devs trying the API. And learn from feedback about which one (or both) are needed.
Related details
- Brandon Walderman
- David Bokan
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Brandon Walderman (Gerrit)
Brandon Walderman added 2 comments![]( "Open in Gerrit")
[CallWith=ScriptState, RaisesException] undefined registerTool(ToolRegistrationParams params);Dave TapuskaIf we are having end users experiment with both approaches should we have a MeasureAs for these?
Khushal SagarWe should be able to just add the Measure (not MeasureAs) to these extended attributes and update the use counters appropriately.
We're far off from end user experiments. :) For now, we just wanted to add both for devs trying the API. And learn from feedback about which one (or both) are needed.
Acknowledged
# Copyright 2025 The Chromium AuthorsBrandon WaldermanRemove?
Done
Related details
- Dave Tapuska
- David Bokan
- Khushal Sagar
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Dave Tapuska (Gerrit)
Dave Tapuska voted Code-Review+1![]( "Open in Gerrit")
- Brandon Walderman
- David Bokan
- Khushal Sagar
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
David Bokan (Gerrit)
David Bokan voted and added 1 comment![]( "Open in Gerrit")
Votes added by David Bokan
| Code-Review | +1 |
1 comment
# found in the LICENSE file.don't gni files need a copyright header? (year needs to be updated though)
Related details
- Brandon Walderman
- Khushal Sagar
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Dave Tapuska (Gerrit)
Dave Tapuska added 1 comment![]( "Open in Gerrit")
# found in the LICENSE file.don't gni files need a copyright header? (year needs to be updated though)
Ugh.. I think the comment I made saying "Remove" was mis-interpretted. I was indicating remove this file from the patch set because it has no material changes. But the author seems to have taken that as "Remove the files on which the comment was made".
Yes please instead remove this file from the patch set.
David Bokan (Gerrit)
David Bokan added 1 comment![]( "Open in Gerrit")
# found in the LICENSE file.Dave Tapuskadon't gni files need a copyright header? (year needs to be updated though)
Ugh.. I think the comment I made saying "Remove" was mis-interpretted. I was indicating remove this file from the patch set because it has no material changes. But the author seems to have taken that as "Remove the files on which the comment was made".
Yes please instead remove this file from the patch set.
We should add a copyright notice on this file though...I don't mind doing it here but it can also be a separate CL
David Bokan (Gerrit)
David Bokan added 1 comment![]( "Open in Gerrit")
# found in the LICENSE file.Dave Tapuskadon't gni files need a copyright header? (year needs to be updated though)
David BokanUgh.. I think the comment I made saying "Remove" was mis-interpretted. I was indicating remove this file from the patch set because it has no material changes. But the author seems to have taken that as "Remove the files on which the comment was made".
Yes please instead remove this file from the patch set.
We should add a copyright notice on this file though...I don't mind doing it here but it can also be a separate CL
err, sorry, I mean we should update the year from 2020 to 2025 (which I think is what Brandon was doing in the original PS).
Dave Tapuska (Gerrit)
Dave Tapuska added 1 comment![]( "Open in Gerrit")
# found in the LICENSE file.Dave Tapuskadon't gni files need a copyright header? (year needs to be updated though)
David BokanUgh.. I think the comment I made saying "Remove" was mis-interpretted. I was indicating remove this file from the patch set because it has no material changes. But the author seems to have taken that as "Remove the files on which the comment was made".
Yes please instead remove this file from the patch set.
David BokanWe should add a copyright notice on this file though...I don't mind doing it here but it can also be a separate CL
err, sorry, I mean we should update the year from 2020 to 2025 (which I think is what Brandon was doing in the original PS).
Yes, but then the rename didn't happen. And then this file was just left updating the copyright and no other changes, so I asked for it to be removed from the patch set.
Khushal Sagar (Gerrit)
Khushal Sagar voted and added 1 comment![]( "Open in Gerrit")
Related details
- Brandon Walderman
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Brandon Walderman (Gerrit)
Brandon Walderman added 1 comment![]( "Open in Gerrit")
# found in the LICENSE file.Dave Tapuskadon't gni files need a copyright header? (year needs to be updated though)
David BokanUgh.. I think the comment I made saying "Remove" was mis-interpretted. I was indicating remove this file from the patch set because it has no material changes. But the author seems to have taken that as "Remove the files on which the comment was made".
Yes please instead remove this file from the patch set.
David BokanWe should add a copyright notice on this file though...I don't mind doing it here but it can also be a separate CL
Dave Tapuskaerr, sorry, I mean we should update the year from 2020 to 2025 (which I think is what Brandon was doing in the original PS).
Yes, but then the rename didn't happen. And then this file was just left updating the copyright and no other changes, so I asked for it to be removed from the patch set.
Removing from this patch set. We'll have a chance to update it again when we've settled on a name change.
Related details
- Dave Tapuska
- David Bokan
- Khushal Sagar
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Khushal Sagar (Gerrit)
Khushal Sagar voted Code-Review+1![]( "Open in Gerrit")
- Brandon Walderman
- Dave Tapuska
- David Bokan
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
David Bokan (Gerrit)
David Bokan voted Code-Review+1![]( "Open in Gerrit")
| Code-Review | +1 |
Related details
- Brandon Walderman
- Dave Tapuska
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Dave Tapuska (Gerrit)
Dave Tapuska voted Code-Review+1![]( "Open in Gerrit")
| Code-Review | +1 |
Related details
- Brandon Walderman
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Brandon Walderman (Gerrit)
Brandon Walderman voted Commit-Queue+2![]( "Open in Gerrit")
Code-Coverage
Code-Owners
Code-Review
| 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
Add WebMCP methods to Script Tools API
This change adds the provideContext and clearContext methods alongside
the registerTool and unregisterTool methods to give web developers the
opportunity to experiment with both approaches.
- M android_webview/test/data/web_tests/webexposed/global-interface-listing-expected.txt
- M chrome/browser/actor/tools/script_tool_browsertest.cc
- M chrome/test/data/actor/script_tool.html
- A chrome/test/data/actor/script_tool_provide_context.html
- M components/test/data/optimization_guide/script_tool.html
- M components/test/data/optimization_guide/script_tool_no_annotation.html
- M components/test/data/optimization_guide/script_tool_no_input_schema.html
- M third_party/blink/renderer/bindings/generated_in_core.gni
- M third_party/blink/renderer/core/script_tools/automation_delegate.cc
- M third_party/blink/renderer/core/script_tools/automation_delegate.h
- M third_party/blink/renderer/core/script_tools/automation_delegate.idl
- M third_party/blink/renderer/core/script_tools/automation_delegate_supplement.cc
- M third_party/blink/renderer/core/script_tools/automation_delegate_supplement.h
- M third_party/blink/renderer/core/script_tools/tool_registration_params.idl
- M third_party/blink/renderer/modules/content_extraction/ai_page_content_agent.cc
- M third_party/blink/web_tests/webexposed/global-interface-listing-expected.txt
- A third_party/blink/web_tests/wpt_internal/script_tools/clear_context.html
- A third_party/blink/web_tests/wpt_internal/script_tools/provide_context_basic.html
- A third_party/blink/web_tests/wpt_internal/script_tools/provide_context_duplicates.html
- A third_party/blink/web_tests/wpt_internal/script_tools/provide_context_errors.html
- A third_party/blink/web_tests/wpt_internal/script_tools/provide_context_mixed_registration.html
Code-Review: +1 by Khushal Sagar, +1 by Dave Tapuska, +1 by David Bokan
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |