diff --git a/devtools/client/framework/test/browser_target_parents.js b/devtools/client/framework/test/browser_target_parents.js index 4ac989ed562e..7c564024a4ce 100644 --- a/devtools/client/framework/test/browser_target_parents.js +++ b/devtools/client/framework/test/browser_target_parents.js @@ -42,15 +42,41 @@ add_task(async function() { const { processes } = await mainRoot.listProcesses(); + // Assert that concurrent calls to getTarget resolves the same target and that it is already attached + // We that, we were chasing a precise race, where a second call to ProcessDescriptor.getTarget() + // happens between the instantiation of ContentProcessTarget and its call to attach() from getTarget + // function. await Promise.all( processes.map(async processDescriptor => { - const process = await processDescriptor.getTarget(); - - is( - process.descriptorFront, - processDescriptor, - "Got the correct descriptorFront from the process target." - ); + const promises = []; + const concurrentCalls = 10; + for (let i = 0; i < concurrentCalls; i++) { + const targetPromise = processDescriptor.getTarget(); + // Every odd runs, wait for a tick to introduce some more randomness + if (i % 2 == 0) { + await wait(0); + } + promises.push( + targetPromise.then(target => { + is( + target.descriptorFront, + processDescriptor, + "Got the correct descriptorFront from the process target." + ); + // activeConsole is one of the only attribute to assess that a Content Process Target is attached + ok(target.activeConsole, "The target is attached"); + return target; + }) + ); + } + const targets = await Promise.all(promises); + for (let i = 1; i < concurrentCalls; i++) { + is( + targets[0], + targets[i], + "All the targets returned by concurrent calls to getTarget are the same" + ); + } }) ); @@ -73,15 +99,39 @@ add_task(async function() { const mainProcess = await mainRoot.getMainProcess(); const { frames } = await mainProcess.listRemoteFrames(); - await Promise.all( - frames.map(async frameDescriptorFront => { - const frameTargetFront = await frameDescriptorFront.getTarget(); - is( - frameTargetFront.descriptorFront, - frameDescriptorFront, - "Got the correct descriptorFront from the frame target." - ); + // Assert that concurrent calls to getTarget resolves the same target and that it is already attached + await Promise.all( + frames.map(async frameDescriptor => { + const promises = []; + const concurrentCalls = 10; + for (let i = 0; i < concurrentCalls; i++) { + const targetPromise = frameDescriptor.getTarget(); + // Every odd runs, wait for a tick to introduce some more randomness + if (i % 2 == 0) { + await wait(0); + } + promises.push( + targetPromise.then(target => { + is( + target.descriptorFront, + frameDescriptor, + "Got the correct descriptorFront from the frame target." + ); + // traits is one attribute to assert that a Frame Target is attached + ok(target.traits, "The target is attached"); + return target; + }) + ); + } + const targets = await Promise.all(promises); + for (let i = 1; i < concurrentCalls; i++) { + is( + targets[0], + targets[i], + "All the targets returned by concurrent calls to getTarget are the same" + ); + } }) ); diff --git a/devtools/shared/fronts/descriptors/frame.js b/devtools/shared/fronts/descriptors/frame.js index 13b65495bfbc..5d2455f8cfca 100644 --- a/devtools/shared/fronts/descriptors/frame.js +++ b/devtools/shared/fronts/descriptors/frame.js @@ -43,33 +43,42 @@ class FrameDescriptorFront extends FrontClassWithSpec(frameDescriptorSpec) { } async getTarget() { + // Only return the cached Target if it is still alive. + // It may have been destroyed in case of navigation trigerring a load in another + // process. if (this._frameTargetFront && this._frameTargetFront.actorID) { return this._frameTargetFront; } + // Otherwise, ensure that we don't try to spawn more than one Target by + // returning the pending promise if (this._targetFrontPromise) { return this._targetFrontPromise; } this._targetFrontPromise = (async () => { + let target = null; try { const targetForm = await super.getTarget(); + // getTarget uses 'json' in the specification type and this prevents converting + // actor exception into front exceptions if (targetForm.error) { - this._targetFrontPromise = null; - return null; + throw new Error(targetForm.error); } - this._frameTargetFront = await this._createFrameTarget(targetForm); - await this._frameTargetFront.attach(); - // clear the promise if we are finished so that we can re-connect if - // necessary - this._targetFrontPromise = null; - return this._frameTargetFront; + target = await this._createFrameTarget(targetForm); + await target.attach(); } catch (e) { // This is likely to happen if we get a lot of events which drop previous // frames. console.log( `Request to connect to frameDescriptor "${this.id}" failed: ${e}` ); - return null; } + // Save the reference to the target only after the call to attach + // so that getTarget always returns the attached target in case of concurrent calls + this._frameTargetFront = target; + // clear the promise if we are finished so that we can re-connect if + // necessary + this._targetFrontPromise = null; + return target; })(); return this._targetFrontPromise; } diff --git a/devtools/shared/fronts/descriptors/process.js b/devtools/shared/fronts/descriptors/process.js index 48f7358dd661..cd0b05fd1d38 100644 --- a/devtools/shared/fronts/descriptors/process.js +++ b/devtools/shared/fronts/descriptors/process.js @@ -55,31 +55,35 @@ class ProcessDescriptorFront extends FrontClassWithSpec(processDescriptorSpec) { } async getTarget() { + // Only return the cached Target if it is still alive. if (this._processTargetFront && this._processTargetFront.actorID) { return this._processTargetFront; } + // Otherwise, ensure that we don't try to spawn more than one Target by + // returning the pending promise if (this._targetFrontPromise) { return this._targetFrontPromise; } this._targetFrontPromise = (async () => { + let targetFront = null; try { const targetForm = await super.getTarget(); - this._processTargetFront = await this._createProcessTargetFront( - targetForm - ); - await this._processTargetFront.attach(); - // clear the promise if we are finished so that we can re-connect if - // necessary - this._targetFrontPromise = null; - return this._processTargetFront; + targetFront = await this._createProcessTargetFront(targetForm); + await targetFront.attach(); } catch (e) { // This is likely to happen if we get a lot of events which drop previous // processes. console.log( `Request to connect to ProcessDescriptor "${this.id}" failed: ${e}` ); - return null; } + // Save the reference to the target only after the call to attach + // so that getTarget always returns the attached target in case of concurrent calls + this._processTargetFront = targetFront; + // clear the promise if we are finished so that we can re-connect if + // necessary + this._targetFrontPromise = null; + return targetFront; })(); return this._targetFrontPromise; }