1147921 - Let addon-sdk/source/lib/sdk/io/stream.js use STREAM transport service instead

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: General, defect, P2)

Description

[Honza Bambas (:mayhemer)]

http://hg.mozilla.org/mozilla-central/annotate/37d3dcbf23a9/addon-sdk/source/lib/sdk/io/stream.js#l296

Where the eventTarget is socket transport service.  It should use "@mozilla.org/network/stream-transport-service;1".

Comment 1

[Honza Bambas (:mayhemer)]

Attachment: v1

Comment 2

[Patrick McManus [:mcmanus]]

thanks honza!

Comment 3

[Erik Vold [:erikvold] (please needinfo? me)]

(In reply to Honza Bambas (:mayhemer) from comment #0)
> http://hg.mozilla.org/mozilla-central/annotate/37d3dcbf23a9/addon-sdk/source/
> lib/sdk/io/stream.js#l296
> 
> Where the eventTarget is socket transport service.  It should use
> "@mozilla.org/network/stream-transport-service;1".

This there a bug describing this change which this bug should be dependent on?

Comment 4

[Will Bamberg [:wbamberg]]

Comment on attachment 8583998 [details] [diff] [review]
v1

I'm not qualified to review this. Erik, would you mind taking it?

Comment 5

[Honza Bambas (:mayhemer)]

(In reply to Erik Vold [:erikvold] (please needinfo? me) from comment #3)
> (In reply to Honza Bambas (:mayhemer) from comment #0)
> > http://hg.mozilla.org/mozilla-central/annotate/37d3dcbf23a9/addon-sdk/source/
> > lib/sdk/io/stream.js#l296
> > 
> > Where the eventTarget is socket transport service.  It should use
> > "@mozilla.org/network/stream-transport-service;1".
> 
> This there a bug describing this change which this bug should be dependent
> on?

I don't understand the question.

Comment 6

[Erik Vold [:erikvold] (please needinfo? me)]

(In reply to Honza Bambas (:mayhemer) from comment #5)
> (In reply to Erik Vold [:erikvold] (please needinfo? me) from comment #3)
> > (In reply to Honza Bambas (:mayhemer) from comment #0)
> > > http://hg.mozilla.org/mozilla-central/annotate/37d3dcbf23a9/addon-sdk/source/
> > > lib/sdk/io/stream.js#l296
> > > 
> > > Where the eventTarget is socket transport service.  It should use
> > > "@mozilla.org/network/stream-transport-service;1".
> > 
> > This there a bug describing this change which this bug should be dependent
> > on?
> 
> I don't understand the question.

I'm not sure why `stream-transport-service` should be instead of `socket-transport-service` here.

Though the purpose of the module is streaming, it appears we can use either method to get the same nsIEventTarget service.

Comment 7

[Dave Townsend [:mossop]]

(In reply to Erik Vold [:erikvold] (please needinfo? me) from comment #6)
> (In reply to Honza Bambas (:mayhemer) from comment #5)
> > (In reply to Erik Vold [:erikvold] (please needinfo? me) from comment #3)
> > > (In reply to Honza Bambas (:mayhemer) from comment #0)
> > > > http://hg.mozilla.org/mozilla-central/annotate/37d3dcbf23a9/addon-sdk/source/
> > > > lib/sdk/io/stream.js#l296
> > > > 
> > > > Where the eventTarget is socket transport service.  It should use
> > > > "@mozilla.org/network/stream-transport-service;1".
> > > 
> > > This there a bug describing this change which this bug should be dependent
> > > on?
> > 
> > I don't understand the question.
> 
> I'm not sure why `stream-transport-service` should be instead of
> `socket-transport-service` here.
> 
> Though the purpose of the module is streaming, it appears we can use either
> method to get the same nsIEventTarget service.

The two implementations of nsIEventTarget differ though. In the socket service there is just a single thread that any events are dispatched to, the stream transport on the other hand manages a pool of 25 threads. I'd guess that using the socket service might mean we block actual network traffic but either way seems sensible to use the stream service given that we're using the stream APIs.

Comment 8

[Erik Vold [:erikvold] (please needinfo? me)]

Comment on attachment 8583998 [details] [diff] [review]
v1

Given Mossop's feedback

Comment 9

[[github robot]]

Commit pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/e3caddf2da1b3441706abd530064e26bbc0e1b67
Bug 1147921 - Let addon-sdk/source/lib/sdk/io/stream.js use STREAM transport service instead r=erikvold