No console output for logger (Log.jsm) when used in content framescript with remoteness turned on

RESOLVED FIXED in Firefox 54

Status

defect
P2
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: whimboo, Assigned: whimboo)

Tracking

Version 3
mozilla55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox54 fixed, firefox55 fixed)

Details

Attachments

(1 attachment)

While working on bug 1335778 I noticed that using `logger.debug()` does not always print the output the console or the log file. It sometimes work but then it doesn't.
Priority: -- → P2
I confirmed from running test_accessibility.py (I was working on something related to it, and it specifically isn’t relevant here) that dump() works in isElementDisplayed in listener.js, but not logger.debug().
So this is actually only happening when the remoteness flag is set. In such cases any call to methods like logger.debug() will result that the output ends in the nirvana. 

Mike, do you know of an issue with log.jsm and remoteness tabs? If not, is there someone else I could ask?
Flags: needinfo?(mconley)
Summary: Using logger from content process does not always print output → No console output for logger (Log.jsm) when used in content framescript with remoteness turned on
So this is somehow related to the DumpAppender Marionette is using. In marionette.js we setup the logger and append one instance of DumpAppender. This works fine for modules running in the chrome process. But when the same logger is used from the framescript (listener.js) the appender doesn't seem to work in case of a remoteness tab. 

When I add a new DumpAppender in listener.js I can see the output even with remoteness turned on. So maybe we need another appender, or better create an own logger for the framescript.
Flags: needinfo?(mconley)
Assignee: nobody → hskupin
Status: NEW → ASSIGNED
Comment on attachment 8862356 [details]
Bug 1357407 - Enable logger output for framescript when remoteness is enabled.

https://reviewboard.mozilla.org/r/134286/#review137280

::: commit-message-0f5ba:3
(Diff revision 2)
> +With remoteness enabled content framescripts don't seem to inherit the
> +appenders for loggers, which have been set by the main script. To get

This sounds like a bug.  Should we file a bug on Log.jsm?

::: testing/marionette/listener.js:95
(Diff revision 2)
>  var logger = Log.repository.getLogger("Marionette");
> +// Append only once to avoid duplicated output after listener.js gets reloaded
> +if (logger.ownAppenders.length == 0) {
> +  logger.addAppender(new Log.DumpAppender());
> +}

Do we need to do this once in every component listener.js imports, or just here?

(Feel free to drop if we don’t.)
Attachment #8862356 - Flags: review?(ato) → review+
Comment on attachment 8862356 [details]
Bug 1357407 - Enable logger output for framescript when remoteness is enabled.

https://reviewboard.mozilla.org/r/134286/#review137280

> This sounds like a bug.  Should we file a bug on Log.jsm?

Maybe there is something else which we do wrong? But I don't see what it should be.

Personally I would like to get feedback first, before filing a bug. Let me put a ni? for someone here on that bug.

> Do we need to do this once in every component listener.js imports, or just here?
> 
> (Feel free to drop if we don’t.)

Given that all are using the same logger, it's enough when the listener.js adds it.
Pushed by hskupin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e1f7afd29fcd
Enable logger output for framescript when remoteness is enabled. r=ato
(In reply to Henrik Skupin (:whimboo) from comment #7)
> https://reviewboard.mozilla.org/r/134286/#review137280
> 
> > This sounds like a bug.  Should we file a bug on Log.jsm?
> 
> Maybe there is something else which we do wrong? But I don't see what it
> should be.
> 
> Personally I would like to get feedback first, before filing a bug. Let me
> put a ni? for someone here on that bug.

Mike, we might still need your input, or at least you could refer to someone who knows the logger code? Thanks.
Flags: needinfo?(mconley)
https://hg.mozilla.org/mozilla-central/rev/e1f7afd29fcd
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
(In reply to Henrik Skupin (:whimboo) from comment #9)
> 
> Mike, we might still need your input, or at least you could refer to someone
> who knows the logger code? Thanks.

The only thing I can think of is that sandboxing might affect logging here.
Flags: needinfo?(mconley)
Small test-only patch which improves logging for the Marionette tests. Lets get it uplifted to mozilla-beta. Thanks
Whiteboard: [checkin-needed-beta]
(In reply to Mike Conley (:mconley) from comment #11)
> The only thing I can think of is that sandboxing might affect logging here.

Thank you Mike. It might indeed be a reason. But for now I don't think it warrants more investigation.
You need to log in before you can comment on or make changes to this bug.