Closed Bug 981845 Opened 6 years ago Closed 4 years ago

Hunt down all the things that implement DOM_OBJECT classinfo in JS

Categories

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

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: bzbarsky, Unassigned)

References

(Blocks 2 open bugs)

Details

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.
Depends on: 886110
Depends on: 899322, 903873
Depends on: 983845
comm-central has some of this junk too, might want to CC the Seamonkey and Chat folks.
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.
(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.
(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?
Depends on: 983916
Depends on: 983920
(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.
> 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...
(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.
> 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?
Depends on: 866528
(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.
Depends on: 984346
Depends on: 985796
Depends on: 986475
Depends on: 986837
Depends on: 986980
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.
Depends on: 901114, 986993, 986992
(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.
Flags: needinfo?(htsai)
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.
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.
Depends on: 987178
(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
Flags: needinfo?(htsai)
Depends on: 904588
No longer depends on: 864489
(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/
(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?
Depends on: 993311
No longer depends on: 904588
Depends on: 994461
Blocks: SH-APIs
No longer blocks: SH-APIs
After bug 994461, there is no DOM_OBJECT classinfo usage in RILContentHelper.js and RadioInterfaceLayer.js. No longer depends on bug 815526.
No longer depends on: deprecate-rilcontenthelper
Blocks: 1207321
All blockers are fixed. What's the next step?
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.
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.