Commit c14411e5 authored by Alexandre Poirot's avatar Alexandre Poirot
Browse files

Bug 1727838 - [devtools] Avoid leaking InspectorFront via the STYLESHEET... 

Bug 1727838 - [devtools] Avoid leaking InspectorFront via the STYLESHEET resource listener. r=nchevobbe

Differential Revision: https://phabricator.services.mozilla.com/D123920
parent 9adda172

devtools/client/fronts/inspector.js

+25 −1
Original line number Diff line number Diff line
@@ -43,6 +43,8 @@ class InspectorFront extends FrontClassWithSpec(inspectorSpec) { 


    // Map of highlighter types to unsettled promises to create a highlighter of that type

    this._pendingGetHighlighterMap = new Map();



    this.noopStylesheetListener = () => {};

  }



  // async initialization
@@ -60,10 +62,17 @@ class InspectorFront extends FrontClassWithSpec(inspectorSpec) { 
        resourceCommand.TYPES.STYLESHEET

      )

    ) {

      // Store `resourceCommand` on the inspector front as we need it later, and we might not be able to retrieve it from

      // the targetFront as its resourceCommand property is nullified by ResourceCommand.onTargetDestroyed.

      this.resourceCommand = resourceCommand;

      await resourceCommand.watchResources([resourceCommand.TYPES.STYLESHEET], {

        // we simply want to start the watcher, we don't have to do anything with those resources.

        onAvailable: () => {},

        onAvailable: this.noopStylesheetListener,

      });

      // Bail out if the inspector is closed while watchResources was pending

      if (this.isDestroyed()) {

        return null;

      }

    }



    this.initialized = await Promise.all([
@@ -105,7 +114,22 @@ class InspectorFront extends FrontClassWithSpec(inspectorSpec) { 
  }



  destroy() {

    if (this.isDestroyed()) {

      return;

    }

    this._compatibility = null;



    // We might not have started watching for STYLESHEET if the debugged context

    // doesn't support the watcher codepath

    if (this.resourceCommand) {

      const { resourceCommand } = this;

      resourceCommand.unwatchResources([resourceCommand.TYPES.STYLESHEET], {

        onAvailable: this.noopStylesheetListener,

      });

      this.resourceCommand = null;

    }

    this.walker = null;



    // CustomHighlighter fronts are managed by InspectorFront and so will be

    // automatically destroyed. But we have to clear the `_highlighters`

    // Map as well as explicitly call `finalize` request on all of them.

devtools/client/inspector/shared/walker-event-listener.js

+5 −0
Original line number Diff line number Diff line
@@ -61,6 +61,11 @@ class WalkerEventListener { 


  async _onTargetAvailable({ targetFront }) {

    const inspectorFront = await targetFront.getFront("inspector");

    // In case of multiple fast navigations, the front may already be destroyed,

    // in such scenario bail out and ignore this short lived target.

    if (inspectorFront.isDestroyed()) {

      return;

    }

    const { walker } = inspectorFront;

    for (const [name, listener] of Object.entries(this._listenerMap)) {

      walker.on(name, listener);

devtools/client/inspector/test/browser_inspector_iframe-picker-bfcache-navigation.js

+6 −3
Original line number Diff line number Diff line
@@ -6,11 +6,11 @@ 


// Test that using the iframe picker usage + bfcache navigations don't break the toolbox



const IFRAME_URL = `https://example.com/document-builder.sjs?html=<div id=in-iframe>frame</div>`;

const IFRAME_URL = `https://example.com/document-builder.sjs?html=<meta charset=utf8><div id=in-iframe>frame</div>`;



const URL =

  "https://example.com/document-builder.sjs?html=" +

  `<iframe src='${IFRAME_URL}'></iframe><div id=top>top</div>`;

  `<meta charset=utf8><iframe src='${IFRAME_URL}'></iframe><div id=top>top</div>`;



add_task(async function() {

  await pushPref("devtools.command-button-frames.enabled", true);
@@ -34,7 +34,7 @@ add_task(async function() { 
  info("Navigate to a different page to put the current page in the bfcache");

  let onMarkupLoaded = getOnInspectorReadyAfterNavigation(inspector);

  await navigateTo(

    "https://example.org/document-builder.sjs?html=example.org page"

    "https://example.org/document-builder.sjs?html=<meta charset=utf8>example.org page"

  );

  await onMarkupLoaded;
@@ -48,6 +48,9 @@ add_task(async function() { 
  ok(!btn.firstChild, "The frame list button doesn't have any children");



  // Open frame menu and wait till it's available on the screen.

  const frameMenu = toolbox.doc.getElementById("toolbox-frame-menu");

  info("Wait for the frame to be populated before opening the tooltip");

  await waitUntil(() => frameMenu.childNodes.length == 2);

  const panel = toolbox.doc.getElementById("command-button-frames-panel");

  btn.click();

  ok(panel, "popup panel has created.");