Closed
Bug 1180555
Opened 9 years ago
Closed 8 years ago
[Bluetooth] Handle PBAP replies from Gaia and pass the results to PbapManager.
Categories
(Firefox OS Graveyard :: Bluetooth, defect)
Tracking
(feature-b2g:2.2r+, firefox43 fixed, b2g-v2.2r fixed)
People
(Reporter: jaliu, Assigned: jaliu)
References
Details
Attachments
(2 files, 5 obsolete files)
32.79 KB,
patch
|
Details | Diff | Splinter Review | |
32.44 KB,
patch
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → jaliu
Status: NEW → ASSIGNED
Assignee | ||
Comment 1•9 years ago
|
||
Assignee | ||
Comment 2•9 years ago
|
||
Attachment #8630265 -
Attachment is obsolete: true
Updated•9 years ago
|
Summary: [Bluetooth] Handle PBAP replys from Gaia and pass the results to PbapManager. → [Bluetooth] Handle PBAP replies from Gaia and pass the results to PbapManager.
Assignee | ||
Updated•9 years ago
|
Attachment #8630940 -
Flags: review?(btian)
Comment 3•9 years ago
|
||
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());
Attachment #8630940 -
Flags: review?(btian)
Assignee | ||
Comment 4•9 years ago
|
||
- revise previous patch based on #comment 3. Thank Ben for the comments.
Attachment #8630940 -
Attachment is obsolete: true
Attachment #8635244 -
Flags: review?(btian)
Comment 5•9 years ago
|
||
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.
Attachment #8635244 -
Flags: review?(btian) → review+
Assignee | ||
Comment 6•9 years ago
|
||
- revise previous patch based on #comment 5 Thank Ben for reviewing the patch.
Attachment #8635244 -
Attachment is obsolete: true
Assignee | ||
Updated•9 years ago
|
Attachment #8637045 -
Attachment description: Handle PBAP replys and pass the results through IPC to PbapManager. (v3) → Handle PBAP replys and pass the results through IPC to PbapManager. (v3), r=btian
Assignee | ||
Comment 8•8 years ago
|
||
- Rebase - Add few empty functions to BluetoothDBusService to solve the compile errors of ICS emulator.
Attachment #8647909 -
Attachment is obsolete: true
Assignee | ||
Comment 9•8 years ago
|
||
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 11•8 years ago
|
||
Mark feature-b2g: 2.2r+ since PBAP is required bluetooth feature.
feature-b2g: --- → 2.2r+
![]() |
||
Comment 13•8 years ago
|
||
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 15•8 years ago
|
||
Comment 17•8 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/3c598d855e33
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox43:
--- → fixed
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → FxOS-S6 (04Sep)
Comment 18•8 years ago
|
||
https://hg.mozilla.org/releases/mozilla-b2g37_v2_2r/rev/564db55d1942
status-b2g-v2.2r:
--- → fixed
See Also: → 1208492
You need to log in
before you can comment on or make changes to this bug.
Description
•