981845 - Hunt down all the things that implement DOM_OBJECT classinfo in JS

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Boris Zbarsky [:bzbarsky]]

These need to move to WebIDL bindings.  The current list is:

    browser/components/feeds/src/FeedWriter.js
    browser/components/feeds/src/WebContentConverter.js
    browser/metro/components/Sidebar.js
    dom/network/src/TCPSocket.js
    dom/network/src/TCPServerSocket.js
    dom/system/gonk/RadioInterfaceLayer.js
    dom/system/gonk/RILContentHelper.js
    dom/system/NetworkGeolocationProvider.js
    dom/wifi/DOMWifiManager.js
    dom/apps/src/Webapps.js
    dom/messages/SystemMessageManager.js
    dom/activities/src/ActivityOptions.js
    dom/activities/src/ActivityRequestHandler.js
    dom/permission/PermissionSettings.js
    dom/payment/Payment.js
    dom/payment/PaymentRequestInfo.js
    dom/inputmethod/MozKeyboard.js
    mobile/android/components/Sidebar.js

This is a tracking bug.  Individual bugs should be filed blocking this one.

Comment 1

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

comm-central has some of this junk too, might want to CC the Seamonkey and Chat folks.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Hmm, indeed.  Looks like:

  suite/common/src/nsSidebar.js
  suite/feeds/src/FeedWriter.js 
  suite/feeds/src/WebContentConverter.js
  chat/modules/jsProtoHelper.jsm

Moving those to WebIDL would be nice.

Comment 3

[Robert Kaiser]

(In reply to Boris Zbarsky [:bz] from comment #2)
>   suite/common/src/nsSidebar.js
>   suite/feeds/src/FeedWriter.js 
>   suite/feeds/src/WebContentConverter.js

Those are pretty much copies of what browser/ has there, IIRC. That said, I'm not working on SeaMonkey any more, but Neil is the over-arching code module owner for suite/ code.

Comment 4

[[e-mail]]

(In reply to Boris Zbarsky from comment #2)
> Hmm, indeed.  Looks like:
> 
>   suite/common/src/nsSidebar.js
>   suite/feeds/src/FeedWriter.js 
>   suite/feeds/src/WebContentConverter.js
> 
> Moving those to WebIDL would be nice.

Last time someone mentioned this it turned out to be impossible. Has something changed?

Comment 5

[(no longer active)]

(In reply to comment #4)
> (In reply to Boris Zbarsky from comment #2)
> > Hmm, indeed.  Looks like:
> > 
> >   suite/common/src/nsSidebar.js
> >   suite/feeds/src/FeedWriter.js 
> >   suite/feeds/src/WebContentConverter.js
> > 
> > Moving those to WebIDL would be nice.
> 
> Last time someone mentioned this it turned out to be impossible. Has something
> changed?

For these three, I don't actually think you need to do anything.  Once I land the m-c fixes for those three (see the dependencies) I bet you can just remove the classinfo goop from those files and everything will work fine.

Comment 6

[Boris Zbarsky [:bzbarsky]]

> Last time someone mentioned this it turned out to be impossible.

Which part was impossible?

The only requirement for moving things like that to webidl is that the binding be compiled into the relevant Gecko...

Comment 7

[[e-mail]]

(In reply to Ehsan Akhgari from comment #5)
> (In reply to comment #4)
> > (In reply to Boris Zbarsky from comment #2)
> > > Hmm, indeed.  Looks like:
> > > 
> > >   suite/common/src/nsSidebar.js
> > >   suite/feeds/src/FeedWriter.js 
> > >   suite/feeds/src/WebContentConverter.js
> > > 
> > > Moving those to WebIDL would be nice.
> > 
> > Last time someone mentioned this it turned out to be impossible. Has
> > something changed?
> 
> For these three, I don't actually think you need to do anything.  Once I
> land the m-c fixes for those three (see the dependencies) I bet you can just
> remove the classinfo goop from those files and everything will work fine.

WebContentConverter.js is a simple port, but our other components differ.

(In reply to Boris Zbarsky from comment #6)
> > Last time someone mentioned this it turned out to be impossible.
> 
> Which part was impossible?
> 
> The only requirement for moving things like that to webidl is that the
> binding be compiled into the relevant Gecko...

Exactly, so as per your newsgroup post, still impossible.

Comment 8

[Boris Zbarsky [:bzbarsky]]

> but our other components differ

Differ in terms of the signatures of the API you want to expose to untrusted pages?

> still impossible

I don't see why.  Just compile the binding?

Comment 9

[(no longer active)]

(In reply to comment #8)
> > but our other components differ
> 
> Differ in terms of the signatures of the API you want to expose to untrusted
> pages?

Yeah, if the API signature is the same, and the contract ID for the JS components implementing them are also the same, things should work out of the box.

Comment 10

[(no longer active)]

So, here's a list of the remaining ones for which I don't have a patch up for review:

dom/alarm/AlarmsManager.js: bug 901114, [1]
dom/apps/src/Webapps.js: bug 899322, Fabrice told me on IRC that he's hoping to work on it at some point
dom/inputmethod/MozKeyboard.js: bug 986992 and bug 986993
dom/network/src/NetworkStatsManager.js: [1]
dom/network/src/TCPServerSocket.js: bug 885982
dom/network/src/TCPSocket.js: bug 885982
dom/payment/Payment.js: need to figure out bug 986455
dom/system/gonk/RILContentHelper.js: I am very afraid to touch this code
dom/system/gonk/RadioInterfaceLayer.js: ditto
dom/wifi/DOMWifiManager.js: bug 886110

[1] needs to block on bug 983502, otherwise porting these APIs to WebIDL properly will break MarketPlace feature detection.

IOW, my involvement here is soon going to be over.  It would be nice if someone can work on the RIL APIs.

Comment 11

[Bobby Holley (:bholley)]

(In reply to :Ehsan Akhgari (lagging on bugmail, needinfo? me!) from comment #10)
> dom/system/gonk/RILContentHelper.js: I am very afraid to touch this code
> dom/system/gonk/RadioInterfaceLayer.js: ditto

> IOW, my involvement here is soon going to be over.  It would be nice if
> someone can work on the RIL APIs.

Hsin-Yi - can you (or someone on your team) convert the RIL APIs to WebIDL? This is a big priority for platform architecture and security.

Comment 12

[aleth [:aleth]]

Could someone please point me at an explanation of what exactly nsIClassInfo::DOM_OBJECT does? This would help in confirming my guess that we can just drop it in comm-central/chat.

Comment 13

[Boris Zbarsky [:bzbarsky]]

It allows untrusted (random web page) code to receive an instance of the object through XPConnect.

Or more precisely, given an nsISupports that XPConnect is wrapping into a web page scope, the wrapping is only allowed if the nsISupports has classinfo and the classinfo is DOM_OJECT.

Comment 14

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Bobby Holley (:bholley) from comment #11)
> (In reply to :Ehsan Akhgari (lagging on bugmail, needinfo? me!) from comment
> #10)
> > dom/system/gonk/RILContentHelper.js: I am very afraid to touch this code
> > dom/system/gonk/RadioInterfaceLayer.js: ditto
> 
> > IOW, my involvement here is soon going to be over.  It would be nice if
> > someone can work on the RIL APIs.
> 
> Hsin-Yi - can you (or someone on your team) convert the RIL APIs to WebIDL?
> This is a big priority for platform architecture and security.

We already had the bugs for this purpose:
1) Bug 815526 - [meta] B2G RIL: deprecate RILContentHelper -- the information object exposed to gaia will be moved to WebIDL
2) Bug 864489, Bug 898445, Bug 906404 and Bug 906398 -- move WebAPI to WebIDL

Comment 15

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Hsin-Yi Tsai  [:hsinyi] from comment #14)
> (In reply to Bobby Holley (:bholley) from comment #11)
> > (In reply to :Ehsan Akhgari (lagging on bugmail, needinfo? me!) from comment
> > #10)
> > > dom/system/gonk/RILContentHelper.js: I am very afraid to touch this code
> > > dom/system/gonk/RadioInterfaceLayer.js: ditto
> > 
> > > IOW, my involvement here is soon going to be over.  It would be nice if
> > > someone can work on the RIL APIs.
> > 
> > Hsin-Yi - can you (or someone on your team) convert the RIL APIs to WebIDL?
> > This is a big priority for platform architecture and security.
> 
> We already had the bugs for this purpose:
> 1) Bug 815526 - [meta] B2G RIL: deprecate RILContentHelper -- the
> information object exposed to gaia will be moved to WebIDL
> 2) Bug 864489, Bug 898445, Bug 906404 and Bug 906398 -- move WebAPI to WebIDL
Sorry, s/bug 864489/bug 904588/

Comment 16

[Bobby Holley (:bholley)]

(In reply to Hsin-Yi Tsai  [:hsinyi] from comment #14)
> We already had the bugs for this purpose:
> 1) Bug 815526 - [meta] B2G RIL: deprecate RILContentHelper -- the
> information object exposed to gaia will be moved to WebIDL
> 2) Bug 864489, Bug 898445, Bug 906404 and Bug 906398 -- move WebAPI to WebIDL

Ok, great! Are they actively resourced? What's the expected timeframe?

Comment 17

[Edgar Chen [:edgar]]

After bug 994461, there is no DOM_OBJECT classinfo usage in RILContentHelper.js and RadioInterfaceLayer.js. No longer depends on bug 815526.

Comment 18

[Masatoshi Kimura [:emk]]

All blockers are fixed. What's the next step?

Comment 19

[Boris Zbarsky [:bzbarsky]]

The main idea was to make sure we didn't have any more DOM_OBJECT classinfo in our tree.  That's done now.  I think this bug as filed is fixed, unless we want to track extensions as well.  I filed bug 1245140 to track that.