1667839 - Watched Data is not passed correctly to targets that already exists

Status: RESOLVED FIXED

(DevTools :: Framework, defect)

Description

[Alexandre Poirot [:ochameau]]

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

[Chris Peterson [:cpeterson]]

Tracking dt-fission-m2-mvp bugs for Fission Nightly milestone (M6c).

Comment 2

[Alexandre Poirot [:ochameau]]

Oh and two additional facts makes this issue harmless:

• ResourceWatcher first call WatcherActor.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 call WatcherActor.watchResources, which works fine and call addWatcherDataEntry 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!

Comment 3

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1667839 - Pass watchedData to already existing targets.

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.

Comment 4

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1667839 - Test console message resources against remoted iframes.

Comment 5

[Neha Kochar [:neha]]

Bulk move of all m2-mvp devtools bugs to Fission M7.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
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

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/015a1ec40270
https://hg.mozilla.org/mozilla-central/rev/057106d68590