896534 - Tests should set the protocol logging pref before loading the debugger server

Status: RESOLVED FIXED

(DevTools :: Debugger, defect, P2)

Description

[Panos Astithas (he/him) [:past] (please ni?)]

The reason protocol logging is currently enabled in the debugger front-end but disabled in the back-end is that it is set after the debugger server is loaded. The server tries to make the check for logging as fast as possible by checking the pref when the module is loaded and closing over the constant afterwards. If we want the setting to have any effect on the server, we need to move it before loading the server module.

Comment 1

[Panos Astithas (he/him) [:past] (please ni?)]

Attachment: logging.patch

This patch will make things more consistent by disabling logging for both frontend and backend as discussed. We'll have to change the value to false when we push to try to investigate oranges.

Comment 2

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 779238 [details] [diff] [review]
logging.patch

Review of attachment 779238 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/devtools/debugger/test/head.js
@@ +24,5 @@
>  let DebuggerTransport = tempScope.DebuggerTransport;
>  let DebuggerClient = tempScope.DebuggerClient;
>  let gDevTools = tempScope.gDevTools;
>  let devtools = tempScope.devtools;
>  let TargetFactory = devtools.TargetFactory;

I feel like this stuff would look better using some destructuring assignments now. tempScope is gross.

up to you if you want to change it in this patch or not.

Comment 3

[Panos Astithas (he/him) [:past] (please ni?)]

(In reply to Rob Campbell [:rc] (:robcee) from comment #2)
> I feel like this stuff would look better using some destructuring
> assignments now. tempScope is gross.
> 
> up to you if you want to change it in this patch or not.

I think I'll do it in another patch. I've been using this along with various tentative intermittent fixes for bug 895426 and I'm wary of invalidating my try runs/results.

Comment 4

[Victor Porof [:vporof][:vp]]

(In reply to Rob Campbell [:rc] (:robcee) from comment #2)
(In reply to Panos Astithas [:past] from comment #3)
> 
> I feel like this stuff would look better using some destructuring
> assignments now. tempScope is gross.
> 
> up to you if you want to change it in this patch or not.

I'm fixing that in bug 876277. Unfortunately for him, I'm going to ask Panos for review :)

Comment 5

[Panos Astithas (he/him) [:past] (please ni?)]

https://hg.mozilla.org/integration/fx-team/rev/c048915f1a4b

(In reply to Victor Porof [:vp] from comment #4)
> I'm fixing that in bug 876277. Unfortunately for him, I'm going to ask Panos
> for review :)

Rats!

Comment 6

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/c048915f1a4b