Closed Bug 1726813 Opened 3 years ago Closed 3 years ago

Ensure AdvanceShutdownPhase is called for relevant phases in xpcshell tests

Categories

(Core :: XPCOM, task)

task

Tracking

()

RESOLVED FIXED
94 Branch
Tracking Status
firefox-esr91 --- fixed
firefox94 --- fixed

People

(Reporter: jstutte, Assigned: jstutte)

References

Details

Attachments

(1 file, 1 obsolete file)

In particular it seems, that we do never reach ShutdownPhase::AppShutdownQM during xpcshell tests.

That makes it impossible to use a check like

if (AppShutdown::GetCurrentShutdownPhase() < ShutdownPhase::AppShutdownQM) { 
    NS_WARNING( "Unexpected attempt to shut down QuotaManager before " 
        "ShutdownPhase::AppShutdownQM."); 
    return;
  } 

to prevent premature calls of shutdown functions.

See Also: → 1726714

I assume, this is the point where we need to do something like here.

Blocks: 1697972
Attachment #9239150 - Attachment is obsolete: true

I just wanted to get rid of that strange changed binary file, so I submitted the patch again.

See Also: → 1651318
Assignee: nobody → jstutte
Attachment #9239162 - Attachment description: WIP: Bug 1726813: Ensure complete shutdown notification sequence in xpcshell. → Bug 1726813: Ensure complete shutdown notification sequence in xpcshell. r?#xpcom-reviewers
Status: NEW → ASSIGNED
Attachment #9239162 - Attachment description: Bug 1726813: Ensure complete shutdown notification sequence in xpcshell. r?#xpcom-reviewers → Bug 1726813: Ensure AppShutdown remains in sync with shutdown notifications in xpcshell. r?#xpcom-reviewers
Attachment #9239162 - Attachment description: Bug 1726813: Ensure AppShutdown remains in sync with shutdown notifications in xpcshell. r?#xpcom-reviewers → Bug 1726813: Ensure AppShutdown remains in sync with shutdown notifications. r?#xpcom-reviewers
Attachment #9239162 - Attachment description: Bug 1726813: Ensure AppShutdown remains in sync with shutdown notifications. r?#xpcom-reviewers → Bug 1726813: Ensure AppShutdown remains in sync with shutdown notifications in the parent process. r?#xpcom-reviewers
Pushed by jstutte@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b92a59571194
Ensure AppShutdown remains in sync with shutdown notifications in the parent process. r=xpcom-reviewers,nika,dom-worker-reviewers,asuth

(In reply to Jens Stutte [:jstutte] from comment #4)

I just wanted to get rid of that strange changed binary file, so I submitted the patch again.

Just for the records: It seems that running mach xpcshell-test locally modifies a file that is under version control. With hg revert -C memory/replace/dmd/test/script-sort-by.json.gz I could repair the situation. FWIW, I filed bug 1730840.

Blocks: 1716533
Blocks: 1716530
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 94 Branch

Comment on attachment 9239162 [details]
Bug 1726813: Ensure AppShutdown remains in sync with shutdown notifications in the parent process. r?#xpcom-reviewers

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: As suggested by :asuth this would help to moot some intermittents that otherwise will keep creating noise for the entire lifecycle of ESR91.
  • User impact if declined: It might as well help to reduce crashes caused by the late instantiation of ServiceWorkerManager.
  • Fix Landed on Version: 94
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): While this patch touches quite some files, it does not change much logic, most is just changing existing call sites in tests to ensure they call through mozilla::AppShutdown::AdvanceShutdownPhase. The only user-impacting logic change is the one in ServiceWorkerManager::GetInstance (which is important), but this change makes xpcshell tests break if we do not do all the rest, so it cannot be split meaningful.
    This could make trigger an existing intermittent to happen more often, but we can uplift the trivial patch from bug 1678896 together to avoid this.
  • String or UUID changes made by this patch: None
Attachment #9239162 - Flags: approval-mozilla-esr91?

Comment on attachment 9239162 [details]
Bug 1726813: Ensure AppShutdown remains in sync with shutdown notifications in the parent process. r?#xpcom-reviewers

Approved for 91.3esr.

Attachment #9239162 - Flags: approval-mozilla-esr91? → approval-mozilla-esr91+
See Also: → 1791350
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: