Last Comment Bug 783520 - [b2g-bluetooth] follow-up to bug 730992 (event codegen and value initialization)
: [b2g-bluetooth] follow-up to bug 730992 (event codegen and value initialization)
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM: Device Interfaces (show other bugs)
: unspecified
: ARM Gonk (Firefox OS)
: -- normal (vote)
: mozilla17
Assigned To: Eric Chou [:ericchou] [:echou]
:
Mentors:
Depends on:
Blocks: b2g-bluetooth
  Show dependency treegraph
 
Reported: 2012-08-17 04:40 PDT by Eric Chou [:ericchou] [:echou]
Modified: 2012-08-23 03:51 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
v1: used codegen and add BluetoothAuthorizeEvent (12.26 KB, patch)
2012-08-21 03:34 PDT, Eric Chou [:ericchou] [:echou]
no flags Details | Diff | Splinter Review
v1: used codegen and add BluetoothAuthorizeEvent (11.48 KB, patch)
2012-08-21 03:40 PDT, Eric Chou [:ericchou] [:echou]
kyle: review+
Details | Diff | Splinter Review
Final: used codegen and add BluetoothAuthorizeEvent (12.92 KB, patch)
2012-08-22 00:39 PDT, Eric Chou [:ericchou] [:echou]
no flags Details | Diff | Splinter Review

Description Eric Chou [:ericchou] [:echou] 2012-08-17 04:40:33 PDT
1. mPasskey & mUuid are not initialized in create() of BluetoothPairingEvent.
2. Use event codegen to implement BluetoothPairingEvent? 

(According to comment: https://bugzilla.mozilla.org/show_bug.cgi?id=730992#c62)
Comment 1 Eric Chou [:ericchou] [:echou] 2012-08-17 04:53:01 PDT
About item (1), I'll separate BluetoothPairingEvent into two events. One contains deviceAddress & uuid, the other one contains deviceAddress & passkey.
Comment 2 Eric Chou [:ericchou] [:echou] 2012-08-21 03:34:36 PDT
Created attachment 653701 [details] [diff] [review]
v1: used codegen and add BluetoothAuthorizeEvent

Used event generator to create BluetoothPairingEvent and BluetoothAuthorizeEvent & avoid passing non-initialized or no-value property in an event.
Comment 3 Eric Chou [:ericchou] [:echou] 2012-08-21 03:40:01 PDT
Created attachment 653702 [details] [diff] [review]
v1: used codegen and add BluetoothAuthorizeEvent

Redundant modification removed.
Comment 4 Kyle Machulis [:kmachulis] [:qdot] 2012-08-21 09:43:59 PDT
Comment on attachment 653702 [details] [diff] [review]
v1: used codegen and add BluetoothAuthorizeEvent

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

::: dom/bluetooth/BluetoothAdapter.cpp
@@ +339,5 @@
> +    e->InitBluetoothPairingEvent(NS_LITERAL_STRING("requestconfirmation"),
> +                                 false,
> +                                 false,
> +                                 arr[0].value().get_nsString(),
> +                                 arr[1].value().get_uint32_t());

Nit: Might want to put NS_ASSERTION checks on array length and value types.
Comment 5 Eric Chou [:ericchou] [:echou] 2012-08-22 00:39:09 PDT
Created attachment 654121 [details] [diff] [review]
Final: used codegen and add BluetoothAuthorizeEvent

Used NS_ASSERTION to verify parameter length and type.
Comment 6 Eric Chou [:ericchou] [:echou] 2012-08-22 00:46:02 PDT
Try: https://tbpl.mozilla.org/?tree=Try&rev=c05e571d1fe5
Comment 7 Eric Chou [:ericchou] [:echou] 2012-08-22 03:38:04 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/e8e54b17e6f2
Comment 8 Eric Chou [:ericchou] [:echou] 2012-08-22 20:32:16 PDT
This patch was uploaded to inbound yesterday (See comment 7), but has not been merged to m-c. So added the "checkin-needed" label to notify.
Comment 9 Ed Morley [:emorley] 2012-08-23 03:12:21 PDT
Patches aren't checked in a second time to mozilla-central - someone just has to manually merge inbound to mozilla-central (which typically happens once a day). We have to wait for a green PGO run (which doesn't happen on every push), which when combined with discovering bustage/backing that out/waiting for green again (PGO runs take up to 4-5 hours), can mean merges are often delayed.

When this merges the mozilla-central changeset will be posted in the bug, and the bug marked fixed :-)
Comment 10 Eric Chou [:ericchou] [:echou] 2012-08-23 03:41:34 PDT
Thank you, Ed. My bad. I did understand how this worked. I asked this question is because this morning I found there's a patch already in m-c, was submitted later than mine. However, I just checked again and couldn't find that patch anymore. I must be so tired. :P

Thanks again. :)
Comment 11 Ed Morley [:emorley] 2012-08-23 03:51:43 PDT
https://hg.mozilla.org/mozilla-central/rev/e8e54b17e6f2

Note You need to log in before you can comment on or make changes to this bug.