1735551 - Fix walkerFront#findNodeFront/NodeFront#waitForFrameLoad with cross origin iframes

Status: RESOLVED FIXED

(DevTools :: Inspector, defect, P3)

Description

[Nicolas Chevobbe [:nchevobbe]]

WalkerFront#findNodeFront [1] is used to retrieve a specific node front given an array of selectors representing the "path" to the node across frames.
It uses NodeFront#waitForFrameLoad as a way to wait for any iframe in the path to have its dom ready [2].

The issue is that to wait for the iframe to be ready, we're using the <iframe> element ownerDocument property.
And if the iframe document doesn't share the same-origin as the enclosing one, ownerDocument won't be available, and as a result, we won't wait for the iframe document to be loaded, which means we might try to query the dom while the element we're looking for isn't there yet.

waitForFrameLoad should probably try to navigate through the frames using the docShell, which would help when Fission is not enabled.
When fission is enabled, we'll have a dedicated target for the iframe, and we should look into a way of getting the information that it did load (e.g. watching for dom-interactive DOCUMENT_EVENT resource)

We might try to move findNodeFront and related functions to the inspector-command file, where they would make more sense than in the walker front.

[1] https://searchfox.org/mozilla-central/rev/b822a27de3947d3f4898defac6164e52caf1451b/devtools/client/fronts/walker.js#394-404,423
[2] https://searchfox.org/mozilla-central/rev/b822a27de3947d3f4898defac6164e52caf1451b/devtools/server/actors/inspector/node.js#677-680

Comment 1

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1735551 - [devtools] Turn WalkerFront#findNodeFront into a command. r=ochameau.

The callsites are migrated to the new command, except from the webconsole test
in which we replace usage with existing inspector test helper.

Comment 2

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1735551 - [devtools] Make NodeActor#waitForFrameLoad handle transient about:blank document. r=ochameau.

This will handle cases when Fission is not enabled.

Depends on D128712

Comment 3

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1735551 - [devtools] Make findNodeFrontFromSelectors command handle remote iframes. r=ochameau.

Depends on D128713

Comment 4

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1735551 - [devtools] Add a timeout to findNodeFrontFromSelectors. r=ochameau.

Since the method might wait for iframe to load, it could stay unsettled for a
while.
Adding a timeout will make the callsites not wait for ever, at the expense of
correctness.
This should be okay as this function is mainly called to re-set things after
reloading the page, which is a nice to have, but not critical feature.

Depends on D128714

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d046e576646a
[devtools] Turn WalkerFront#findNodeFront into a command. r=ochameau.
https://hg.mozilla.org/integration/autoland/rev/d445f5d8b08a
[devtools] Make NodeActor#waitForFrameLoad handle transient about:blank document. r=ochameau.
https://hg.mozilla.org/integration/autoland/rev/a351a8718aea
[devtools] Make findNodeFrontFromSelectors command handle remote iframes. r=ochameau.
https://hg.mozilla.org/integration/autoland/rev/5a2367e856d2
[devtools] Add a timeout to findNodeFrontFromSelectors. r=ochameau.

Comment 6

[Cristina Cozmuta (:CrissCozmuta)]

https://hg.mozilla.org/mozilla-central/rev/d046e576646a
https://hg.mozilla.org/mozilla-central/rev/d445f5d8b08a
https://hg.mozilla.org/mozilla-central/rev/a351a8718aea
https://hg.mozilla.org/mozilla-central/rev/5a2367e856d2