1180555 - [Bluetooth] Handle PBAP replies from Gaia and pass the results to PbapManager.

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Bluetooth, defect)

Description

[Jamin Liu [:jaliu][:Jamin]]

Handle PBAP replys from Gaia and pass the results to PbapManager, the PBAP reply includes replyTovCardPulling, replyToPhonebookPulling and replyTovCardListing.

This bug should also support vCardSelecting.

Comment 1

[Jamin Liu [:jaliu][:Jamin]]

Attachment: Handle PBAP replys and pass the results through IPC to PbapManager. (v0)

Comment 2

[Jamin Liu [:jaliu][:Jamin]]

Attachment: Handle PBAP replys and pass the results through IPC to PbapManager. (v1)

Comment 3

[Ben Tian (inactive)]

Comment on attachment 8630940 [details] [diff] [review]
Handle PBAP replys and pass the results through IPC to PbapManager. (v1)

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

Please see my comment below.

::: dom/bluetooth/bluedroid/BluetoothPbapManager.cpp
@@ +753,5 @@
>    return propSelector;
>  }
>  
>  void
> +BluetoothPbapManager::ReplyToPullPhonebook(BlobParent* aActor, uint16_t aPhonebookSize)

nit: this line exceeds 80 characters. Please check all whole for too long lines.

@@ +766,4 @@
>  BluetoothPbapManager::ReplyToPullPhonebook(Blob* aBlob, uint16_t aPhonebookSize)
>  {
>    // TODO: Implement this function (Bug 1180556)
> +void

I think here results in compilation error. Can you confirm it?

::: dom/bluetooth/bluedroid/BluetoothPbapManager.h
@@ +62,5 @@
>    static const int MAX_PACKET_LENGTH = 0xFFFE;
>  
>    static BluetoothPbapManager* Get();
>    bool Listen();
> +  void ReplyToPullPhonebook(BlobParent* aActor, uint16_t aPhonebookSize);

Leave comment to state the different usages.

::: dom/bluetooth/bluedroid/BluetoothServiceBluedroid.cpp
@@ +1892,5 @@
> +  if (!pbap) {
> +    DispatchReplyError(aRunnable,
> +                       NS_LITERAL_STRING("Replay to vCardPulling failed"));
> +    return;
> +  } else {

Use guard clause instead of nested condition, here and following.

  if (!pbap) {
    ...
    return;
  }

  pbap->ReplyToPullvCardEntry(aBlobParent);

::: dom/bluetooth/bluetooth1/BluetoothAdapter.cpp
@@ +647,5 @@
>    }
>  
> +  nsRefPtr<BluetoothPbapRequestHandle> handle =
> +    BluetoothPbapRequestHandle::Create(GetOwner());
> +  init.mHandle = handle;

Assign directly, here and following

  init.mHandle = BluetoothPbapRequestHandle::Create(GetOwner());

Comment 4

[Jamin Liu [:jaliu][:Jamin]]

Attachment: Handle PBAP replys and pass the results through IPC to PbapManager. (v2)

- revise previous patch based on #comment 3.

Thank Ben for the comments.

Comment 5

[Ben Tian (inactive)]

Comment on attachment 8635244 [details] [diff] [review]
Handle PBAP replys and pass the results through IPC to PbapManager. (v2)

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

r=me with comment addressed.

::: dom/bluetooth/bluedroid/BluetoothPbapManager.h
@@ +64,5 @@
>    static BluetoothPbapManager* Get();
>    bool Listen();
> +
> +  /**
> +   * Reply vCard object to the *IPC* 'pullphonebook' request .

nit: remove extra space before period.

@@ +66,5 @@
> +
> +  /**
> +   * Reply vCard object to the *IPC* 'pullphonebook' request .
> +   *
> +   * @param aActor [in] a blob actor contained the vCard objects

nit: 'containing', here and following.

@@ +67,5 @@
> +  /**
> +   * Reply vCard object to the *IPC* 'pullphonebook' request .
> +   *
> +   * @param aActor [in] a blob actor contained the vCard objects
> +   * @param aPhonebookSize [in]  the number of vCard indexes in the blob.

nit: remove the period. Also please align these two lines, here and following.

   * @param aActor [in]          a blob actor containing the vCard objects
   * @param aPhonebookSize [in]  the number of vCard indexes in the blob

@@ +97,4 @@
>    void ReplyToPullvCardListing(Blob* aBlob, uint16_t aPhonebookSize);
> +
> +  /**
> +   * Reply vCard object to the *IPC* 'pullvcardentry' request .

nit: remove extra space before period.

::: dom/bluetooth/bluedroid/BluetoothServiceBluedroid.cpp
@@ +1890,5 @@
> +{
> +  BluetoothPbapManager* pbap = BluetoothPbapManager::Get();
> +  if (!pbap) {
> +    DispatchReplyError(aRunnable,
> +                       NS_LITERAL_STRING("Replay to vCardPulling failed"));

nit: 'Reply', here and following.

@@ +1939,5 @@
> +  uint16_t aPhonebookSize,
> +  BluetoothReplyRunnable* aRunnable)
> +{
> +  BluetoothPbapManager* pbap = BluetoothPbapManager::Get();
> +  if (!pbap) {

Guard clause please.

Comment 6

[Jamin Liu [:jaliu][:Jamin]]

Attachment: Handle PBAP replys and pass the results through IPC to PbapManager. (v3), r=btian

- revise previous patch based on #comment 5

Thank Ben for reviewing the patch.

Comment 7

[Jamin Liu [:jaliu][:Jamin]]

Attachment: Handle PBAP replys and pass the results through IPC to PbapManager. (v3.1), r=btian

- rebase

Comment 8

[Jamin Liu [:jaliu][:Jamin]]

Attachment: (final) Handle PBAP replys and pass the results through IPC to PbapManager. (v3.4), r=btian

- Rebase
- Add few empty functions to BluetoothDBusService to solve the compile errors of ICS emulator.

Comment 9

[Jamin Liu [:jaliu][:Jamin]]

Treeherder:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=4c4c52e8ba7e (ICS build was busted.)
https://treeherder.mozilla.org/#/jobs?repo=try&revision=144cddc7877e (ICS build was fixed by #Attachment 8649650 [details] [diff] in #Bug 1180555)

Comment 10

[Pulsebot]

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

Comment 11

[Ben Tian (inactive)]

Mark feature-b2g: 2.2r+ since PBAP is required bluetooth feature.

Comment 12

[Pulsebot]

https://hg.mozilla.org/integration/b2g-inbound/rev/5bdcc058e6d6

Comment 13

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out 3 changesets (131251625ee8 for bug 1180556, 5bdcc058e6d6 for bug 1180555, f7e0cd74c082 for bug 1180554) for B"G ICS Emulator opt M8 and debug M19 failures. r=backout
https://hg.mozilla.org/integration/b2g-inbound/rev/00b847293e70

Comment 14

[Pulsebot]

https://hg.mozilla.org/integration/b2g-inbound/rev/3c598d855e33

Comment 15

[Ben Tian (inactive)]

Attachment: [2.2r] Handle PBAP replys and pass the results through IPC to PbapManager. (v3.4), r=btian

Comment 16

[Ben Tian (inactive)]

checkin-needed to land 2.2r patch in comment 15.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3c598d855e33

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g37_v2_2r/rev/564db55d1942