790133 - [b2g-bluetooth] Fire devicecreated event with a BluetoothDevice

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[Gina Yeh [:gyeh] [:ginayeh]]

This is a followup of Bug 789014. When there's a random device try to pair with us, we need to create an actual BluetoothDevice first, and pass it to Gaia with a ondevicecreated event.

Comment 1

[Gina Yeh [:gyeh] [:ginayeh]]

Attachment: v1: Fire devicecreated event with a BluetoothDevice

When Service gets "DeviceCreated", we create another thread to get device properties, and send the dict back to adapter which fires ondevicecreated event with an instance of BluetoothDevice.

Comment 2

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Comment on attachment 660343 [details] [diff] [review]
v1: Fire devicecreated event with a BluetoothDevice

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

::: dom/bluetooth/linux/BluetoothDBusService.cpp
@@ +1243,5 @@
> +    mDevicePath(aDevicePath),
> +    mSignalPath(aSignalPath)
> +  {
> +    MOZ_ASSERT(NS_IsMainThread());
> +    MOZ_ASSERT(aDevicePath && aSignalPath);

Nit: These are references, so not sure what you're asserting here?

Comment 3

[Gina Yeh [:gyeh] [:ginayeh]]

Oops! I'll remove the assertion in the final version patch. Thanks!

Comment 4

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 660343 [details] [diff] [review]
v1: Fire devicecreated event with a BluetoothDevice

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

sr=me with a couple of nits picked and one real comment fixed.

::: dom/bluetooth/BluetoothAdapter.cpp
@@ +315,5 @@
> +    NS_NewDOMBluetoothDeviceEvent(getter_AddRefs(event), nullptr, nullptr);
> +
> +    nsCOMPtr<nsIDOMBluetoothDeviceEvent> e = do_QueryInterface(event);
> +    e->InitBluetoothDeviceEvent(NS_LITERAL_STRING("devicecreated"),
> +                                     false, false, device);

Nit: this looks misindented.

::: dom/bluetooth/linux/BluetoothDBusService.cpp
@@ +919,5 @@
> +      NS_WARNING("BluetoothService not available!");
> +    }
> +
> +    if (NS_FAILED(bs->GetDevicePropertiesInternal(v, signalPath))) {
> +		  NS_WARNING("get properties failed");

Nit: looks like some tabs crept in here.

@@ +1277,5 @@
> +    if (msg) {
> +      dbus_message_unref(msg);
> +    }
> +
> +    BluetoothSignal signal(NS_ConvertUTF8toUTF16("DeviceCreated"),

This should be NS_LITERAL_STRING.

Comment 5

[Gina Yeh [:gyeh] [:ginayeh]]

Attachment: Final version: Fire devicecreated event with a BluetoothDevice, r=qdot, sr=mrbkap

Nit picked.

Comment 6

[Gina Yeh [:gyeh] [:ginayeh]]

try server: https://tbpl.mozilla.org/?tree=Try&rev=2cf0e556bf34

Comment 7

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/35c8685ee065

Comment 8

[Ed Morley [:emorley]]

Backed out for failing to compile on B2G:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c97bb39c2ea6

Comment 9

[Gina Yeh [:gyeh] [:ginayeh]]

Attachment: v3: Fire devicecreated event with a BluetoothDevice

The old patch has been out-dated since the ipc patch has been landed, so I changed thee patch a little bit to fit in the current code base. Please help me to check, thanks.

Comment 10

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Comment on attachment 661449 [details] [diff] [review]
v3: Fire devicecreated event with a BluetoothDevice

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

Looks good code-wise, at least in-process, need to file a followup for OOP. Have you tried running it with the settings app OOP'd just to make sure it doesn't crash? (Go into gaia/apps/system/js/window-manager.js, search for "Settings", it'll be an entry in the oop-blacklist or something like that, comment out that line, rebuild gaia)

Comment 11

[Gina Yeh [:gyeh] [:ginayeh]]

try server: https://tbpl.mozilla.org/?tree=Try&rev=10d6ba9b8b21

Comment 12

[Gina Yeh [:gyeh] [:ginayeh]]

Attachment: Final version: Fire devicecreated event with a BluetoothDevice, r=qdot, sr=mrbkap

Comment 13

[Gina Yeh [:gyeh] [:ginayeh]]

try server: https://tbpl.mozilla.org/?tree=Try&rev=fa8a0263abc0

Comment 14

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/5796eacef43c

Comment 15

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

Backout 5796eacef43c (bug 790133) for B2G compilation failures again 

https://hg.mozilla.org/integration/mozilla-inbound/rev/6f6429620051

Talked with Gina, and we think this is because I didn't push the right patch but pushed former one. Sorry about this.

Comment 16

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/20200b3d443e

Comment 17

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/20200b3d443e