in bug 605241, we're trying to figure out where errors are coming from in external scripts loaded in a webpage. We have a window.onerror() handler currently, but this might be easier to obtain from the JS Engine itself. from Mihai's comment 0 in that bug: Currently we use the window.onerror event handler to work around the problem with the nsIConsoleService observer which fails to catch errors from external scripts. However, the window.onerror event handler fails to catch warnings. We need to fix this.

adding some interested CCs.

Uh, no. This isn't a duplicate. The other bug has way wider scope. I plan to fix this bug for all the things that went through window.onerror. I don't plan to fix the other at the same time (though one could build on what I do in this bug to fix the other).

Comment on attachment 484408 [details] [diff] [review] Add an nsIScriptError2 interface that exposes an outer window id, and set the window id for script errors coming through the DOM JS error reporter. This is somewhat suboptimal in the sense that I'd prefer we put an inner window id on the error, but I'm not seeing a quick way to get one in the JSEnvironment code right this second, and this is good enough for non-edge cases (errors triggered during document navigation) for the web console, I think.

The web console, as currently written, really needs a window object reference (from which it can find the containing <browser> via its docshell's chromeEventHandler, and from there find the actual console object). That's kind of a pain to get given a window ID, right? Can we make that easier somehow?

> really needs a window object reference Then it's kinda broken, no? I'm totally opposed to adding new APIs the expose window object references, because they completely fail in the e10s case.

That said, I can see how mapping from window id to <browser> might be a pain. How is Fennec solving that problem right now?

For the specific problem of console logging, we remote that from the content-->chrome process, and the content process has a window so all is well. More generally, I don't know what window IDs are so I can't speak for that. CC'ing mfinkle who should be able to.

Window ids are unique 64-bit numbers assigned to every window object. The idea was that they can be used to represent window identity without having to hand out actual window objects (both for e10s, and so people wouldn't hold leaky refs to them, and so that we can represent the identity of _destroyed_ window objects). I suppose I could just stick a window (or "context", as nsISupports) on nsIScriptError2 if that would be better all around.

I wasn't that we need to give exception objects window references, but rather pointing out that the web console kind of needs a way to get them somehow, at least for the moment. A getWindowById() method or somesuch would do, but I don't know of any existing code that tracks all windows in that manner, and I could see that being troublesome. The patch in bug 587734 adds a getWindowByWindowId in the frontend that uses the window enumerator, but that is problematic in several ways - slow, tabbrowser-specific, doesn't find subdocument windows, etc. The Web Console could alternatively do something like keeps its own list of child window IDs for a given <browser> (not sure how that would be populated...) and use that for the mapping, I guess, but keeping that list up to date across navigations/document creation and such seems like it would be ugly and error-prone. I haven't given much thought to how this should work in an e10s world (or even how it should ideally work now - I'm mostly looking into this code after the fact...).

OK. It sounds like the web console has one single consoleservice listener, not listener-per-tab? If it had the latter it could just walk its tab's windows (using window.frames) looking for the id in question. I suppose that could also get slow in rare cases.... Another option is to have the error include both the ID of the originating window _and_ of its window.top. That should make it pretty easy for console to tell what's going on, I hope. Does make logging the error slower, though... Jonas, thoughts?

that's right. The console has a single consoleservice listener.

It has one nsIConsoleService listener, yeah. Those can't be per-tab. It also has window.onerror handlers attached to every window (except subframes, until bug 598438 is fixed anyways), but those associate their errors differently (via a closure to the actual console object until bug 587734 lands, after which it will start using IDs and we'll have the same problem...)

> It also has window.onerror handlers attached to every window Yes, that's what this bug is trying to make unnecessary. Rob, what API would make things simplest/sanest for you here? Would exposing the id of .top on the error (which you could then map to a tab using a hashtable, basically) work?

(In reply to comment #15) > Rob, what API would make things simplest/sanest for you here? Would exposing > the id of .top on the error (which you could then map to a tab using a > hashtable, basically) work? That would be great - much better than the solution we use now. The downside is, as gavin mentioned, our method to map back to a window is slow and Firefox-specific.

Yes, I'm saying you should use a different method. ;) I suppose the other option is to just stick the window on there for now. Jonas, thoughts?

Either of these two methods would be preferable to the onError hack.

Using the windowID would be the safest in the long run. When Fx goes e10s, storing the window won't be usable, unless handles auto-magically save the day.

I think a window reference is what we should do for now. It's hard to say what things will look like in an e10s world so lets worry about that post FF4. I'd even say stick a .window and a .context property on there. That way consumers can rely on .window always being there and being a window, and .context can contain the relevant element or XHR or whatever as appropriate per error.

I would "vote" for the originating window ID, and that only. The window.top window object can be inferred from the originating window ID, and we can find the window objects based on ID. The getWindowById() method we have is suboptimal, but far better than the rest of the work arounds we have now. Boris: can't we have a Gecko level getWindowById() method? One that does a better job in finding the exact window object for the given ID. At the HUDService level we are obviously left with the only choice but to hold a map of window IDs and hudIds (HeadsUpDisplay objects). Based on messages coming from each window we would then easily and more nicely display messages in each Web Console, as needed. With regards to a .context property: that wouldn't be bad at all. It would be nice if it would point to the relevant element or XHR. But then, we get into the land of: what is "relevant"? The XHR object or the <script> element that includes the code ... which triggered the message? And so on. Ultimately, I believe we should focus: focus on what we *need*. That is a window ID and ideally a getWindowById(). We can improve our implementation of getWindowById(): we can hold a map of window IDs to hudIds, and a map of huIds to HUD objects. Each HUD object already points to the contentWindow (which is the top window from each xul:browser of each tab). When we want to display an error message coming from the nsIConsoleService we don't really need the window object reference at all. We want to find our HUD object, actually. To do that we just need the window ID. Thanks!

> Boris: can't we have a Gecko level getWindowById() method? You sure can, as long as you're ok with it returning null if the window is dead.

(In reply to comment #22) > You sure can, as long as you're ok with it returning null if the window is > dead. That'd be fine. Just adding windowID to nsIScriptErrors + adding a getWindowById() method in the core sounds like a good plan to me.

absolutely. Sounds like an easy solution.

Comment on attachment 484856 [details] [diff] [review] part 1. Add an nsIScriptError2 interface that exposes an outer window id, and set the window id for script errors coming through the DOM JS error reporter. I went with "ID" over "Id" because the existing windowutils functions capitalize it that way....

Comment on attachment 484857 [details] [diff] [review] part 2. Add a way to get an outer window object given its window id via windowutils. And here I went with "Id" to be consistent with GetElementById.... let me know if you think that's a terrible idea?

Beltzner said this is blocking b7 if I get it in expeditiously.

Pushed on m-c: http://hg.mozilla.org/mozilla-central/rev/ec4198d99816 http://hg.mozilla.org/mozilla-central/rev/768ada595b2b http://hg.mozilla.org/mozilla-central/rev/a72ca32bac1d (to make the new method not usable from untrusted script) and on b7 relbranch: http://hg.mozilla.org/mozilla-central/rev/666a6214dc7d http://hg.mozilla.org/mozilla-central/rev/d2ddc5f80339 http://hg.mozilla.org/mozilla-central/rev/455860e4a4ad

Holy Crap! Yay!

fantastic turn-around. Thanks bz and mrbkap!

Documented: https://developer.mozilla.org/en/XPCOM_Interface_Reference/nsIScriptError2 https://developer.mozilla.org/en/XPCOM_Interface_Reference/nsIDOMWindowUtils#getOuterWindowWithId()