about:net-internals can leave random pending exceptions lying around

RESOLVED FIXED

Status

()

Core
Networking
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: bz, Assigned: valentin)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

The code checked in in bug looks to me like it can leave exceptions lying around on the safe JS context when it returns to the event loop.  For example, Dashboard::GetSockets runs directly off the event loop and as far as I can tell, and has all sorts of returns on things that set pending exceptions without ever reporting or clearing them.  Again, as far as I can tell.

What I don't quite understand, honestly, is why we're doing all this manual jsapi stuff.  We have various binding code that can do these sorts of things, and if it's lacking somehow for this use case we can fix it as needed instead of hand-rolling error-prone JSAPI code...
(In reply to Boris Zbarsky (:bz) from comment #0)
> The code checked in in bug looks...

bug 783205
Also, you really shouldn't use nsContentUtils from netwerk/ if you can avoid it. You can get the context stack from XPConnect directly (through @mozilla.org/js/xpc/ContextStack;1).
(Assignee)

Comment 3

6 years ago
 (In reply to Boris Zbarsky (:bz) from comment #0)
> What I don't quite understand, honestly, is why we're doing all this manual
> jsapi stuff.  We have various binding code that can do these sorts of
> things, and if it's lacking somehow for this use case we can fix it as
> needed instead of hand-rolling error-prone JSAPI code...

@bz
At the time it seemed like the best way to achieve this.
Could you provide a link to the helper code I could use instead of this approach?
What I think we should do is see if these objects can be expressed as WebIDL dictionaries.  If so, I would recomend doing that and using existing facilities for converting a dictionary to a JS value.  See <https://developer.mozilla.org/en-US/docs/Mozilla/WebIDL_bindings#Dictionary_types>.  There would still need to be manual exception-reporting, unfortunately, but it would happen at a single chokepoint and you wouldn't need the rest of the JSAPI usage.

Note that the dictionary bits involved have been in the tree since October.  If they're not sufficient for some reason and you're interested in using WebIDL here, please let me know what's missing and I'll fix it.

But obviously this bug can be fixed while sticking to manual JSAPI too.  If we want to do the dictionary thing it might want a separate bug.
(Assignee)

Comment 5

6 years ago
(In reply to Boris Zbarsky (:bz) from comment #4)
> 
> Note that the dictionary bits involved have been in the tree since October. 
> If they're not sufficient for some reason and you're interested in using
> WebIDL here, please let me know what's missing and I'll fix it.
> 

I'm trying to convert it all to webidl, however, I'm running into the next issue:

interface NetDashboard {
readonly attribute DOMString[] host;
...
};

The error I get is:

  File "/home/icecold/ff_trunk_debug1/dom/bindings/Codegen.py", line 3612, in getRetvalDeclarationForType
    returnType)
TypeError: Don't know how to declare return value for StringArray

I also tried defining the attribute as a sequence<DOMString>, but apparently that's against the webidl specifications and so it doesn't work.
What do you want this .host attribute to actually return?  If it's a new JS array every time, you should probably use a method, not an attribute, right?

That said, the current API has no such methods. It just hands out JS objects (in WebIDL those would be dictionaries) with a "host" array on them (a sequence<DOMString> member in the dictionary in WebIDL terms).
(Assignee)

Updated

6 years ago
Depends on: 843865
(Assignee)

Updated

5 years ago
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.