Closed Bug 1601245 Opened 2 years ago Closed 2 months ago

Enable Puppeteer and CDP tests in mochitest-remote for Fission

Categories

(Remote Protocol :: Agent, task, P2)

task
Points:
8

Tracking

(Fission Milestone:Future, firefox-esr78 disabled, firefox-esr91 disabled, firefox92 disabled, firefox93 disabled, firefox94 wontfix, firefox95 fixed)

RESOLVED FIXED
95 Branch
Fission Milestone Future
Tracking Status
firefox-esr78 --- disabled
firefox-esr91 --- disabled
firefox92 --- disabled
firefox93 --- disabled
firefox94 --- wontfix
firefox95 --- fixed

People

(Reporter: ato, Assigned: whimboo)

References

(Blocks 4 open bugs, Regressed 1 open bug, )

Details

(Whiteboard: [bidi-m2-mvp][dt-fission-future])

Attachments

(9 files, 1 obsolete file)

12.54 KB, text/plain
Details
10.51 KB, text/plain
Details
30.36 KB, text/plain
Details
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review

We should also run the remote agent browser-chrome tests, M(remote), with Fission enabled.

Priority: -- → P3

Note that with mach try fuzzy --full you can already run the remote tests with Fission on try. What's missing is that we run those by default.

Summary: Run M(remote) with Fission → Run M(remote) with Fission enabled by default (similar to other Mochitests)

Tracking for Fission Nightly (M6) for devtools tests

Whiteboard: [fission:m6]
See Also: → 1604259
Fission Milestone: --- → M6
Whiteboard: [fission:m6]

Tracking for Fission M6b Nightly milestone.

Fission Milestone: M6 → M6b
Fission Milestone: M6b → M6c

Moving Remote Protocol bugs to Fission Beta milestone (M7).

Fission Milestone: M6c → M7
Whiteboard: dt-fission-future

Honza said this is not part of dt-fission-m3-mvp and is not considered Fission M7 blocker.

Fission Milestone: M7 → ---

dt-fission-future don't need to block Fission MVP.

Fission Milestone: --- → Future
Status: NEW → RESOLVED
Closed: 9 months ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1694836

Andrew, we cannot dupe it yet. Remote Agent is not Fission compatible and tests will fail all over the place. As such when mochitests get enabled on more platforms please exclude the Remote Agent browser chrome ones.

We should keep this bug open for later (until we have made a decision).

Flags: needinfo?(ahal)
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---

My mistake, I thought the title was just about CI configuration. I'll change it to be a bit more clear and block the other bug.

Blocks: 1694836
Flags: needinfo?(ahal)
Summary: Run M(remote) with Fission enabled by default (similar to other Mochitests) → Make remote agent Fission compatible

Well, there is bug 1601245 for making Remote Agent Fission compatible. So whether we dupe it against that one, or really keep it as separate bug. Up to you. For now reverting the summary back so it covers the tests.

Summary: Make remote agent Fission compatible → Run M(remote) with Fission enabled by default (similar to other Mochitests)

I'm confused... so this should be duped against bug 1694836 after all? What is the difference between this and that?

If they are both about running the tasks in CI, I vote we use the other one as it is more explicit about which tasks to enable and it is already copied to our team's JIRA backlog.

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+1] from comment #10)

Well, there is bug 1601245 for making Remote Agent Fission compatible. So whether we dupe it against that one, or really keep it as separate bug. Up to you. For now reverting the summary back so it covers the tests.

Henrik, you said bug 1601245 is for making Remote Agent Fission compatible, but that is the bug # for this bug. Did you paste the wrong bug number? :)

(In reply to Neha Kochar [:neha] from comment #5)

Honza said this is not part of dt-fission-m3-mvp and is not considered Fission M7 blocker.

If running the mochitest-remote tests doesn't block shipping Fission, what should we do with these tests when we make Fission the default? Will we need to explicitly disable Fission when running mochitest-remote (until the Remote Agent is Fission compatible)?

Flags: needinfo?(hskupin)

(In reply to Andrew Halberstadt [:ahal] from comment #11)

If they are both about running the tasks in CI, I vote we use the other one as it is more explicit about which tasks to enable and it is already copied to our team's JIRA backlog.

There is still no clarity yet if we want to make the Remote Agent and specifically its CDP related code Fission compatible. As long as this isn't done the Puppeteer tests with Fission enabled are not a real target due to massive failures. As such I don't think that we should make them part of bug 1694836, which itself will most likely be fixed kinda soon.

Flags: needinfo?(hskupin)

(In reply to Chris Peterson [:cpeterson] from comment #12)

Henrik, you said bug 1601245 is for making Remote Agent Fission compatible, but that is the bug # for this bug. Did you paste the wrong bug number? :)

Sorry, I meant the meta bug 1600054 that is blocked by this bug.

(In reply to Neha Kochar [:neha] from comment #5)

Honza said this is not part of dt-fission-m3-mvp and is not considered Fission M7 blocker.

If running the mochitest-remote tests doesn't block shipping Fission, what should we do with these tests when we make Fission the default? Will we need to explicitly disable Fission when running mochitest-remote (until the Remote Agent is Fission compatible)?

That would be the expected step, yes. That's why we currently also force Fission disabled for Puppeteer.

Status: REOPENED → RESOLVED
Closed: 9 months ago6 months ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1694836

Oh sorry, I made the same mistake again :)

Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---

In Bug 1723026, we are already enabling mochitest-remote for Fission jobs in order to run WebDriverBiDi tests against Fission.
We are going to skip individual CDP test suites in browser.ini instead of disabling Fission via the mochitest-remote job configuration.

Repurposing this bug to be about removing the individual skip-if from those browser.ini

Depends on: 1723026
Summary: Run M(remote) with Fission enabled by default (similar to other Mochitests) → Enable CDP test suites in mochitest-remote with Fission

So some good news. We might be close to enable both Puppeteer and CDP browser-chrome tests for Fission with the following preferences set:

  fission.bfcacheInParent=false
  fission.webContentIsolationStrategy=0

We have to consider this solution as workaround until CDP is fully Fission compatible. But it's ok to set this preferences for the time being to get CDP working at all given that Fission will be default in release builds soon. As discussed with Nika both of the preferences will exist for a while, and should get us going to start work towards Fission compatibility. Still happy to get more details about that from Nika if there are ETAs set to remove the prefs.

For Puppeteer it means that we have to remove forcing off Fission, and instead provide the above mentioned preferences as defaults for the Firefox launcher. If Fission is off they don't have an impact through, which means Puppeteer will work in both modes.

There are still a couple tests that fail locally mostly around events. I'll further investigate once I have a better overview from the following try job:
https://treeherder.mozilla.org/jobs?repo=try&revision=1c38f2727694204e8b8208e20e544cf4d6877d5b

Assignee: nobody → hskupin
Status: REOPENED → ASSIGNED
Flags: needinfo?(nika)
Summary: Enable CDP test suites in mochitest-remote with Fission → Enable Puppeteer and CDP tests in mochitest-remote for Fission
Priority: P3 → P2
Whiteboard: dt-fission-future → [bidi-m2-mvp][dt-fission-future]

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+1] from comment #18)

We have to consider this solution as workaround until CDP is fully Fission compatible. But it's ok to set this preferences for the time being to get CDP working at all given that Fission will be default in release builds soon. As discussed with Nika both of the preferences will exist for a while, and should get us going to start work towards Fission compatibility. Still happy to get more details about that from Nika if there are ETAs set to remove the prefs.

I don't think that the prefs will be disappearing any time soon, though I don't think new features will be built with keeping this config working long-term.

Flags: needinfo?(nika)
Depends on: 1732250
Blocks: 1732263
Comment on attachment 9242704 [details]
Page load events when navigating to a page with a frame (Fission disabled)

Obsoleted attachment
Attachment #9242704 - Attachment is obsolete: true

Stripped down list of commands, responses, and events with simplified ids as sent out by Chrome:

SEND ► {"sessionId":"1","method":"Page.navigate","params":{"url":"https://root.document","frameId":"1"},"id":16} +2ms
RECV ◀ {"method":"Network.requestWillBeSent","params":{"requestId":"1","loaderId":"1","documentURL":"https://root.document","request":{..},"sessionId":"1"} +5ms
RECV ◀ {"method":"Network.responseReceived","params":{"requestId":"1","loaderId":"1","timestamp":364964.131939,"type":"Document","response":{..},"sessionId":"1"} +2ms
RECV ◀ {"id":16,"result":{"frameId":"1","loaderId":"1"},"sessionId":"1"} +2ms
RECV ◀ {"method":"Page.frameStartedLoading","params":{"frameId":"1"},"sessionId":"1"} +2ms
RECV ◀ {"method":"Page.lifecycleEvent","params":{"frameId":"1","loaderId":"1","name":"init","timestamp":364964.135438},"sessionId":"1"} +1ms
RECV ◀ {"method":"Runtime.executionContextsCleared","params":{},"sessionId":"1"} +20ms
RECV ◀ {"method":"Page.frameNavigated","params":{"frame":{"id":"1","loaderId":"1","url":"https://root.document",..},"sessionId":"1"} +0ms
RECV ◀ {"method":"Runtime.executionContextCreated","params":{"context":{"id":3,"origin":"https://root.document","name":"",..},"sessionId":"1"} +1ms
RECV ◀ {"method":"Runtime.executionContextCreated","params":{"context":{"id":4,"origin":"://","name":"__puppeteer_utility_world__",..},"sessionId":"1"} +1ms
RECV ◀ {"method":"Page.frameAttached","params":{"frameId":"2","parentFrameId":"1"},"sessionId":"1"} +2ms
RECV ◀ {"method":"Page.lifecycleEvent","params":{"frameId":"2","loaderId":"2","name":"init","timestamp":364964.176966},"sessionId":"1"} +0ms
RECV ◀ {"method":"Page.lifecycleEvent","params":{"frameId":"2","loaderId":"2","name":"DOMContentLoaded","timestamp":364964.177288},"sessionId":"1"} +0ms
RECV ◀ {"method":"Page.frameStartedLoading","params":{"frameId":"2"},"sessionId":"1"} +0ms
RECV ◀ {"method":"Network.requestWillBeSent","params":{"requestId":"2","loaderId":"2","documentURL":"https://frame.document/","request":{..},"sessionId":"1"} +4ms
RECV ◀ {"method":"Page.domContentEventFired","params":{"timestamp":364964.22072},"sessionId":"1"} +2ms
RECV ◀ {"method":"Page.lifecycleEvent","params":{"frameId":"1","loaderId":"1","name":"DOMContentLoaded","timestamp":364964.22072},"sessionId":"1"} +0ms
RECV ◀ {"method":"Network.responseReceived","params":{"requestId":"2","loaderId":"2","timestamp":364964.721253,"type":"Document","response":{..},"sessionId":"1"} +1ms
RECV ◀ {"method":"Target.targetCreated","params":{"targetInfo":{"targetId":"2","type":"iframe","title":"","url":"","attached":false,"canAccessOpener":false,"browserContextId":"6E3F1792BA973F0C0AE03CF90C99BF56"}}} +1ms
RECV ◀ {"method":"Target.attachedToTarget","params":{"sessionId":"2","targetInfo":{"targetId":"2","type":"iframe","title":"","url":"","attached":true,"canAccessOpener":false,"browserContextId":"6E3F1792BA973F0C0AE03CF90C99BF56"},"waitingForDebugger":false},"sessionId":"1"} +0ms
puppeteer:frame The frame '2' moved to another session. Out-of-process iframes (OOPIF) are not supported by Puppeteer yet. https://github.com/puppeteer/puppeteer/issues/2548 +0ms
SEND ► {"sessionId":"2","method":"Runtime.enable","id":17} +1s
RECV ◀ {"method":"Runtime.executionContextCreated","params":{"context":{"id":1,"origin":"https://frame.document","name":"","uniqueId":"6522317831718132631.-7764833264105841202","auxData":{"isDefault":true,"type":"default","frameId":"2"}}},"sessionId":"2"} +0ms
RECV ◀ {"id":17,"result":{},"sessionId":"2"} +0ms
RECV ◀ {"method":"Page.frameStoppedLoading","params":{"frameId":"2"},"sessionId":"1"} +0ms
RECV ◀ {"method":"Page.frameDetached","params":{"frameId":"2","reason":"swap"},"sessionId":"1"} +1ms
RECV ◀ {"method":"Page.loadEventFired","params":{"timestamp":364964.752165},"sessionId":"1"} +20ms
RECV ◀ {"method":"Page.lifecycleEvent","params":{"frameId":"1","loaderId":"1","name":"load","timestamp":364964.752165},"sessionId":"1"} +1ms
RECV ◀ {"method":"Page.frameStoppedLoading","params":{"frameId":"1"},"sessionId":"1"} +20ms

Stripped down list of commands, responses, and events with simplified ids as sent out by Firefox (not Fission):

SEND ► {"sessionId":"1","method":"Page.navigate","params":{"url":"https://root.document/","frameId":"1"},"id":16} +49ms
RECV ◀ {"method":"Network.requestWillBeSent","params":{"requestId":"1","loaderId":"1","documentURL":"https://root.document/","request":{..},"sessionId":"1"} +10ms
RECV ◀ {"method":"Network.responseReceived","params":{"requestId":"1","loaderId":"1","timestamp":1632410094.564,"type":"Document","response":{..},"sessionId":"1"} +697ms
RECV ◀ {"id":16,"result":{"frameId":"1","loaderId":"1"},"sessionId":"1"} +4ms
RECV ◀ {"method":"Runtime.executionContextDestroyed","params":{"executionContextId":1},"sessionId":"1"} +0ms
RECV ◀ {"method":"Runtime.executionContextDestroyed","params":{"executionContextId":2},"sessionId":"1"} +0ms
RECV ◀ {"method":"Runtime.executionContextsCleared","params":{},"sessionId":"1"} +1ms
RECV ◀ {"method":"Page.frameStartedLoading","params":{"frameId":"1"},"sessionId":"1"} +0ms
RECV ◀ {"method":"Page.lifecycleEvent","params":{"frameId":"1","loaderId":"1","name":"init","timestamp":1632410094.568},"sessionId":"1"} +1ms
RECV ◀ {"method":"Page.frameNavigated","params":{"frame":{"id":"1","loaderId":"1","url":"https://root.document/","securityOrigin":null,"mimeType":null}},"sessionId":"1"} +2ms
RECV ◀ {"method":"Runtime.executionContextCreated","params":{"context":{"id":3,"origin":"https://root.document/","name":"","auxData":{"isDefault":true,"frameId":"1","type":"default"}}},"sessionId":"1"} +1ms
RECV ◀ {"method":"Runtime.executionContextCreated","params":{"context":{"id":4,"origin":"https://root.document/","name":"__puppeteer_utility_world__","auxData":{"isDefault":false,"frameId":"1","type":"isolated"}}},"sessionId":"1"} +15ms
RECV ◀ {"method":"Page.domContentEventFired","params":{"timestamp":1632410094.576},"sessionId":"1"} +9ms
RECV ◀ {"method":"Page.lifecycleEvent","params":{"frameId":"1","loaderId":"1","name":"DOMContentLoaded","timestamp":1632410094.576},"sessionId":"1"} +0ms
RECV ◀ {"method":"Network.requestWillBeSent","params":{"requestId":"2","loaderId":"2","documentURL":"https://root.document/","request":{..},"sessionId":"1"} +8ms
RECV ◀ {"method":"Network.responseReceived","params":{"requestId":"2","loaderId":"2","timestamp":1632410094.843,"type":"Subdocument","response":{..},"sessionId":"1"} +236ms
RECV ◀ {"method":"Page.frameAttached","params":{"frameId":"2","parentFrameId":"1","stack":null},"sessionId":"1"} +7ms
RECV ◀ {"method":"Page.frameStartedLoading","params":{"frameId":"2"},"sessionId":"1"} +0ms
RECV ◀ {"method":"Page.lifecycleEvent","params":{"frameId":"2","loaderId":"2","name":"init","timestamp":1632410094.848},"sessionId":"1"} +0ms
RECV ◀ {"method":"Page.frameNavigated","params":{"frame":{"id":"2","parentId":"1","loaderId":"2","url":"https://frame.document/","name":"","securityOrigin":null,"mimeType":null}},"sessionId":"1"} +0ms
RECV ◀ {"method":"Runtime.executionContextCreated","params":{"context":{"id":5,"origin":"https://frame.document/","name":"","auxData":{"isDefault":true,"frameId":"2","type":"default"}}},"sessionId":"1"} +1ms
RECV ◀ {"method":"Runtime.executionContextCreated","params":{"context":{"id":6,"origin":"https://frame.document/","name":"__puppeteer_utility_world__","auxData":{"isDefault":false,"frameId":"2","type":"isolated"}}},"sessionId":"1"} +1ms
RECV ◀ {"method":"Page.lifecycleEvent","params":{"frameId":"2","loaderId":"2","name":"DOMContentLoaded","timestamp":1632410094.851},"sessionId":"1"} +1ms
RECV ◀ {"method":"Page.lifecycleEvent","params":{"frameId":"2","loaderId":"2","name":"load","timestamp":1632410094.852},"sessionId":"1"} +0ms
RECV ◀ {"method":"Page.navigatedWithinDocument","params":{"frameId":"2","url":"https://frame.document/"},"sessionId":"1"} +0ms
RECV ◀ {"method":"Page.frameStoppedLoading","params":{"frameId":"2"},"sessionId":"1"} +0ms
RECV ◀ {"method":"Page.loadEventFired","params":{"timestamp":1632410094.853},"sessionId":"1"} +4ms
RECV ◀ {"method":"Page.lifecycleEvent","params":{"frameId":"1","loaderId":"1","name":"load","timestamp":1632410094.853},"sessionId":"1"} +0ms
RECV ◀ {"method":"Page.navigatedWithinDocument","params":{"frameId":"1","url":"https://root.document/"},"sessionId":"1"} +1ms
RECV ◀ {"method":"Page.frameStoppedLoading","params":{"frameId":"1"},"sessionId":"1"} +0ms

Stripped down list of commands, responses, and events with simplified ids as sent out by Firefox (Fission):

SEND ► {"sessionId":"1","method":"Page.navigate","params":{"url":"https://root.document/","frameId":"1"},"id":16} +53ms
RECV ◀ {"method":"Network.requestWillBeSent","params":{"requestId":"1","loaderId":"1","documentURL":"https://root.document/","request":{..},"sessionId":"1"} +9ms
RECV ◀ {"method":"Network.responseReceived","params":{"requestId":"1","loaderId":"1","timestamp":1632410149.287,"type":"Document","response":{..},"sessionId":"1"} +827ms
RECV ◀ {"id":16,"result":{"frameId":"1","loaderId":"1"},"sessionId":"1"} +5ms
RECV ◀ {"method":"Page.frameStartedLoading","params":{"frameId":"1"},"sessionId":"1"} +3ms
RECV ◀ {"method":"Page.lifecycleEvent","params":{"frameId":"1","loaderId":"1","name":"init","timestamp":1632410149.292},"sessionId":"1"} +0ms
RECV ◀ {"method":"Runtime.executionContextDestroyed","params":{"executionContextId":1},"sessionId":"1"} +4ms
RECV ◀ {"method":"Runtime.executionContextDestroyed","params":{"executionContextId":2},"sessionId":"1"} +0ms
RECV ◀ {"method":"Runtime.executionContextsCleared","params":{},"sessionId":"1"} +0ms
RECV ◀ {"method":"Page.frameNavigated","params":{"frame":{"id":"1","loaderId":"1","url":"https://root.document/","securityOrigin":null,"mimeType":null}},"sessionId":"1"} +0ms
RECV ◀ {"method":"Runtime.executionContextCreated","params":{"context":{"id":3,"origin":"https://root.document/","name":"","auxData":{"isDefault":true,"frameId":"1","type":"default"}}},"sessionId":"1"} +1ms
RECV ◀ {"method":"Runtime.executionContextCreated","params":{"context":{"id":4,"origin":"https://root.document/","name":"__puppeteer_utility_world__","auxData":{"isDefault":false,"frameId":"1","type":"isolated"}}},"sessionId":"1"} +15ms
RECV ◀ {"method":"Page.domContentEventFired","params":{"timestamp":1632410149.302},"sessionId":"1"} +8ms
RECV ◀ {"method":"Page.lifecycleEvent","params":{"frameId":"1","loaderId":"1","name":"DOMContentLoaded","timestamp":1632410149.302},"sessionId":"1"} +0ms
RECV ◀ {"method":"Network.requestWillBeSent","params":{"requestId":"2","loaderId":"2","documentURL":"https://root.document/","request":{..},"sessionId":"1"} +3ms
RECV ◀ {"method":"Network.responseReceived","params":{"requestId":"2","loaderId":"2","timestamp":1632410149.773,"type":"Subdocument","response":{..},"sessionId":"1"} +440ms
RECV ◀ {"method":"Page.frameStartedLoading","params":{"frameId":"2"},"sessionId":"1"} +0ms
RECV ◀ {"method":"Page.lifecycleEvent","params":{"frameId":"2","loaderId":"2","name":"init","timestamp":1632410149.783},"sessionId":"1"} +0ms
RECV ◀ {"method":"Page.frameAttached","params":{"frameId":"2","parentFrameId":"1","stack":null},"sessionId":"1"} +5ms
RECV ◀ {"method":"Page.frameStartedLoading","params":{"frameId":"2"},"sessionId":"1"} +1ms
RECV ◀ {"method":"Page.lifecycleEvent","params":{"frameId":"2","loaderId":"2","name":"init","timestamp":1632410149.784},"sessionId":"1"} +0ms
RECV ◀ {"method":"Page.frameNavigated","params":{"frame":{"id":"2","parentId":"1","loaderId":"2","url":"https://frame.document/","name":"","securityOrigin":null,"mimeType":null}},"sessionId":"1"} +0ms
RECV ◀ {"method":"Runtime.executionContextCreated","params":{"context":{"id":5,"origin":"https://frame.document/","name":"","auxData":{"isDefault":true,"frameId":"2","type":"default"}}},"sessionId":"1"} +0ms
RECV ◀ {"method":"Runtime.executionContextCreated","params":{"context":{"id":6,"origin":"https://frame.document/","name":"__puppeteer_utility_world__","auxData":{"isDefault":false,"frameId":"2","type":"isolated"}}},"sessionId":"1"} +3ms
RECV ◀ {"method":"Page.lifecycleEvent","params":{"frameId":"2","loaderId":"2","name":"DOMContentLoaded","timestamp":1632410149.787},"sessionId":"1"} +2ms
RECV ◀ {"method":"Page.lifecycleEvent","params":{"frameId":"2","loaderId":"2","name":"load","timestamp":1632410149.788},"sessionId":"1"} +1ms
RECV ◀ {"method":"Page.navigatedWithinDocument","params":{"frameId":"2","url":"https://frame.document/"},"sessionId":"1"} +0ms
RECV ◀ {"method":"Page.frameStoppedLoading","params":{"frameId":"2"},"sessionId":"1"} +0ms
RECV ◀ {"method":"Page.loadEventFired","params":{"timestamp":1632410149.789},"sessionId":"1"} +0ms
RECV ◀ {"method":"Page.lifecycleEvent","params":{"frameId":"1","loaderId":"1","name":"load","timestamp":1632410149.789},"sessionId":"1"} +1ms
RECV ◀ {"method":"Page.navigatedWithinDocument","params":{"frameId":"1","url":"https://root.document/"},"sessionId":"1"} +1ms
RECV ◀ {"method":"Page.frameStoppedLoading","params":{"frameId":"1"},"sessionId":"1"} +0ms
Blocks: 1694127
Points: --- → 8

I've applied my changes to Puppeteer in the upstream repository, and triggered a CI job that tests the recent Nightly build on all platforms and not just Linux like in our CI. The result is amazing... we pass all the to-run unit tests:

https://github.com/puppeteer/puppeteer/pull/7610/checks?check_run_id=3734585540

As such I also tried locally on MacOS via npm run funit with the same result. Everything is fine.

So why does it fail when we run via mach? Here some steps that I want to do:

  1. Update our downstream sync of Puppeteer to 10.4.0
  2. Check if we specify some custom arguments when running via mach, which might enable some extra features
  3. Once local tests pass I'll check for failures in CI

Nevertheless given that all tests are passing I assume that the order of events that we sent out is not important. That will help a lot.

Depends on: 1732958

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+1] from comment #28)

  1. Update our downstream sync of Puppeteer to 10.4.0

This update brings in a lot of test changes, so I'm reluctant in actually doing that now. Lets first get the Puppeteer job running for Fission.

  1. Check if we specify some custom arguments when running via mach, which might enable some extra features
  2. Once local tests pass I'll check for failures in CI

As I've noticed yesterday there is bug 1651542 that is causing trouble for the CI job. Due to a missing font-cache initialization during the docker image generation the first start of Firefox takes about 15s. Because the overall test timeout in Puppeteer is 20s it always causes the browser to be killed. Adding the same workaround (run firefox --screenshot) before, makes the issue go away. Here a CI job that shows us green jobs:

https://treeherder.mozilla.org/jobs?repo=try&revision=d91121a9cee2f4a79e7a624a96c5c30309a5d277&searchStr=pup

Nevertheless given that all tests are passing I assume that the order of events that we sent out is not important. That will help a lot.

Based on the recent results I'll go ahead and remove event order assertions in our browser chrome tests where needed.

No longer depends on: 1732958
Depends on: 1733255
Depends on: 1733463

Currently most of the browser-chrome tests for navigation make
use of data urls, which actually cause process switches due to
cross-origin requests with Fission enabled.

By using the pages as served by httpd.js we can force the same
navigation events to be emitted when Fission is enabled or disabled.

Pushed by hskupin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/67db7d4b74ad
[remote] Use same-origin URLs for browser-chrome navigation tests. r=webdriver-reviewers,jdescottes

The required code changes will land in batches. For now I just pushed some basic changes to avoid unnecessary browsing context changes in our browser-chrome tests, to make them behave identical between Fission and non-Fission.

I still encounter issues with the usage of DOMWindowCreated instead of webnavigation-create, which I have to solve before being able to upload the remaining patches.

A recent try build with all the patches can be found at:
https://treeherder.mozilla.org/jobs?repo=try&revision=a2bee81260ffca0bda987b58c9f816ccdd75d850

Keywords: leave-open

I pushed another try build earlier today:
https://treeherder.mozilla.org/jobs?repo=try&revision=22c14bf07008722fc2519d0e3b46215537e41500

This showed one remaining issue for Puppeteer jobs but this time in non-Fission mode. After some investigation I noticed that Puppeteer relies on the Page.frameAttached event to happen before any network request/response is done for that particular frame. By using DOMWindowCreated for Fission this is all fine given that we pre-load about:blank in that frame before loading the real URL. But with Fission disabled this event fires too late, and Puppeteer cannot associate the network request/response to the new frame because it's id isn't know at this time yet. As such tests are timing out for related promises. Also one of our browser-chrome tests fail due to that incompatible order.

As solution I'm going to keep the usage of webnavigation-create for non-Fission builds, which makes sure that Page.frameAttached gets fired as early as possible.

Here a new try build:
https://treeherder.mozilla.org/jobs?repo=try&revision=9311b2685eb4c7add14dcb2ead1c42eee3b048f7

Because with Fission enabled the "DOMWindowCreated" event
is emitted way early it can be used to detect when a new
frame has been created.

But with Fission disabled the event is received too late,
and as such the appropriate network requests and responses
for the document have already been done. That means that
Puppeteer actually hangs waiting for them.

As such keep using the "webnavigation-create" observer
notification for Fission-disabled builds for now.

Depends on D127520

The amount of time as spent on this bug should be preferable 13 points. Putting back for triage in the next meeting on Monday.

Points: 8 → ?
Attachment #9244311 - Attachment description: Bug 1601245 - [remote] Fix the handling of life-cycle events for Fission. → Bug 1601245 - [remote] Fix the handling of life-cycle "init" events for initial about:blank pages.
Pushed by hskupin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/88c7f76c4fc8
[remote] Update browser-chrome tests to assert on event count and not event order. r=webdriver-reviewers,jdescottes
https://hg.mozilla.org/integration/autoland/rev/33b2332510dd
[remote] With Fission enabled use "DOMWindowCreated" event to detect newly created frames. r=webdriver-reviewers,jdescottes
https://hg.mozilla.org/integration/autoland/rev/bec94f6ef8a7
[remote] Fix the handling of life-cycle "init" events for initial about:blank pages. r=webdriver-reviewers,jdescottes
https://hg.mozilla.org/integration/autoland/rev/c48240f42eda
[remote] Use "document-element-inserted" notification to determine when a frame is ready. r=webdriver-reviewers,jdescottes
https://hg.mozilla.org/integration/autoland/rev/58f77625e990
[remote] Enable Puppeteer and CDP browser-chrome mochitests for Fission. r=webdriver-reviewers,jdescottes

Hi Chris, would you like to see an uplift of my recent patches to the beta branch? Or is it fine with letting it ride the trains and forcing Fission to be disabled for Puppeteer up to the 94 release?

Status: ASSIGNED → RESOLVED
Closed: 6 months ago2 months ago
Flags: needinfo?(cpeterson)
Keywords: leave-open
Resolution: --- → FIXED
Target Milestone: --- → 95 Branch
Blocks: 1734354

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+1] from comment #44)

Hi Chris, would you like to see an uplift of my recent patches to the beta branch? Or is it fine with letting it ride the trains and forcing Fission to be disabled for Puppeteer up to the 94 release?

You're the subject matter expert, so I defer to your recommendation.

Will Puppeteer users need to manually disable Fission themselves in 94 or will Puppeteer handle that automatically? How will this workaround be communicated to Puppeteer users?

Fission will be slowly rolled out to 94 release users over four weeks, so for better or worse, Fission is not a problem that Puppeteer users will all experience on day 1 of the 94 release.

Flags: needinfo?(cpeterson) → needinfo?(hskupin)

(In reply to Chris Peterson [:cpeterson] from comment #45)

Will Puppeteer users need to manually disable Fission themselves in 94 or will Puppeteer handle that automatically? How will this workaround be communicated to Puppeteer users?

There is nothing users of Puppeteer will actually have to do. It's all done by Puppeteer itself. Right now it forces Fission to be disabled.

Whenever I'm happy with our test jobs I'm going to propose the following change to Puppeteer:
https://github.com/puppeteer/puppeteer/pull/7610/files

This will no longer force disable Fission, but allow this feature to be enabled or disabled, while turning off the BFCacheInParent and forcing all web pages in a single shared content process.

So as long as the fission.autostart preference will be around we don't have a problem. Note that Puppeteer right now pulls a Nightly build of Firefox, and we want to change that to a release build. Uplifting would speed it up by 4 weeks, but we could take the safe path and just let it ride the trains.

Flags: needinfo?(hskupin)

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+1] from comment #46)

This will no longer force disable Fission, but allow this feature to be enabled or disabled, while turning off the BFCacheInParent and forcing all web pages in a single shared content process.

So as long as the fission.autostart preference will be around we don't have a problem. Note that Puppeteer right now pulls a Nightly build of Firefox, and we want to change that to a release build. Uplifting would speed it up by 4 weeks, but we could take the safe path and just let it ride the trains.

SGTM. The fission.autostart preference definitely won't be removed in 94 or 95. It might be removed in Nightly 96 (November) or 97 (December).

Setting status-firefox94=wontfix since we can let your fixes ride the trains with 95.

My patch for Puppeteer upstream has been merged and will be part of the next Puppeteer release. I mentioned that we will get started to no longer allow disabling Fission, and that shipping a Puppeteer release would be required.

Regressions: 1734896
Regressions: 1735131
Points: ? → 8
You need to log in before you can comment on or make changes to this bug.