[CDP] introduce Inspector.workerScriptLoaded [chromium/src : main]
Dmitry Gozman (Gerrit)
Dmitry Gozman added 1 comment![]( "Open in Gerrit")
Andrey, could you please take a look?
Related details
- Andrey Kosyakov
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Andrey Kosyakov (Gerrit)
Andrey Kosyakov voted and added 4 comments![]( "Open in Gerrit")
Votes added by Andrey Kosyakov
| Code-Review | +1 |
4 comments
lgtm with comments
event workerScriptLoaded`experimental`, please!
inspector_agents_;Can we avoid this and piggy-back on the probe sink instead?
if (auto* controller = GetThread()->GetWorkerInspectorController()) {Why not just go with a regular probe here and save on explicit session collection in the WorkerInspectorController?
Related details
- Dmitry Gozman
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. |
Dmitry Gozman (Gerrit)
Dmitry Gozman added 1 comment![]( "Open in Gerrit")
inspector_agents_;Can we avoid this and piggy-back on the probe sink instead?
We can, but we'll still need this map to addToInstrumentingAgents on attach and removeFromInstrumentingAgents on detach. So I borrowed this pattern from `WebDevToolsAgentImpl`.
Any ideas on getting rid of this map are appreciated.
Related details
- Andrey Kosyakov
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. |
Andrey Kosyakov (Gerrit)
Andrey Kosyakov added 1 comment![]( "Open in Gerrit")
inspector_agents_;Dmitry GozmanCan we avoid this and piggy-back on the probe sink instead?
We can, but we'll still need this map to addToInstrumentingAgents on attach and removeFromInstrumentingAgents on detach. So I borrowed this pattern from `WebDevToolsAgentImpl`.
Any ideas on getting rid of this map are appreciated.
Ugh. I just realized we don't have Inspector.enable and emit unsolicited events ;-)
Related details
- Dmitry Gozman
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. |
Dmitry Gozman (Gerrit)
Dmitry Gozman added 1 comment![]( "Open in Gerrit")
inspector_agents_;Dmitry GozmanCan we avoid this and piggy-back on the probe sink instead?
Andrey KosyakovWe can, but we'll still need this map to addToInstrumentingAgents on attach and removeFromInstrumentingAgents on detach. So I borrowed this pattern from `WebDevToolsAgentImpl`.
Any ideas on getting rid of this map are appreciated.
Ugh. I just realized we don't have Inspector.enable and emit unsolicited events ;-)
That's correct. I tried to put this behind `Inspector.enable()`, see patchset #1, but that does not work nicely, because it now makes `Inspector.enable()` call roundtrip to the renderer, and chromedriver does not expect that. I guess it's being awaited before issuing `Runtime.runIfWaitingForDebugger()` or something along these lines.
Interestingly, we already do this in the [Inspector domain](https://source.chromium.org/chromium/chromium/src/+/main:content/browser/devtools/protocol/inspector_handler.cc;l=37-40;drc=62a9a00176f862e688fa47919ed6f19b59f77b5f;bpv=1;bpt=1).
If you think this behavior is not ideal, we'll have to put this event into a different domain. Nothing comes to mind though...
Related details
- Andrey Kosyakov
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. |
Andrey Kosyakov (Gerrit)
Andrey Kosyakov added 1 comment![]( "Open in Gerrit")
inspector_agents_;Dmitry GozmanCan we avoid this and piggy-back on the probe sink instead?
Andrey KosyakovWe can, but we'll still need this map to addToInstrumentingAgents on attach and removeFromInstrumentingAgents on detach. So I borrowed this pattern from `WebDevToolsAgentImpl`.
Any ideas on getting rid of this map are appreciated.
Dmitry GozmanUgh. I just realized we don't have Inspector.enable and emit unsolicited events ;-)
That's correct. I tried to put this behind `Inspector.enable()`, see patchset #1, but that does not work nicely, because it now makes `Inspector.enable()` call roundtrip to the renderer, and chromedriver does not expect that. I guess it's being awaited before issuing `Runtime.runIfWaitingForDebugger()` or something along these lines.
Interestingly, we already do this in the [Inspector domain](https://source.chromium.org/chromium/chromium/src/+/main:content/browser/devtools/protocol/inspector_handler.cc;l=37-40;drc=62a9a00176f862e688fa47919ed6f19b59f77b5f;bpv=1;bpt=1).
If you think this behavior is not ideal, we'll have to put this event into a different domain. Nothing comes to mind though...
Would putting the event into a different domain result in a different behavior though? I think present implementation is acceptable for a provisional workaround, and we should perhaps eventually come up with a cleaner solution.
Related details
- Dmitry Gozman
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. |
Dmitry Gozman (Gerrit)
Dmitry Gozman voted Commit-Queue+2![]( "Open in Gerrit")
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. |
Dmitry Gozman (Gerrit)
Dmitry Gozman added 2 comments![]( "Open in Gerrit")
event workerScriptLoadedDmitry Gozman`experimental`, please!
Done
if (auto* controller = GetThread()->GetWorkerInspectorController()) {Why not just go with a regular probe here and save on explicit session collection in the WorkerInspectorController?
Acknowledged
Related details
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Dmitry Gozman (Gerrit)
Dmitry Gozman voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
Related details
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 with unreviewed changes![]( "Open in Gerrit")
Unreviewed changes
3 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: third_party/blink/public/devtools_protocol/domains/Inspector.pdl
Insertions: 1, Deletions: 1.
@@ -25,4 +25,4 @@
event targetReloadedAfterCrash
# Fired on worker targets when main worker script and any imported scripts have been evaluated.
- event workerScriptLoaded
+ experimental event workerScriptLoaded
```
Change information
[CDP] introduce Inspector.workerScriptLoaded
This event tells when the main worker script, and all imported
scripts, have been evaluated. Most of the time, accessing worker
before this moment is useless, as you get an empty context.
Now clients can setup the worker before the main script is evaluated,
then call Runtime.runIfWaitingForDebugger, and then wait until the
main script is evaluated before evaluating anything in the fully
setup worker.
- M third_party/blink/public/devtools_protocol/domains/Inspector.pdl
- M third_party/blink/renderer/core/inspector/BUILD.gn
- M third_party/blink/renderer/core/inspector/build.gni
- A third_party/blink/renderer/core/inspector/inspector_inspector_agent.cc
- A third_party/blink/renderer/core/inspector/inspector_inspector_agent.h
- M third_party/blink/renderer/core/inspector/inspector_protocol_config.json
- M third_party/blink/renderer/core/inspector/worker_inspector_controller.cc
- M third_party/blink/renderer/core/inspector/worker_inspector_controller.h
- M third_party/blink/renderer/core/workers/worker_global_scope.cc
- A third_party/blink/web_tests/http/tests/inspector-protocol/inspector/worker-script-loaded-expected.txt
- A third_party/blink/web_tests/http/tests/inspector-protocol/inspector/worker-script-loaded.js
- A third_party/blink/web_tests/http/tests/inspector-protocol/resources/workerClassic.js
- A third_party/blink/web_tests/http/tests/inspector-protocol/resources/workerClassicImport.js
- A third_party/blink/web_tests/http/tests/inspector-protocol/resources/workerModule.js
- A third_party/blink/web_tests/http/tests/inspector-protocol/resources/workerModuleImport.js
Code-Review: +1 by Andrey Kosyakov
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
luci-bisection@appspot.gserviceaccount.com (Gerrit)
luci-bi...@appspot.gserviceaccount.com has created a revert of this change![]( "Open in Gerrit")
Related details
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |