1223722 - Transfer strongly typed values in |BluetoothValue|

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Bluetooth, defect)

Description

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

Many value are transfered as nsString in |BluetoothValue|. They should be stored in dedicated types instead.

Comment 1

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

Attachment: [01] Bug 1223722: Transfer Bluetooth addresses in |BluetoothValue|

Comment 2

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

Attachment: [02] Bug 1223722: Transfer Bluetooth remote names in |BluetoothValue|

Comment 3

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

Attachment: [03] Bug 1223722: Transfer arrays of Bluetooth UUIDs in |BluetoothValue|

Comment 4

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

Hi!

This is the patch set for sending typed values from parent to content process. Currently these values are sent as |nsString|.

Comment 5

[Jocelyn Liu [:jocelyn] [:joliu]]

Comment on attachment 8699403 [details] [diff] [review]
[01] Bug 1223722: Transfer Bluetooth addresses in |BluetoothValue|

Sorry, I would like to forward the review request to Bruce since I'll be on PTO from 12/23 to 1/6.

Thanks a lot for your help, Bruce.

Comment 6

[Bruce Sun [:brsun]]

Comment on attachment 8699403 [details] [diff] [review]
[01] Bug 1223722: Transfer Bluetooth addresses in |BluetoothValue|

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

LGTM

::: dom/bluetooth/common/webapi/BluetoothPairingListener.cpp
@@ +53,4 @@
>                                                 const nsAString& aPasskey,
>                                                 const nsAString& aType)
>  {
> +  MOZ_ASSERT(!aName.IsEmpty() && !aType.IsEmpty());

nit: the assertion for |aAddress|

Comment 7

[Bruce Sun [:brsun]]

Comment on attachment 8699404 [details] [diff] [review]
[02] Bug 1223722: Transfer Bluetooth remote names in |BluetoothValue|

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

LGTM

Comment 8

[Bruce Sun [:brsun]]

Comment on attachment 8699405 [details] [diff] [review]
[03] Bug 1223722: Transfer arrays of Bluetooth UUIDs in |BluetoothValue|

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

Basically it looks good to me.

It would be good to add comments in codes or in commit messages to describe more about the reason why sorting stuffs are moved from the parent process to the content process. The decision making would be interesting and worth being noted.

Comment 9

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

(In reply to Bruce Sun [:brsun] from comment #8)
> Comment on attachment 8699405 [details] [diff] [review]
> [03] Bug 1223722: Transfer arrays of Bluetooth UUIDs in |BluetoothValue|
> 
> Review of attachment 8699405 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Basically it looks good to me.
> 
> It would be good to add comments in codes or in commit messages to describe
> more about the reason why sorting stuffs are moved from the parent process
> to the content process. The decision making would be interesting and worth
> being noted.

That's a good point. I will add a rational in the patch's next iteration.

Comment 10

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

Attachment: [01] Bug 1223722: Transfer Bluetooth addresses in |BluetoothValue| (v2)

Changes since v1:

  - added missing |MOZ_ASSERT| for address

Comment 11

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

Attachment: [02] Bug 1223722: Transfer Bluetooth remote names in |BluetoothValue| (v2)

Changes since v1:

  - fixed incorrect type when setting device name (string->remote name)

Comment 12

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

Attachment: [03] Bug 1223722: Transfer arrays of Bluetooth UUIDs in |BluetoothValue| (v2)

Changes since v1:

  - added rational to commit message

Comment 13

[Pulsebot]

https://hg.mozilla.org/integration/b2g-inbound/rev/21454026054e
https://hg.mozilla.org/integration/b2g-inbound/rev/a3eb613f20c9
https://hg.mozilla.org/integration/b2g-inbound/rev/d32bbbc72726

Comment 14

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

Thanks Bruce!

https://treeherder.mozilla.org/#/jobs?repo=b2g-inbound&revision=d32bbbc72726

Comment 15

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

https://hg.mozilla.org/mozilla-central/rev/21454026054e
https://hg.mozilla.org/mozilla-central/rev/a3eb613f20c9
https://hg.mozilla.org/mozilla-central/rev/d32bbbc72726