Bugzilla

Thanks for taking this on!

(In reply to Alexandre Poirot [:ochameau] from comment #25)
> The WorkerDebugger.initialize call is no longer blocked and the test now completes...
> ..but I get something totaly unrelated to throw when running the test:
> ```
> $ ./mach mochitest --headless dom/workers/test/browser_WorkerDebugger_waiting.initialize.js
> ...
> FAIL test left unexpected repeating timer dom::PeriodicGCTimerCallback (duration: 1000ms)
> ```

I believe that the [Tester.getNewRepeatingTimers](https://searchfox.org/mozilla-central/rev/a9cb8b07d4cf4290e693bc31ba23412d0f042cb8/testing/mochitest/browser-test.js#616) call is complaining about the worker's GC timer because it doesn't look like any action is being taken to terminate the worker (until the global that the `Worker` instance is torn down).

Your test probably wants to invoke `worker.terminate()` as part of the test and it could be reasonable to wait for the test to also use something like [waitForUnregister](https://searchfox.org/mozilla-central/rev/a9cb8b07d4cf4290e693bc31ba23412d0f042cb8/dom/workers/test/dom_worker_helper.js#70-83) to help ensure the worker actually went away.
 
> From comment 21, I ignored two things:
> * the WorkerDebuggerEventTarget, I don't quite follow this point.

This is specific to an enhancement (https://bugzilla.mozilla.org/show_bug.cgi?id=1672493#c1) that has not happened yet.

> * the filtering by global, to use `GetCurrentRealmOrNull` to only drain when we aren't in the debugger global.

I'm not 100% on my exact thought, but I think my idea was to compare against the `GetGlobalJSObject` (like you've done) from [WorkerPrivate::GlobalScope](https://searchfox.org/mozilla-central/rev/a9cb8b07d4cf4290e693bc31ba23412d0f042cb8/dom/workers/WorkerPrivate.h#464) instead of WorkerPrivate::DebuggerGlobalScope since there should only ever be one worker global whereas there currently can be a bunch of debugger globals.  So we infer we're in the debugger if we're not in content.

Thanks for taking this on!

(In reply to Alexandre Poirot [:ochameau] from comment #25)
> The WorkerDebugger.initialize call is no longer blocked and the test now completes...
> ..but I get something totaly unrelated to throw when running the test:
> ```
> $ ./mach mochitest --headless dom/workers/test/browser_WorkerDebugger_waiting.initialize.js
> ...
> FAIL test left unexpected repeating timer dom::PeriodicGCTimerCallback (duration: 1000ms)
> ```

I believe that the [Tester.getNewRepeatingTimers](https://searchfox.org/mozilla-central/rev/a9cb8b07d4cf4290e693bc31ba23412d0f042cb8/testing/mochitest/browser-test.js#616) call is complaining about the worker's GC timer because it doesn't look like any action is being taken to terminate the worker (until the global that the `Worker` instance is torn down).

Your test probably wants to invoke `worker.terminate()` as part of the test and it could be reasonable to wait for the test to also use something like [waitForUnregister](https://searchfox.org/mozilla-central/rev/a9cb8b07d4cf4290e693bc31ba23412d0f042cb8/dom/workers/test/dom_worker_helper.js#70-83) to help ensure the worker actually went away.
 
> From comment 21, I ignored two things:
> * the WorkerDebuggerEventTarget, I don't quite follow this point.

This is specific to an enhancement (https://bugzilla.mozilla.org/show_bug.cgi?id=1672493#c1) that has not happened yet.

> * the filtering by global, to use `GetCurrentRealmOrNull` to only drain when we aren't in the debugger global.

I'm not 100% on my exact thought, but I think my idea was to compare against the `GetGlobalJSObject` (like you've done) from [WorkerPrivate::GlobalScope](https://searchfox.org/mozilla-central/rev/a9cb8b07d4cf4290e693bc31ba23412d0f042cb8/dom/workers/WorkerPrivate.h#464) instead of WorkerPrivate::DebuggerGlobalScope since there should only ever be one worker global whereas there currently can be a bunch of debugger globals.  So we infer we're in the debugger if we're not in content.

edit addition: I should note that it seems plausible that the check mechanism doesn't work like I was assuming, so it's potentially worthwhile trying to validate the idea by adding a printf and also creating a situation where you try and trigger an interrupt when the debugger is on the stack.  This could be a reasonable test to add, but does fundamentally end up requiring a busy-loop which is not a great thing to do in a test.