1788133 - Remove comments about cross origin iframes in devtools DAMP tests

Status: RESOLVED FIXED

(DevTools :: General, task, P3)

Description

[Julian Descottes [:jdescottes]]

In many DAMP test apges we can find the following comment

<!-- For now we load the iframe in the same site as the damp top frame.
       We will switch to a different-site URL (eg http://damp.iframe.com) when
       DevTools work against remote frames. -->

We now fully support remote frames so we could just go ahead and apply the suggestion. Or simply remove the comment if we don't want to change the setup.

Comment 1

[Julian Descottes [:jdescottes]]

Note that to actually switch to cross origin iframes, the debugger and inspector custom tests need to be updated because they currently directly reach into the frame via the top level page:

Example:
https://searchfox.org/mozilla-central/rev/b22ec3f983078ff98b04cee7dafe4b90342a42bf/testing/talos/talos/tests/devtools/addon/content/tests/inspector/custom.js#63-78

const contentMethod = function (_selector) {
  const { ContentDOMReference } = ChromeUtils.importESModule(
    "resource://gre/modules/ContentDOMReference.sys.mjs"
  );
  const iframe = content.document.querySelector("iframe");
  const win = iframe.contentWindow;
  const element = win.document.querySelector(_selector);
  const domReference = ContentDOMReference.get(element);
  sendAsyncMessage("get-dom-reference-done", domReference);
};

const wrappedMethod = encodeURIComponent(`function () {
  (${contentMethod})("${selector}");
 }`);

messageManager.loadFrameScript(`data:,(${wrappedMethod})()`, false);

I don't think we want to load something such as SpecialPowers in our performance tests, maybe we would still need a lightweight pair of JSWindowActors to evaluate scripts.

Comment 2

[Julian Descottes [:jdescottes]]

Note also that the styleeditor test can be migrate to use a cross origin frame but it does lead to a performance regression in a few tests:
https://perf.compare/subtests-compare-results?baseRev=e16e9ba62b8459c04bdae0375bf99c9a1c135617&baseRepo=try&newRev=40da685ddd74ab86d4e21153f35c5e3f411a2e6b&newRepo=try&framework=12&baseParentSignature=5288973&newParentSignature=5288973&filter_confidence=high

• custom.styleeditor.open.DAMP 4%
• custom.styleeditor.reload.DAMP 26%
• inspector.mutations 46%

The last one is surprising. It doesn't involve the styleeditor document, but the test runs right after the custom.styleeditor one. Also it only shows up on Linux, so might be a sideeffect due to a shifting GC ?

Overall, only custom.styleeditor.reload.DAMP seems to be consistently affected.

Comment 3

[Julian Descottes [:jdescottes]]

Attachment: Bug 1788133 - [devtools] Update comments about cross origin iframes in DAMP tests

Comment 4

[Julian Descottes [:jdescottes]]

Attachment: Bug 1788133 - [devtools] DAMP: use cross origin frame for styleeditor custom test

Depends on D243177

As mentioned in the bug, this regresses custom.styleeditor reload consistently on all platforms.
custom.styleeditor.open also slightly regresses but not visible on all platforms.
And the next test inspector.mutations also regresses significantly on Linux

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c499e4f31eb9
[devtools] Update comments about cross origin iframes in DAMP tests r=devtools-reviewers,perftest-reviewers,nchevobbe,fbilt
https://hg.mozilla.org/integration/autoland/rev/f8a44e555577
[devtools] DAMP: use cross origin frame for styleeditor custom test r=devtools-reviewers,perftest-reviewers,nchevobbe,fbilt

Comment 6

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/c499e4f31eb9
https://hg.mozilla.org/mozilla-central/rev/f8a44e555577