997137 - [Bluetooth] Inherit DroidSocketImpl from UnixFDWatcher

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Bluetooth, defect)

Description

[Thomas Zimmermann [:tzimmermann] [:tdz]]

|UnixFdWatcher| implements most of the infrastructure for maintaining file descritors on the I/O loop. We should use it for making DroidSocketImpl a bit more compatible with UnixSocketConsumer.

Comment 1

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Attachment: [01] Bug 997137: Inherit |DroidSocketImpl| from |UnixFdWatcher|

Comment 2

[Eric Chou [:ericchou] [:echou]]

Hi Thomas,

I have several 1.3/1.3T/1.4 plus review requests in hand. I'll review once those have been done. :)

Comment 3

[Eric Chou [:ericchou] [:echou]]

Comment on attachment 8408058 [details] [diff] [review]
[01] Bug 997137: Inherit |DroidSocketImpl| from |UnixFdWatcher|

Review of attachment 8408058 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with nits addressed or questions answered. Sorry for the late review. :)

::: dom/bluetooth/bluedroid/BluetoothSocket.cpp
@@ +7,5 @@
>  #include "BluetoothSocket.h"
>  
>  #include <hardware/bluetooth.h>
>  #include <hardware/bt_sock.h>
> +#include <fcntl.h>

super-nit: alphabetic order, please.

@@ +461,2 @@
>  
> +  if (!(flags&O_NONBLOCK)) {

nit: we may need whitespaces around the '&', couldn't find related rules in the coding style guide, though. (In fact I guess you might tend to do this since you didn't leave blanks around neither '&' nor '|', however personally I prefer adding blanks around).

@@ +692,5 @@
>    MOZ_ASSERT(NS_IsMainThread());
>    NS_ENSURE_FALSE(mImpl, false);
>  
>    mIsServer = false;
> +  mImpl = new DroidSocketImpl(XRE_GetIOMessageLoop(), this, aDeviceAddress, aChannel, mAuth, mEncrypt);

nit: 80 chars per line, please.

@@ +706,5 @@
>    MOZ_ASSERT(NS_IsMainThread());
>    NS_ENSURE_FALSE(mImpl, false);
>  
>    mIsServer = true;
> +  mImpl = new DroidSocketImpl(XRE_GetIOMessageLoop(), this, aChannel, mAuth, mEncrypt);

ditto.

Comment 4

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Attachment: [01] Bug 997137: Inherit |DroidSocketImpl| from |UnixFdWatcher| (v2)

Changes since v1:

  - reordered include statements
  - added whitespaces around operators
  - wrap lines before 80th column

Comment 5

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Thanks, Eric!

https://hg.mozilla.org/integration/b2g-inbound/rev/4d4ba42d23b3

https://tbpl.mozilla.org/?tree=B2g-Inbound&rev=4d4ba42d23b3

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/4d4ba42d23b3