Add support for embedder-managed session state in V8InspectorSession [v8/v8 : main]
Andrey Kosyakov (Gerrit)
Andrey Kosyakov voted and added 1 comment![]( "Open in Gerrit")
Votes added by Andrey Kosyakov
| Commit-Queue | +1 |
1 comment
PTAL -- this CL lets us stop sending scripts (as breakpoint conditions) accross the renderers by managing breakpoints on the browser side instead.
Related details
- Kim-Anh Tran
- Simon Zünd
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Simon Zünd (Gerrit)
Simon Zünd voted and added 2 comments![]( "Open in Gerrit")
Votes added by Simon Zünd
| Code-Review | +1 |
2 comments
struct V8_EXPORT URLBreakpoint {nit: consistent naming. Unfortunately, all of the inspector (even the API) uses `camelCase` 😞 Here and throughout the CL.
"dispatch, is_fully_trusted, embedder_breakpoints).");Why just the breakpoints here instead of a full object representing the embedder state? If we'd pass something like `{ url_breakpoints: [{...}, {...}], }` wouldn't that match what we do on the C++ v8-inspector.h side?
I assume we'll move more state to `EmbedderState` in the future?
Related details
- Andrey Kosyakov
- Kim-Anh Tran
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Andrey Kosyakov (Gerrit)
Andrey Kosyakov added 3 comments![]( "Open in Gerrit")
nit: consistent naming. Unfortunately, all of the inspector (even the API) uses `camelCase` 😞 Here and throughout the CL.
Done
Why just the breakpoints here instead of a full object representing the embedder state? If we'd pass something like `{ url_breakpoints: [{...}, {...}], }` wouldn't that match what we do on the C++ v8-inspector.h side?
I assume we'll move more state to `EmbedderState` in the future?
Yup, makes total sense, done! Re moving more state -- hopefully not. I'm a bit reluctant to move state management to the browser side in general, as it constitutes a layering violation and costs us quite a bit of extra code, so we plan on doing this as an exception only when required by security for now. The ultimate solution would be to leave state management to the client (but this is CDP 2.0).
For now, I don't see any other strings that are readily expoitable, although I wonder about regexps using for blackboxing a bit:
https://source.chromium.org/search?q=f:v8%2F.*%2Fv8.*agent-impl%20setstring&sq=&ss=chromium
Related details
- Kim-Anh Tran
- Simon Zünd
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Simon Zünd (Gerrit)
Simon Zünd voted Code-Review+1![]( "Open in Gerrit")
- Andrey Kosyakov
- Kim-Anh Tran
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |