Closed Bug 1642676 Opened 4 months ago Closed 3 months ago

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

Categories

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

enhancement

Tracking

()

RESOLVED FIXED
mozilla80
Iteration:
80.1 - June 29 - July 12
Tracking Status
firefox80 --- fixed

People

(Reporter: rpl, Assigned: rpl)

References

Details

Attachments

(3 files)

+++ 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
Assignee: nobody → lgreco
Status: NEW → ASSIGNED
Keywords: leave-open

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).

Flags: needinfo?(bugmail)
Attachment #9153462 - Attachment description: Bug 1642676 - Ensure remote workers are launched in a child process based on the expected remoteType. → Bug 1642676 - Ensure remote workers are launched in a child process based on the expected remoteType. r=asuth!
Attachment #9153463 - Attachment description: Bug 1642676 - Add test case to ensure we have test coverage for RemoteWorkerManager LaunchNewContentProcess and RegisterActor methods. → Bug 1642676 - Add test case to ensure we have test coverage for RemoteWorkerManager LaunchNewContentProcess and RegisterActor methods. r=asuth!
Summary: Select an explict remoteType for remote workers and then check that it matches the one set on the child process → Select an explicit remoteType for remote workers and then check that it matches the one set on the child process
Attachment #9158305 - Attachment description: Bug 1642676 - Move test case for SharedWorker disallowed on in-process-webextensions mode into a separate test-file. r?mixedpuppy! → Bug 1642676 - Move test case for SharedWorker disallowed on in-process-webextensions mode into a separate test file. r?mixedpuppy!
Attachment #9158305 - Attachment description: Bug 1642676 - Move test case for SharedWorker disallowed on in-process-webextensions mode into a separate test file. r?mixedpuppy! → Bug 1642676 - Move test case for SharedWorker disallowed on in-process-webextensions mode into a separate test-file. r?mixedpuppy!

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

Flags: needinfo?(bugmail)
Pushed by luca.greco@alcacoop.it:
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
Backout by csabou@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/65edebab7a93
Backed out 3 changesets fro sharedworker related failures. CLOSED TREE

(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

Iteration: --- → 80.1 - June 29 - July 12
Flags: needinfo?(lgreco)
Attachment #9158305 - Attachment description: Bug 1642676 - Move test case for SharedWorker disallowed on in-process-webextensions mode into a separate test-file. r?mixedpuppy! → Bug 1642676 - Move test case for SharedWorker disallowed on in-process-webextensions mode into a separate test file. r?mixedpuppy!
Blocks: 1648792
Blocks: 1568597
Pushed by luca.greco@alcacoop.it:
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
Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla80
Regressions: 1649391
Regressions: 1649390
You need to log in before you can comment on or make changes to this bug.