Closed Bug 1704805 Opened 4 years ago Closed 4 years ago

Empty and erroneous Watcher data entries are processed by the Target Actor Mixin

Categories

(DevTools :: Framework, defect, P3)

defect

Tracking

(firefox90 fixed)

RESOLVED FIXED
90 Branch
Tracking Status
firefox90 --- fixed

People

(Reporter: ochameau, Assigned: ochameau)

References

Details

Attachments

(1 file)

https://searchfox.org/mozilla-central/rev/3de2db87f3c9001ae478318d47a2ca3427574382/devtools/server/connectors/js-window-actor/DevToolsFrameChild.jsm#237-240

    // Pass initialization data to the target actor
    for (const type in initialData) {
      targetActor.addWatcherDataEntry(type, initialData[type]);
    }

In this code, initialData is a dictionnary with many attributes, some will be real data entries. Some other won't be data entries, like browserId and watcherTraits.

This data object is being prefilled from here:
https://searchfox.org/mozilla-central/rev/3de2db87f3c9001ae478318d47a2ca3427574382/devtools/server/actors/watcher/WatcherRegistry.jsm#113-128

      watchedData = {
        // The Browser ID will be helpful to identify which BrowsingContext should be considered
        // when running code in the content process. Browser ID, compared to BrowsingContext ID won't change
        // if we navigate to the parent process or if a new BrowsingContext is used for the <browser> element
        // we are currently inspecting.
        browserId: watcher.browserId,
        // The DevToolsServerConnection prefix will be used to compute actor IDs created in the content process
        connectionPrefix: watcher.conn.prefix,
        // Expose watcher traits so we can retrieve them in content process.
        // This should be removed as part of Bug 1700092.
        watcherTraits: watcher.form().traits,
      };
      // Define empty default array for all data
      for (const name of Object.values(SUPPORTED_DATA)) {
        watchedData[name] = [];
      }

And even if this is a real data entry, it may be an empty array.
It causes issues especially for BREAKPOINTS, which will attach the target whereas we don't really have too.
Actually, I haven't found a real user-facing bug because of this, it only prevented me from easily reproducing a race around target actor attach. This code was forcing the attach of the target actor, whereas my bug was only reproducing with a not-yet-attached target.

Blocks: 1704806

We were, for example, processing empty breakpoints list,
which forced to attach the target actor for no reason.

Attachment #9215443 - Attachment description: Bug 1704805 - Avoid processing erronous or empty data entries. → Bug 1704805 - [devtools] Avoid processing erronous or empty data entries.
Severity: -- → S3
Priority: -- → P3
Pushed by apoirot@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/a87ae0b75018 [devtools] Avoid processing erronous or empty data entries. r=nchevobbe
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 90 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: