1642676 - Select an explicit remoteType for remote workers and then check that it matches the one set on the child process

Status: RESOLVED FIXED

(Core :: DOM: Workers, enhancement, P3)

Description

[Luca Greco [:rpl] [:luca] [:lgreco]]

+++ This bug was initially created as a clone of Bug #1612690 +++

Follow up for Bug #1612690, from which we landed changes to fix a subset of the issues related to the selecting the child process remoteType for the remote workers (in particular making sure that service workers were only launched is web content process types).

In this followup we are going to move the two patches that:

• explicitly select a remoteType for the remote workers and then check (in the child process) that the selected remoteType does match the one set on the child process where the remote worker is being launched
• a new test case to make sure that we have coverage for the RemoteWorkerManager's LaunchNewContentProcess and RegisterActor methods

Comment 1

[Luca Greco [:rpl] [:luca] [:lgreco]]

Attachment: Bug 1642676 - Ensure remote workers are launched in a child process based on the expected remoteType. r=asuth!

Comment 2

[Luca Greco [:rpl] [:luca] [:lgreco]]

Attachment: Bug 1642676 - Add test case to ensure we have test coverage for RemoteWorkerManager LaunchNewContentProcess and RegisterActor methods. r=asuth!

Depends on D61708

Comment 3

[Luca Greco [:rpl] [:luca] [:lgreco]]

Hi Andrew, as we agreed last week I filed a separate follow up issue and moved D61708 and D63697 from Bug 1612690 to this newly filed bugzilla issue (and also tweaked the testing approach in D63697 as we discussed).

I'm needinfo-ing you just to let you know where the two phabricator revision has been moved and that they are up for a new review round (as agreed in our last meeting).

Comment 4

[Luca Greco [:rpl] [:luca] [:lgreco]]

Attachment: Bug 1642676 - Move test case for SharedWorker disallowed on in-process-webextensions mode into a separate test file. r?mixedpuppy!

Depends on D63697

Comment 5

[Luca Greco [:rpl] [:luca] [:lgreco]]

Clear needinfo assigned to asuth (Andrew and I already discussed about this bugzilla issue over slack)

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ca0591418c37
Ensure remote workers are launched in a child process based on the expected remoteType. r=asuth,mixedpuppy
https://hg.mozilla.org/integration/autoland/rev/110cd0433bbe
Add test case to ensure we have test coverage for RemoteWorkerManager LaunchNewContentProcess and RegisterActor methods. r=dom-workers-and-storage-reviewers,asuth
https://hg.mozilla.org/integration/autoland/rev/166624b9d855
Move test case for SharedWorker disallowed on in-process-webextensions mode into a separate test-file. r=mixedpuppy

Comment 7

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/65edebab7a93
Backed out 3 changesets fro sharedworker related failures. CLOSED TREE

Comment 8

[Cosmin Sabou [:CosminS]]

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&resultStatus=testfailed%2Cbusted%2Cexception&tochange=65edebab7a932e978ee40a98f56e6ce6c65322d1&fromchange=166624b9d855cbcb38f5f11c3cab3e3e66e8f825&failure_classification_id=2&selectedTaskRun=Afb0VYEkSTGOXJintK7_yQ.0

Failure logs: https://treeherder.mozilla.org/logviewer.html#?job_id=307274326&repo=autoland
https://treeherder.mozilla.org/logviewer.html#?job_id=307273503&repo=autoland
https://treeherder.mozilla.org/logviewer.html#?job_id=307274224&repo=autoland
https://treeherder.mozilla.org/logviewer.html#?job_id=307274235&repo=autoland

Backout link: https://hg.mozilla.org/integration/autoland/rev/65edebab7a932e978ee40a98f56e6ce6c65322d1

Comment 9

[Luca Greco [:rpl] [:luca] [:lgreco]]

(In reply to Cosmin Sabou [:CosminS] from comment #8)

Failure logs: https://treeherder.mozilla.org/logviewer.html#?job_id=307274326&repo=autoland
https://treeherder.mozilla.org/logviewer.html#?job_id=307273503&repo=autoland
https://treeherder.mozilla.org/logviewer.html#?job_id=307274224&repo=autoland
https://treeherder.mozilla.org/logviewer.html#?job_id=307274235&repo=autoland

These failures look all related to tests running in Fission-enabled jobs, which honestly I should have actually expected to happen ([1]) and checked upfront.

I'm evaluating some options to deal with these failures and I'm going to discuss them with :asuth to agreed on a preferred approach.

I'm going to re-aiming to have this ready to be pushed on autoland on early 80 cycle.

[1]: because as part of the change there is an explicit check in RemoteWorkerChild that is verify that the remote worker remoteType does match the content process remoteType and explicitly fail it doesn't, and that is what is triggering this failure. In my opinion we should keep that check in place but also deal with Fission-enabled jobs more explicitly than I originally agreed with :asuth

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fd12e8fa9699
Ensure remote workers are launched in a child process based on the expected remoteType. r=asuth,mixedpuppy
https://hg.mozilla.org/integration/autoland/rev/1d3d39aae183
Add test case to ensure we have test coverage for RemoteWorkerManager LaunchNewContentProcess and RegisterActor methods. r=dom-workers-and-storage-reviewers,asuth
https://hg.mozilla.org/integration/autoland/rev/9bddaf4551d6
Move test case for SharedWorker disallowed on in-process-webextensions mode into a separate test file. r=mixedpuppy

Comment 11

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/fd12e8fa9699
https://hg.mozilla.org/mozilla-central/rev/1d3d39aae183
https://hg.mozilla.org/mozilla-central/rev/9bddaf4551d6