Closed
Bug 1067576
Opened 10 years ago
Closed 10 years ago
[e10s] Console.jsm doesn't work from child process on Windows
Categories
(DevTools :: General, defect)
Tracking
(e10sm3+)
RESOLVED
FIXED
Firefox 35
Tracking | Status | |
---|---|---|
e10s | m3+ | --- |
People
(Reporter: zombie, Assigned: billm)
References
Details
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
mossop
:
review+
|
Details | Diff | Splinter Review |
+++ This bug was initially created as a clone of Bug #1060093 +++
Ally says the debugger and console output work for JS running in the e10s content process when testing with the "File > New e10s Window", but not with the "browser.tabs.remote.autostart" pref enabled. The "New e10s Window" is a temporary testing feature; autostart enables the e10s code we plan to ship, including extra add-on compatibility shims that apparently interfere with the devtools.
Ally recommends that we fix the debugger and console for our e10s "M2" milestone so add-on developers have a fighting chance of fixing their add-ons' e10s problems.
Reporter | ||
Comment 1•10 years ago
|
||
bug 1060093 was scoped down to just debugging JS in the child process, so this one is about the Console.jsm not working from the child process on windows:
> hey Mike, i tested console in the child process on windows, using
> scratchpad, and it doesn't seem to show up, either in the browser console
> nor the browser toolbox console:
>
> let mm = gBrowser.addTab('data:text/html,dumb').linkedBrowser.messageManager;
>
> mm.addMessageListener(7, function() {
> console.log('truc');
> })
>
> mm.loadFrameScript('data:,new ' + function() {
> let CA = Components.utils.import("resource://gre/modules/devtools/Console.jsm", {});
> let console = new CA.ConsoleAPI();
> console.log('blah');
> sendAsyncMessage(7);
> }, true);
>
> according to my discussion with Panos and your testing (on OSX), i
> understand this should work, so maybe it's a windows-only bug?
and note that browser console is totally busted in current Nightly (unrelated bug), and the last one where this could be reproduced is from 9/13.
Reporter | ||
Updated•10 years ago
|
tracking-e10s:
--- → ?
Reporter | ||
Updated•10 years ago
|
No longer depends on: 1060093
OS: All → Windows 7
Priority: P3 → --
Summary: [e10s] console doesn't work on windows in child process → [e10s] Console.jsm doesn't work from child process on Windows
Assignee | ||
Comment 2•10 years ago
|
||
This patch is a hack. It watches for console-api-log-message notifications in the child and forwards them to the parent. However, if the arguments to console.log are objects, it needs to replace them with the string "<unavailable>" since we can't send objects using the message manager.
The right fix for this is to run a webconsole actor in the child process. However, it seems like that would have many of the same complications as bug 1060093. Right now I'd just like to get something working.
If not being able to send objects is a big problem, then we could use CPOWs. However, I'd really like to keep those out of devtools.
Panos, does this seem okay to you? It would really be nice if add-on authors could use console.log.
Attachment #8489755 -
Flags: feedback?(past)
Comment 3•10 years ago
|
||
Comment on attachment 8489755 [details] [diff] [review]
console-log
Review of attachment 8489755 [details] [diff] [review]:
-----------------------------------------------------------------
I'm guessing this isn't the right patch?
Attachment #8489755 -
Flags: feedback?(past)
(In reply to Bill McCloskey (:billm) from comment #2)
> If not being able to send objects is a big problem, then we could use CPOWs.
> However, I'd really like to keep those out of devtools.
Logging objects to the console is quite useful to trace behavior or to inspect them in the object inspector thingy, just because something runs in a frame script doesn't change that.
Maybe sending it in a pre-serialized form to display it in the console output + a unique ID that can be used to fetch a CPOW on demand if the user clicks on it to inspect the object. That could avoid performance penalties while still maintain current functionality.
Assignee | ||
Comment 5•10 years ago
|
||
Sorry. Here is the patch.
Attachment #8489755 -
Attachment is obsolete: true
Attachment #8490037 -
Flags: feedback?(past)
Updated•10 years ago
|
Comment 6•10 years ago
|
||
Comment on attachment 8490037 [details] [diff] [review]
console-log v2
Review of attachment 8490037 [details] [diff] [review]:
-----------------------------------------------------------------
So if I got it right, console.log works from content but not from chrome code in child processes, because the console actor in the child process drops the console-api-log-event in the chrome case (no innerWindowID), even though it receives it for both cases. Is that correct?
I don't think the approach in this patch is too bad, as far as hacks go. It will certainly provide add-on authors with basic debugging capabilities.
Attachment #8490037 -
Flags: feedback?(past) → feedback+
Updated•10 years ago
|
Assignee: nobody → wmccloskey
Assignee | ||
Comment 7•10 years ago
|
||
Comment on attachment 8490037 [details] [diff] [review]
console-log v2
Yes, that's correct. I guess I'll switch this to review then.
Attachment #8490037 -
Flags: feedback+ → review?(past)
Comment 8•10 years ago
|
||
Comment on attachment 8490037 [details] [diff] [review]
console-log v2
Review of attachment 8490037 [details] [diff] [review]:
-----------------------------------------------------------------
Can we add a test to make sure we don't accidentally regress this?
Attachment #8490037 -
Flags: review?(past) → review+
Comment 9•10 years ago
|
||
Oh, and I'm not a toolkit/ peer (apart from toolkit/devtools), so you might want to get a formal approval from someone who is.
Comment 10•10 years ago
|
||
Comment on attachment 8490037 [details] [diff] [review]
console-log v2
Review of attachment 8490037 [details] [diff] [review]:
-----------------------------------------------------------------
I'm a toolkit peer and this doesn't look right to me. The browser-child framescript is going to get run for every tab in the child process, each one will receive and relay every console message. So you'll get a duplicate console message logged for every open tab.
You need some singleton in the child process to do the relaying instead, maybe add the listener in to BrowserUtils.jsm?
Attachment #8490037 -
Flags: review+ → review-
Assignee | ||
Comment 11•10 years ago
|
||
This patch adds a new service that runs in the parent process and another for the child process. It builds on top of bug 930243. This probably seems like overkill for this bug, but I think these things will be useful for other facilities.
I also moved the loading of browser-content.js earlier so that all content scripts can use console.*.
Attachment #8490037 -
Attachment is obsolete: true
Attachment #8496309 -
Flags: review?(dtownsend+bugmail)
Comment 12•10 years ago
|
||
Comment on attachment 8496309 [details] [diff] [review]
console-log v3
Review of attachment 8496309 [details] [diff] [review]:
-----------------------------------------------------------------
Looks good but I'm not sure the name "global" is right. Global is already used to mean other things in our platform and this doesn't fit in so I think something else would make more sense. How about ProcessSingleton?
Attachment #8496309 -
Flags: review?(dtownsend+bugmail) → review+
Assignee | ||
Comment 13•10 years ago
|
||
Comment 14•10 years ago
|
||
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 35
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•