Watched Data is not passed correctly to targets that already exists
Categories
(DevTools :: Framework, defect)
Tracking
(Fission Milestone:M7, firefox83 fixed)
Tracking | Status | |
---|---|---|
firefox83 | --- | fixed |
People
(Reporter: ochameau, Assigned: ochameau)
References
Details
(Whiteboard: dt-fission-m2-mvp)
Attachments
(2 files)
While working on bug 1573327 I forgot refactoring the createTargets method of FrameHelper module:
https://searchfox.org/mozilla-central/source/devtools/server/actors/watcher/target-helpers/frame-helper.js#46
So that later in DevToolsFrameChild we pass a watchedResources
object instead of watchedData
to _createTargetActor
:
https://searchfox.org/mozilla-central/rev/f27594d62e7f1d57626889255ce6a3071d67209f/devtools/server/connectors/js-window-actor/DevToolsFrameChild.jsm#337-341
This is really surprising that no test started failing because of this.
I imagine it still works for the top level target thanks to this code:
https://searchfox.org/mozilla-central/source/devtools/server/actors/watcher.js#310-314
const targetActor = this.browserElement
? TargetActorRegistry.getTargetActor(this.browserId)
: TargetActorRegistry.getParentProcessTargetActor();
if (targetActor) {
await targetActor.watchTargetResources(frameResourceTypes);
So, we probably miss a test covering listening to pre-existing remote iframe, with pre-existing resources.
Comment 1•4 years ago
|
||
Tracking dt-fission-m2-mvp bugs for Fission Nightly milestone (M6c).
Assignee | ||
Comment 2•4 years ago
|
||
Oh and two additional facts makes this issue harmless:
ResourceWatcher
first callWatcherActor.watchTarget
, which is buggy, but without any "watchedData", without any watched resources:
https://searchfox.org/mozilla-central/source/devtools/shared/resources/resource-watcher.js#118-131
await this._watchAllTargets();
for (const resource of resources) {
// If we are registering the first listener, so start listening from the server about
// this one resource.
if (!this._hasListenerForResource(resource)) {
await this._startListening(resource);
}
}
So that, yes, the call to createTargetActor comes with no watched data, but the frontend don't pass any anyway...
- Then
ResourceWatcher
callWatcherActor.watchResources
, which works fine and calladdWatcherDataEntry
and start listening for resources on all the targets!
I tried writing a test for this, but that would require calling WatcherFront manually and bypass ResourceWatcher. I'm not sure it is worth doing it?
But I'll provide a first Resource test covering fission. So far, only TargetList tests were using test page with remote iframes!
Assignee | ||
Comment 3•4 years ago
|
||
The previous code was all wrong and we weren't correctly listening to resource
for already existing additional targets. Top level target was still working fine
thanks to WatcherActor.watchResources calling watchTargetResource directly.
Assignee | ||
Comment 4•4 years ago
|
||
Comment 5•4 years ago
|
||
Bulk move of all m2-mvp devtools bugs to Fission M7.
Pushed by apoirot@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/015a1ec40270 Pass watchedData to already existing targets. r=nchevobbe https://hg.mozilla.org/integration/autoland/rev/057106d68590 Test console message resources against remoted iframes. r=nchevobbe
Comment 7•4 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/015a1ec40270
https://hg.mozilla.org/mozilla-central/rev/057106d68590
Description
•