Closed Bug 1622347 Opened 5 years ago Closed 4 years ago

parsing-xhtml-documents/adopt-while-parsing-001.html.ini WPT ERROR with Fission

Categories

(Core :: DOM: HTML Parser, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla78
Fission Milestone M6b
Tracking Status
firefox76 --- wontfix
firefox77 --- wontfix
firefox78 --- fixed

People

(Reporter: cpeterson, Unassigned)

References

(Blocks 1 open bug)

Details

Priority: -- → P3

The DOM Fission team is relying on feature teams to debug and fix Fission failures in their tests. If the failure looks like a bug in Fission's DOM or IPC changes, you can send the bug back to me.

We're hoping to enable Fission for a subset of Nightly users in early Q3, so we would like WPT tests to be green for Fission by end of Q2. Whether a particular test failure actually blocks shipping Fission is up to the discretion of the feature team. You all would know better than the DOM Fission which test failures are most important.

You can enable Fission when running WPT tests locally using mach wpt --enable-fission.

Hi Alphan, please help us to understand the importance and priority of this bug, and do feel free to reset if you see fit. Thank you.

Flags: needinfo?(alchen)

The failure when enable fission is like below.

0:13.92 pid:4024 JavaScript error: http://web-platform.test:8000/html/the-xhtml-syntax/parsing-xhtml-documents/adopt-while-parsing.xhtml, line 5: SecurityError: Permission denied to access property "parsingInterrupted" on cross-origin object

The current design doesn't work with Fission.
Hi Emilio, since you are the author of this test, could you have some inputs here?

Flags: needinfo?(alchen) → needinfo?(emilio)

This seems like a fission bug. This iframe looks same-origin to me.

Flags: needinfo?(emilio)

(In reply to Emilio Cobos Álvarez (:emilio) from comment #4)

This seems like a fission bug. This iframe looks same-origin to me.

Kashav, this WPT failure looks like a Fission bug our team should investigate.

Flags: needinfo?(kmadan)

Running the test locally, I'm also seeing

UnknownError: SecurityError: Permission denied to access property "windowUtils" on cross-origin object
flushWindow@chrome://marionette/content/listener.js:1715:17
flushWindow@chrome://marionette/content/listener.js:1733:18
flushRendering@chrome://marionette/content/listener.js:1736:14
maybeResolve@chrome://marionette/content/listener.js:1809:7
paintComplete/<@chrome://marionette/content/listener.js:1824:5
paintComplete@chrome://marionette/content/listener.js:1807:10
reftestWait@chrome://marionette/content/listener.js:1765:9
async*dispatch/</req<@chrome://marionette/content/listener.js:525:17
dispatch/<@chrome://marionette/content/listener.js:520:15
MessageListener.receiveMessage*startListeners@chrome://marionette/content/listener.js:605:21
registerSelf@chrome://marionette/content/listener.js:501:5
@chrome://marionette/content/listener.js:1855:1

(we're trying to access windowUtils on a cross-origin window here.)

This is probably caused by Marionette being Fission-incompatible, I assume it'll be fixed in bug 1518468. I'm also seeing the SecurityError from comment #3, but I'm wondering if that's also caused by the Marionette thing. The frame calling parsingInterrupted is definitely same-origin.

Flags: needinfo?(kmadan)

(In reply to :kashav from comment #6)

This is probably caused by Marionette being Fission-incompatible, I assume it'll be fixed in bug 1518468.

Oops, bug 1574508 is already tracking this. The parsingInterrupted error is probably caused by something else then. Will update with what I find.

I've spent some time looking at what's going on here, but I'm off-ish for the next little while, so I'll dump my findings here. I'll pick this up again when I'm back, but someone else can take a look if they have any ideas.

:mccr8 had a suspicion that we end up in this block for the same-origin iframe, but, based on some logs, it looks like we never actually end up in that block for the entirety of the test (we thought we might not have been setting mIsInProcess on the associated browsing context in time, but that doesn't seem to be the case). It also appears that the only time we do call GetRemoteOuterWindowProxy is here (we call ToJSValue here, when trying to access the parent window from the subframe in the test).

When loading the subframe, I notice that we load the initial "about:blank" document in a new process, which is expected, but then we go through the whole "process switch" process, and load the final document in another new process. Something to note is that the reference window (which is opened and screenshotted first) and the "test" window do load in the same process, as expected.

I also tried loading the exact test page manually (with fission enabled), and noticed some discrepancies in the "ProcessSwitch" / E10SUtils logs:

  • When opening the page manually:

    console.info: ProcessSwitch: "currentRemoteType (webIsolated=http://web-platform.test) preferredRemoteType: webIsolated=http://web-platform.test"
    console.debug: ProcessSwitch: "validatedWebRemoteType()"
    console.debug: ProcessSwitch: "  aPreferredRemoteType: webIsolated=http://web-platform.test"
    console.debug: ProcessSwitch: "  aTargetUri: http://web-platform.test:3000/html/the-xhtml-syntax/parsing-xhtml-documents/adopt-while-parsing.xhtml"
    console.debug: ProcessSwitch: "  aCurrentUri: undefined"
    console.debug: ProcessSwitch: "  validatedWebRemoteType() returning: webIsolated=http://web-platform.test"
    console.debug: ProcessSwitch: "webIsolated=http://web-platform.test, webIsolated=http://web-platform.test, false"
    console.debug: ProcessSwitch: "type (webIsolated=http://web-platform.test) is compatible - ignoring"
    
  • and in the test:

    console.info: ProcessSwitch: "currentRemoteType (web) preferredRemoteType: web"
    console.debug: ProcessSwitch: "validatedWebRemoteType()"
    console.debug: ProcessSwitch: "  aPreferredRemoteType: web"
    console.debug: ProcessSwitch: "  aTargetUri: http://web-platform.test:8000/html/the-xhtml-syntax/parsing-xhtml-documents/adopt-while-parsing.xhtml"
    console.debug: ProcessSwitch: "  aCurrentUri: undefined"
    console.debug: ProcessSwitch: "  validatedWebRemoteType() returning: webIsolated=http://web-platform.test"
    console.debug: ProcessSwitch: "web, webIsolated=http://web-platform.test, false"
    

For this reason, I was leaning towards the problem being caused by something that the test runner is doing (I could be completely wrong). I haven't looked into the reftest runner much yet, but it looks like the main stuff starts in compareUrls, which opens the two URLs, captures the resulting windows, and then compares the captures.

Assignee: nobody → kmadan

I've looked into this more, I think I have a better idea of what's going on:

  • The two top-level http://web-platform.test:8000/... documents are loaded in a non-tabbrowser browser, and so we hit this process-switch abort ("browser is not loaded in a tab"). These documents end up loading in a process with remote type of web (instead of webIsolated=http://web-platform.test).
  • When we try to load the same-origin subframe, it does have a tabbrowser, and so we proceed with the process switch. We look for a process with remote type of webIsolated=http://web-platform.test, but can't find one, and load the subframe in a new process. This results in the subframe being in a different process than its embedder (even though they're same-origin), and this causes the cross-origin access error.
  • As a test, I hard-coded the browser's default remote type to be webIsolated=http://web-platform.test here. This fixes the test, but it's not really a viable solution, since we load different origins in other WPTs. I'm wondering if it'd make sense to create a separate xul:browser for each document instead (and then choose the correct remote type with E10SUtils).

I'm throwing some relevant links below, mostly for my own reference:

  • We open the reftest.xhtml window here, and dynamically create its xul:browser here (we set the default remote type a few lines below).
  • We request the load of a new document here. The navigation happens in this function (more specifically, by setting the location of the framescript window in this callback).

This is probably affecting other wpts as well.

Matt, I'm wondering if we can easily make DocumentLoadListener work for non-tabbrowser toplevel loads. If not, kmag mentioned that the regular (non-wpt) reftest harness had a similar problem with fission. Does this seem like it'll require a similar fix?

Flags: needinfo?(matt.woodrow)

That change is a bit of a hack, but is probably the simplest way forward to unblock you. The change was in bug 1599662, and I think something similar should be fairly easy.

It just does a lookup of the desired remote type each time we try to load a URL in the reftest harness, and changes the remote type on the <browser> element if it's different (and then makes sure we re-init the message manager etc in the new process).

Getting support for doing the switch from within DocumentLoadListener is a bit harder (but is what we want longer term). Happy to explain my thoughts on that if it's useful.

Flags: needinfo?(matt.woodrow)
Depends on: 1635904

Tracking WPT Fission bugs for Fission M6b (Q2)

Fission Milestone: M6 → M6b

Because this bug's Severity has not been changed from the default since it was filed, and it's Priority is P3 (Backlog,) indicating it has been triaged, the bug's Severity is being updated to S3 (normal.)

Assignee: kmadan → nobody
Severity: normal → S3

This was fixed by bug 1635904.

Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → WORKSFORME
Resolution: WORKSFORME → FIXED
Target Milestone: --- → mozilla78
You need to log in before you can comment on or make changes to this bug.