Bug 1712574 - Avoid trying to register the legacy listener twice. r=jdescottes

This code calling `watchResourcesForTarget` should only be called when
we already called `TargetCommand.watchTargets()`.
When we call `ResourceCommand.watchResources` for the first time, `TargetCommand.watchTargets`
will process already existing targets and call `ResourceCommand.onTargetAvailable` (which calls `watchResourcesForTarget`).
But for any subsequent call, we should use `TargetCommand.getAllTargets` and call `watchResourcesForTarget` manually.

Differential Revision: https://phabricator.services.mozilla.com/D116027
This commit is contained in:
Alexandre Poirot 2021-06-02 12:45:12 +00:00
Родитель fb7cb8d3fc
Коммит 4ed1c008de
1 изменённых файлов: 52 добавлений и 27 удалений

Просмотреть файл

@ -261,6 +261,9 @@ class ResourceCommand {
*/
async _watchAllTargets() {
if (!this._watchTargetsPromise) {
// If this is the very first listener registered, of all kind of resource types:
// * we want to start observing targets via TargetCommand
// * _onTargetAvailable will be called for each already existing targets and the next one to come
this._watchTargetsPromise = this.targetCommand.watchTargets(
this.targetCommand.ALL_TYPES,
this._onTargetAvailable,
@ -286,6 +289,37 @@ class ResourceCommand {
);
}
/**
* For a given resource type, start the legacy listeners for all already existing targets.
* Do that only if we have to. If this resourceType requires legacy listeners.
*/
async _startLegacyListenersForExistingTargets(resourceType) {
// If we were already listening to targets, we want to start the legacy listeners
// for all already existing targets.
const shouldRunLegacyListeners =
!this.hasResourceCommandSupport(resourceType) ||
this._shouldRunLegacyListenerEvenWithWatcherSupport(resourceType);
if (shouldRunLegacyListeners) {
const promises = [];
const targets = this.targetCommand.getAllTargets(
this.targetCommand.ALL_TYPES
);
for (const targetFront of targets) {
// We disable warning in case we already registered the legacy listener for this target
// as this code may race with the call from onTargetAvailable if we end up having multiple
// calls to _startListening in parallel.
promises.push(
this._watchResourcesForTarget({
targetFront,
resourceType,
disableWarning: true,
})
);
}
await Promise.all(promises);
}
}
/**
* Method called by the TargetCommand for each already existing or target which has just been created.
*
@ -346,7 +380,7 @@ class ResourceCommand {
// ...request existing resource and new one to come from this one target
// *but* only do that for backward compat, where we don't have the watcher API
// (See bug 1626647)
await this._watchResourcesForTarget(targetFront, resourceType);
await this._watchResourcesForTarget({ targetFront, resourceType });
}
}
@ -778,27 +812,6 @@ class ResourceCommand {
this._processingExistingResources.add(resourceType);
const shouldRunLegacyListeners =
!this.hasResourceCommandSupport(resourceType) ||
this._shouldRunLegacyListenerEvenWithWatcherSupport(resourceType);
if (shouldRunLegacyListeners) {
// If this is the very first listener registered, of all kind of resource types:
// 1) TargetCommand may not be initialized yet, so that targetCommand.getAllTargets will return an empty array
// 2) The following call to watchAllTargets will process all existing targets when it will call onTargetAvailable
//
// So this code is meant for all but the very first registered listener of all kinds.
// TargetCommand will already be watching for targets and the following call to watchAllTargets will be a no-op.
// So that we have to manually process all existing targets here.
const promises = [];
const targets = this.targetCommand.getAllTargets(
this.targetCommand.ALL_TYPES
);
for (const target of targets) {
promises.push(this._watchResourcesForTarget(target, resourceType));
}
await Promise.all(promises);
}
// Ensuring enabling listening to targets.
// This will be a no-op expect for the very first call to `_startListening`,
// where it is going to call `onTargetAvailable` for all already existing targets,
@ -806,7 +819,13 @@ class ResourceCommand {
//
// Do this *before* calling WatcherActor.watchResources in order to register "resource-available"
// listeners on targets before these events start being emitted.
await this._watchAllTargets();
await this._watchAllTargets(resourceType);
// When we are calling _startListening for the first time, _watchAllTargets
// will register legacylistener when it will call onTargetAvailable for all existing targets.
// But for any next calls to _startListening, _watchAllTargets will be a no-op,
// and nothing will start legacy listener for each already registered targets.
await this._startLegacyListenersForExistingTargets(resourceType);
// If the server supports the Watcher API and the Watcher supports
// this resource type, use this API
@ -850,7 +869,11 @@ class ResourceCommand {
* Call backward compatibility code from `LegacyListeners` in order to listen for a given
* type of resource from a given target.
*/
async _watchResourcesForTarget(targetFront, resourceType) {
async _watchResourcesForTarget({
targetFront,
resourceType,
disableWarning = false,
}) {
if (this._hasResourceCommandSupportForTarget(resourceType, targetFront)) {
// This resource / target pair should already be handled by the watcher,
// no need to start legacy listeners.
@ -872,9 +895,11 @@ class ResourceCommand {
const legacyListeners =
this._existingLegacyListeners.get(targetFront) || [];
if (legacyListeners.includes(resourceType)) {
console.warn(
`Already started legacy listener for ${resourceType} on ${targetFront.actorID}`
);
if (!disableWarning) {
console.warn(
`Already started legacy listener for ${resourceType} on ${targetFront.actorID}`
);
}
return;
}
this._existingLegacyListeners.set(