792823 - [b2g-bluetooth] BluetoothOppManager prototype

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

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

We need a basic implementation of BluetoothOppManager, including registering OPP service.

Comment 1

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

Attachment: patch 1: v1: register ObjectPush service

Comment 2

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

Assuming this is the end of our service additions for v1, we might want to remove the unused constants for the time being, just to clean up the warnings.

Comment 3

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

Attachment: patch 1: v2: register ObjectPush service

Comment 4

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

Attachment: patch 2: v1: BluetoothOppManager interface and prototype

Please note that SendFile, StopSendingFile and ReceiveSocketData will be implemented in another issue because they are all related to OBEX protocol.

Comment 5

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

Comment on attachment 663370 [details] [diff] [review]
patch 2: v1: BluetoothOppManager interface and prototype

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

::: dom/bluetooth/BluetoothOppManager.cpp
@@ +11,5 @@
> +#include "BluetoothServiceUuid.h"
> +
> +USING_BLUETOOTH_NAMESPACE
> +
> +static BluetoothOppManager* sInstance = nullptr;

Make this a RefPtr. Since it's inheriting UnixSocketConsumer it'll get RefCounted from there, and bad, afternoon eating things will happen if we don't do this. (Also: Change HfpManager to refcounted)

@@ +74,5 @@
> +  }
> +
> +  nsRefPtr<BluetoothReplyRunnable> runnable = aRunnable;
> +
> +  nsresult rv = bs->CloseSocket(this, runnable);

Using the new socket stuff, we can just call close on ourselves. No need to pull a service.

::: dom/bluetooth/BluetoothOppManager.h
@@ +35,5 @@
> +   * process. At the end, the application will get a system message
> +   * indicating that te process is complete, then it can either call
> +   * Disconnect() to close RFCOMM connection or start another file-sending
> +   * thread via calling SendFile() again.
> +   */

Since we're explaining things there, might want to add something about sending progress as a system message too?

Comment 6

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

Attachment: patch 1: final: register ObjectPush service and remove unused services, r=qdot

Comment 7

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

Attachment: patch 2: v2: BluetoothOppManager interface and prototype

* Added more comments 
* Passed a BlobParent* variable in SendFile()
* Disconnect => void Disconnect()

Comment 8

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

Comment on attachment 665453 [details] [diff] [review]
patch 2: v2: BluetoothOppManager interface and prototype

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

::: dom/bluetooth/BluetoothOppManager.cpp
@@ +15,5 @@
> +
> +USING_BLUETOOTH_NAMESPACE
> +using namespace mozilla::ipc;
> +
> +static nsRefPtr<BluetoothOppManager> sInstance = nullptr;

Just a RefPtr, actually, not an nsRefPtr. We aren't an XPCOM object... yet. :)

Comment 9

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

Attachment: patch 2: final: BluetoothOppManager interface and prototype, r=qdot

Nit fixed. Used nsAutoPtr instead of nsRefPtr to hold sInstance.

Comment 10

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

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

Comment 11

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

Attachment: patch 2: final: BluetoothOppManager interface and prototype, r=qdot

Replace nsRefPtr with RefPtr

Comment 12

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

"Try" again: https://tbpl.mozilla.org/?tree=Try&rev=0010bc06f156

Comment 13

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/9946c7e4dd92
https://hg.mozilla.org/integration/mozilla-inbound/rev/5ce162e8bb47

Comment 14

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/9946c7e4dd92
https://hg.mozilla.org/mozilla-central/rev/5ce162e8bb47