Bug 1257977 - Fix SWR updatefound event timing. r=dom-worker-reviewers,edenchuang

This corrects a longstanding race in our updatefound logic and makes
testing/web-platform/tests/service-workers/service-worker/ServiceWorkerGlobalScope/registration-attribute.https.html
consistently pass.  (Other patches in this stack made the test no
longer permafail and removed the meta .ini, but surfaced the race.)

https://bugzilla.mozilla.org/show_bug.cgi?id=1257977#c12 provides
detailed context, but the basic idea is that bug 1510809 cleaned up
our updatefound logic but left a runnable delay introduced in
bug 1471929 that made sense for the fix there, but stopped making sense
with bug 1510809.

This fix repurposes the FireUpdateFound() method declaration that had
no actual implementing method to call into the private
MaybeDispatchUpdateFound which is part of a sufficiently confusing
internal state machine that it makes sense to leave it private and use
a more sane public name.

Differential Revision: https://phabricator.services.mozilla.com/D213726
This commit is contained in:
Andrew Sutherland 2024-10-24 03:02:41 +00:00
parent a83f090d39
commit d17ba40906
3 changed files with 10 additions and 23 deletions

View File

@ -523,13 +523,18 @@ void ServiceWorkerRegistration::WhenVersionReached(
void ServiceWorkerRegistration::MaybeScheduleUpdateFound(
const Maybe<ServiceWorkerDescriptor>& aInstallingDescriptor) {
// This function sets mScheduledUpdateFoundId to note when we were told about
// a new installing worker. We rely on a call to
// MaybeDispatchUpdateFoundRunnable (called indirectly from UpdateJobCallback)
// to actually fire the event.
// a new installing worker. We rely on a call to MaybeDispatchUpdateFound via
// ServiceWorkerRegistrationChild::RecvFireUpdateFound to trigger the properly
// timed notification...
uint64_t newId = aInstallingDescriptor.isSome()
? aInstallingDescriptor.ref().Id()
: kInvalidUpdateFoundId;
// ...but the whole reason this logic exists is because SWRegistrations are
// bootstrapped off of inherently stale descriptor snapshots and receive
// catch-up updates once the actor is created and registered in the parent.
// To handle the catch-up case where we need to generate a synthetic
// updatefound that would otherwise be lost, we immediately flush here.
if (mScheduledUpdateFoundId != kInvalidUpdateFoundId) {
if (mScheduledUpdateFoundId == newId) {
return;
@ -548,22 +553,6 @@ void ServiceWorkerRegistration::MaybeScheduleUpdateFound(
mScheduledUpdateFoundId = newId;
}
void ServiceWorkerRegistration::MaybeDispatchUpdateFoundRunnable() {
if (mScheduledUpdateFoundId == kInvalidUpdateFoundId) {
return;
}
nsIGlobalObject* global = GetParentObject();
NS_ENSURE_TRUE_VOID(global);
nsCOMPtr<nsIRunnable> r = NewCancelableRunnableMethod(
"ServiceWorkerRegistration::MaybeDispatchUpdateFound", this,
&ServiceWorkerRegistration::MaybeDispatchUpdateFound);
Unused << global->SerialEventTarget()->Dispatch(r.forget(),
NS_DISPATCH_NORMAL);
}
void ServiceWorkerRegistration::MaybeDispatchUpdateFound() {
uint64_t scheduledId = mScheduledUpdateFoundId;
mScheduledUpdateFoundId = kInvalidUpdateFoundId;

View File

@ -105,11 +105,9 @@ class ServiceWorkerRegistration final : public DOMEventTargetHelper {
void WhenVersionReached(uint64_t aVersion,
ServiceWorkerBoolCallback&& aCallback);
void MaybeDispatchUpdateFoundRunnable();
void RevokeActor(ServiceWorkerRegistrationChild* aActor);
void FireUpdateFound();
void FireUpdateFound() { MaybeDispatchUpdateFound(); }
private:
ServiceWorkerRegistration(

View File

@ -34,7 +34,7 @@ IPCResult ServiceWorkerRegistrationChild::RecvUpdateState(
IPCResult ServiceWorkerRegistrationChild::RecvFireUpdateFound() {
if (mOwner) {
mOwner->MaybeDispatchUpdateFoundRunnable();
mOwner->FireUpdateFound();
}
return IPC_OK();
}