Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2024-12-01 17:23:59 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Bug 1717005 - [devtools] Stop using unsafeSynchronize. r=jdescottes,bomsy

Browse Source 
I don't think it was any useful to block synchronously on the call to `doResume`.
In the past it used to send an event to the client synchronously,
it may have been important back then.

test_nesting-03.js is still covering nested event loop by using two clients.

Differential Revision: https://phabricator.services.mozilla.com/D118173
This commit is contained in:
Alexandre Poirot 2021-07-12 21:04:55 +00:00
parent e89956fbff
commit e260f44463
4 changed files with 13 additions and 146 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

49
devtools/server/actors/thread.js
View File

@ -1339,41 +1339,6 @@ const ThreadActor = ActorClassWithSpec(threadSpec, {
this.hideOverlay();
},
/**
* Spin up a nested event loop so we can synchronously resolve a promise.
*
* DON'T USE THIS UNLESS YOU ABSOLUTELY MUST! Nested event loops suck: the
* world's state can change out from underneath your feet because JS is no
* longer run-to-completion.
*
* @param p
* The promise we want to resolve.
* @returns The promise's resolution.
*/
unsafeSynchronize(p) {
let needNest = true;
let eventLoop;
let returnVal;
p.then(resolvedVal => {
needNest = false;
returnVal = resolvedVal;
})
.catch(e => reportException("unsafeSynchronize", e))
.then(() => {
if (eventLoop) {
eventLoop.resolve();
}
});
if (needNest) {
eventLoop = this._nestedEventLoops.push();
eventLoop.enter();
}
return returnVal;
},
/**
* Set the debugging hook to pause on exceptions if configured to do so.
*/
@ -1797,6 +1762,9 @@ const ThreadActor = ActorClassWithSpec(threadSpec, {
},
_onWindowReady({ isTopLevel, isBFCache, window }) {
// Note that this code relates to the disabling of Debugger API from will-navigate listener.
// And should only be triggered when the target actor doesn't follow WindowGlobal lifecycle.
// i.e. when the Thread Actor manages more than one top level WindowGlobal.
if (isTopLevel && this.state != STATES.DETACHED) {
this.sourcesManager.reset();
this.clearDebuggees();
@ -1826,7 +1794,16 @@ const ThreadActor = ActorClassWithSpec(threadSpec, {
// Proceed normally only if the debuggee is not paused.
if (this.state == STATES.PAUSED) {
this.unsafeSynchronize(Promise.resolve(this.doResume()));
// If we were paused while navigating to a new page,
// we resume previous page execution, so that the document can be sucessfully unloaded.
// And we disable the Debugger API, so that we do not hit any breakpoint or trigger any
// thread actor feature. We will re-enable it just before the next page starts loading,
// from window-ready listener. That's for when the target doesn't follow WindowGlobal
// lifecycle.
// When the target follows the WindowGlobal lifecycle, we will stiff resume and disable
// this thread actor. It will soon be destroyed. And a new target will pick up
// the next WindowGlobal and spawn a new Debugger API, via ThreadActor.attach().
this.doResume();
this.dbg.disable();
}

39
devtools/server/tests/xpcshell/test_nesting-01.js
View File

@ -1,39 +0,0 @@
/* Any copyright is dedicated to the Public Domain.
http://creativecommons.org/publicdomain/zero/1.0/ */
"use strict";
// Test that we can nest event loops when needed in
// ThreadActor.prototype.unsafeSynchronize.
add_task(
threadFrontTest(async ({ threadFront, client }) => {
// Reach over the protocol connection and get a reference to the thread actor.
// TODO: rewrite the test so we don't do this..
const thread = client._transport._serverConnection.getActor(
threadFront.actorID
);
test_nesting(thread);
})
);
function test_nesting(thread) {
let currentStep = 0;
const p = new Promise(resolve => {
executeSoon(function() {
// Should be on the first step
Assert.equal(++currentStep, 1);
// We should have one nested event loop from unsfeSynchronize
Assert.equal(thread._nestedEventLoops.size, 1);
resolve(true);
});
});
Assert.equal(thread.unsafeSynchronize(p), true);
// Should be on the second step
Assert.equal(++currentStep, 2);
// There shouldn't be any nested event loops anymore
Assert.equal(thread._nestedEventLoops.size, 0);
}

69
devtools/server/tests/xpcshell/test_nesting-02.js
View File

@ -1,69 +0,0 @@
/* Any copyright is dedicated to the Public Domain.
http://creativecommons.org/publicdomain/zero/1.0/ */
"use strict";
// Test that we can nest event loops and then automatically exit nested event
// loops when requested.
add_task(
threadFrontTest(async ({ threadFront, client }) => {
// Reach over the protocol connection and get a reference to the thread actor.
// TODO: rewrite the test so we don't do this..
const thread = client._transport._serverConnection.getActor(
threadFront.actorID
);
test_nesting(thread);
})
);
function test_nesting(thread) {
// The following things should happen (in order):
// 1. In the new event loop (created by unsafeSynchronize)
// 2. Resolve the promise (shouldn't exit any event loops)
// 3. Exit the event loop (should also then exit unsafeSynchronize's event loop)
// 4. Be after the unsafeSynchronize call
let currentStep = 0;
const p = new Promise(resolve => {
executeSoon(function() {
executeSoon(function() {
// Should be at step 2
Assert.equal(++currentStep, 2);
// Before resolving, should have the unsafeSynchronize event loop and the
// one just created.
Assert.equal(thread._nestedEventLoops.size, 2);
executeSoon(function() {
// Should be at step 3
Assert.equal(++currentStep, 3);
// Before exiting the manually created event loop, should have the
// unsafeSynchronize event loop and the manual event loop.
Assert.equal(thread._nestedEventLoops.size, 2);
// Should have the event loop
Assert.ok(!!eventLoop);
eventLoop.resolve();
});
resolve(true);
// Shouldn't exit any event loops because a new one started since the call
// to unsafeSynchronize
Assert.equal(thread._nestedEventLoops.size, 2);
});
// Should be at step 1
Assert.equal(++currentStep, 1);
// Should have only the unsafeSynchronize event loop
Assert.equal(thread._nestedEventLoops.size, 1);
const eventLoop = thread._nestedEventLoops.push();
eventLoop.enter();
});
});
Assert.equal(thread.unsafeSynchronize(p), true);
// Should be on the fourth step
Assert.equal(++currentStep, 4);
// There shouldn't be any nested event loops anymore
Assert.equal(thread._nestedEventLoops.size, 0);
}

2
devtools/server/tests/xpcshell/xpcshell.ini
View File

@ -41,8 +41,6 @@ support-files =
[test_addons_actor.js]
[test_animation_name.js]
[test_animation_type.js]
[test_nesting-01.js]
[test_nesting-02.js]
[test_nesting-03.js]
[test_forwardingprefix.js]
[test_getyoungestframe.js]