1392540 - DOM Promises from debugger modules are frozen while debugging

Status: RESOLVED FIXED

(Core :: DOM: Workers, enhancement, P2)

Description

[Alexandre Poirot [:ochameau]]

See bug 1387128 comment 10.
It looks like DOM Promises used by the debugger itself are froken when we are debugging a worker script.
This prevents getting rid of Promise.jsm, which itsn't frozen as Promise implementation itself is in plain JS.

Comment 1

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1392540 - Keep running debugger Promise while debugging workers.

Review commit: https://reviewboard.mozilla.org/r/171072/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/171072/

Comment 2

[Alexandre Poirot [:ochameau]]

Andrea, TBH, I didn't knew who to ask for this patch. Eddy and Kyle were the main contributor for this...
Feel free to redirect to whoever makes sense!

Comment 3

[Alexandre Poirot [:ochameau]]

Comment on attachment 8899738 [details]
Bug 1392540 - Keep running debugger Promise while debugging workers.

Ben, May be you have more knowledges on the worker implementation?

Comment 4

[Ben Kelly [:bkelly, not reviewing]]

Comment on attachment 8899738 [details]
Bug 1392540 - Keep running debugger Promise while debugging workers.

This looks reasonable to me, but I'm not super confident I understand the microtask checkpoint stuff.  Andrea, what do you think?

Comment 5

[Alexandre Poirot [:ochameau]]

Andrew, Could you help me find a reviewer for this?

Comment 6

[Andrea Marchesini [:baku]]

Comment on attachment 8899738 [details]
Bug 1392540 - Keep running debugger Promise while debugging workers.

https://reviewboard.mozilla.org/r/171072/#review181252

sorry for the delay.

Comment 7

[Andrew Overholt [:overholt]]

Are we ready to land this now, Alexandre?

Comment 8

[Alexandre Poirot [:ochameau]]

I'm a bit stressed by this patch as I have no idea what are the real consequences of it.
I would like to have a dedicated test for this, but I haven't been able to make one.
At least, not one in dom/ folder which doesn't involve the whole devtools codebase.
So I would be more confident landing this in FF58, hopefully, with a test.

Comment 9

[Andrew Overholt [:overholt]]

Waiting for 58 sounds good to me :)

Comment 10

[Alexandre Poirot [:ochameau]]

Comment on attachment 8899738 [details]
Bug 1392540 - Keep running debugger Promise while debugging workers.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/171072/diff/1-2/

Comment 11

[Alexandre Poirot [:ochameau]]

Comment on attachment 8899738 [details]
Bug 1392540 - Keep running debugger Promise while debugging workers.

https://reviewboard.mozilla.org/r/171072/#review189094

::: dom/workers/test/WorkerDebugger_promise_debugger.js:29
(Diff revision 2)
>        };
> +      // Test bug 1392540 where DOM Promises from debugger principal
> +      // where frozen while hitting a worker breakpoint.
> +      Promise.resolve().then(() => {
> -      postMessage("paused");
> +        postMessage("paused");
> +      });

Baku, could you take a quick look at the test modification I did to cover this?
I modified an existing test instead of introducing a new one just for that.

It pass with the c++ patch:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=68480a97faecbfdbf9f62a4a295a51fd288f1006
And timesout without it:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=4114c1fe691067a15c39f658c937ea1508b67e6e&selectedJob=133424752

Comment 12

[Andrea Marchesini [:baku]]

Comment on attachment 8899738 [details]
Bug 1392540 - Keep running debugger Promise while debugging workers.

https://reviewboard.mozilla.org/r/171072/#review189098

Yes, it seems correct to me.

Comment 13

[Alexandre Poirot [:ochameau]]

Thanks for the fast response!

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/469ad7100cae
Keep running debugger Promise while debugging workers. r=baku

Comment 15

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/469ad7100cae