[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. |