762240 - Make console.{log, etc.} work from content processes

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

I thought we had remoted this as part of the xul-fennec work, but console.log() doesn't work in current b2g.

Comment 1

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: Gaia test

Comment 2

[Josh Matthews [:jdm]]

We only remoted the console service, not the web console object.

Comment 3

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

I was under the impression that the web console used the console service, but I'm not 100% sure how this works in b2g.

Comment 4

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

So shell.js implements console.* in b2g.  It's not loaded as a frame script.

I moved the console.* impl from shell.js into webapi.js, which is supposed to be loaded as a frame script, but it doesn't work either.

If the problem is that the webapi.js frame script isn't being in out-of-process frames, then that would explain this bug and bug 761924.

Comment 5

[Justin Lebar (not reading bugmail)]

> If the problem is that the webapi.js frame script isn't being in out-of-process frames

Ah, maybe we need something akin to http://hg.mozilla.org/mozilla-central/file/c779db92fef2/dom/base/BrowserElementParent.js#l119

Comment 6

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

shell.js already tries to load webapi.js as a frame script.

I think the problem is that the out-of-process iframes are loaded by the *system app*, which is a different nsIFrameLoader.  So we probably need webapi.js to propagate itself.

That won't fix this bug since we must only have one webconsole listener per process, but it could fix bug 761924.

Comment 7

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Thanks jlebar!

Yes, it turns out that the way we're loading webapi.js is totally wrong.  I'll see how much I can fix before my hack time runs out.

Comment 8

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: Install a console.* shim for all browser frames, and ensure we load webapi.js properly

Comment 10

[Mounir Lamouri (:mounir)]

Hmm, how bug 761924 is a duplicate of that one?

Comment 11

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

They're manifestations of the same underlying problem so I fixed them together.

Comment 12

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Comment on attachment 630853 [details] [diff] [review]
Install a console.* shim for all browser frames, and ensure we load webapi.js properly

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

::: b2g/chrome/content/shell.js
@@ -349,5 @@
> -               " at " + message.filename + ":" + message.lineNumber +
> -               " in " + (message.functionName || "anonymous") + ": ";
> -  Services.console.logStringMessage(prefix + Array.join(message.arguments, " "));
> -}, "console-api-log-event", false);
> -

Now that bug 673148 has landed we should think about building a remote web console (i have a broken proto on my disk that I need to revived) but I guess this code is fine for now.

nit: the rest of the file use ' instead of "

::: b2g/components/ProcessGlobal.js
@@ +62,5 @@
> +      let mm = frameLoader.messageManager;
> +      try {
> +        mm.loadFrameScript(kWebApiShimFile, true);
> +      } catch (e) {
> +        log("Error loading "+ kWebApiShimFile +" as frame script: "+ e +"\n");

nit: it miss some spaces before and after some +

Comment 13

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

(In reply to Vivien Nicolas (:vingtetun) from comment #12)
> Comment on attachment 630853 [details] [diff] [review]
> Install a console.* shim for all browser frames, and ensure we load
> webapi.js properly
> 
> > +        log("Error loading "+ kWebApiShimFile +" as frame script: "+ e +"\n");
> 
> nit: it miss some spaces before and after some +

Ugh really? :)  That whitespace just obfuscates the interesting things.  The ' ' characters inside the strings serve to delimit the interesting lexemes.  But W.

Comment 14

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: updated patch

Comment 15

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

https://hg.mozilla.org/integration/mozilla-inbound/rev/332ed5402ccf

Comment 16

[Justin Lebar (not reading bugmail)]

Was console.log broken with in-process <iframe mozbrowser>?  This patch injects code for both in-process and OOP...

Comment 17

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

No.  But the injected code doesn't hook console.log.

Comment 18

[Graeme McCutcheon [:graememcc]]

https://hg.mozilla.org/mozilla-central/rev/332ed5402ccf

(Merged by Ed Morley)