Closed Bug 1668376 Opened 4 years ago Closed 4 years ago

Test failure in dom/base/test/test_window_close.html when e10s-multi is enabled

Categories

(GeckoView :: Sandboxing, defect, P1)

Unspecified
All

Tracking

(firefox83 fixed)

RESOLVED FIXED
83 Branch
Tracking Status
firefox83 --- fixed

People

(Reporter: bugzilla, Assigned: bugzilla)

References

Details

(Whiteboard: [geckoview:m83])

Attachments

(2 files)

The symptom was that the BroadcastChannel message being sent to the parent was being lost due to content process termination.

I had a chat with Nika and learned that ContentParent::MarkAsDead only marks the content process as no longer eligible for new content being allocated to it, but the web content itself may still be active (and doing things like flushing BroadcastChannel messages).

OTOH, our GV implementation assumes that MarkAsDead means that the content process is actually dead, so it goes off and unbinds at a moment that, as it turns out, is premature.

Commenting out the call to the Java MarkAsDead "fixes" the test failure. The purpose of this bug is to update GV's MarkAsDead with my corrected understanding of what its semantics should be.

Since the semantics of ContentParent::MarkAsDead are significantly different
from GeckoProcessManager::MarkAsDead, let's rename the latter to better
reflect what it actually does.

Because GeckoView content processes are hosted inside Android Services, and because
there is a hard limit on those services, we cannot assume that it is safe to launch
a new content process while another one is still in the process of shutting down.

We add a GeckoView-specific counter that tracks how many ContentParent objects
still exist but are unavailable for receiving new content. We increment that
counter the first time that MarkAsDead is called on a particular ContentParent,
and decrement that counter when the ContentParent is finally being closed.

During used process allocation we charge that count against the maximum number of
available processes so that the selection algorithm does not attempt to start
a new process when none are actually available.

We also move the code that tells the Java side of Android that we're shutting
down to a more reasonable location.

Depends on D92649

Attachment #9179958 - Attachment description: Bug 1668376: Part 2 - Compensate for unavailable ContentParent objects when selecting a used content process in GeckoView; r=nika,kmag → Bug 1668376: Part 2 - On Android, do not mark content processes as dead in ContentParent::NotifyTabDestroying; r=nika
Pushed by aklotz@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/817f3d254144
Part 1 - Rename GeckoProcessManager::MarkAsDead to GeckoProcessManager::ShutdownProcess; r=geckoview-reviewers,necko-reviewers,agi,dragana
https://hg.mozilla.org/integration/autoland/rev/d7b7b9133f0b
Part 2 - On Android, do not mark content processes as dead in ContentParent::NotifyTabDestroying; r=nika
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 83 Branch

Moving some e10s bugs to the new GeckoView::Sandboxing component.

Component: General → Sandboxing
See Also: → 1847608
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: