Last Comment Bug 775038 - B2G MMS: various small defects in WSP/MMS PDU parsers
: B2G MMS: various small defects in WSP/MMS PDU parsers
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM: Device Interfaces (show other bugs)
: unspecified
: ARM Gonk (Firefox OS)
: -- normal (vote)
: mozilla17
Assigned To: Vicamo Yang [:vicamo][:vyang]
:
Mentors:
Depends on:
Blocks: 761057
  Show dependency treegraph
 
Reported: 2012-07-18 04:07 PDT by Vicamo Yang [:vicamo][:vyang]
Modified: 2012-07-23 06:40 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Part 1: fix minor defects (26.18 KB, patch)
2012-07-18 04:54 PDT, Vicamo Yang [:vicamo][:vyang]
philipp: review+
Details | Diff | Review
Part 2: Refactor X-Mms-Retrieve-Status decoding (6.38 KB, patch)
2012-07-18 04:54 PDT, Vicamo Yang [:vicamo][:vyang]
philipp: review+
Details | Diff | Review
Part 3: fix type parameter decode (5.69 KB, patch)
2012-07-18 04:55 PDT, Vicamo Yang [:vicamo][:vyang]
philipp: review+
Details | Diff | Review
Part 1: fix minor defects : V2 (27.70 KB, patch)
2012-07-20 05:32 PDT, Vicamo Yang [:vicamo][:vyang]
philipp: review+
Details | Diff | Review
Part 2: refactor ContentLocationValue test cases (4.05 KB, patch)
2012-07-20 05:34 PDT, Vicamo Yang [:vicamo][:vyang]
philipp: review+
Details | Diff | Review
Part 3: Refactor X-Mms-Retrieve-Status decoding : V2 (5.66 KB, patch)
2012-07-20 05:35 PDT, Vicamo Yang [:vicamo][:vyang]
philipp: review+
Details | Diff | Review
Part 4: fix type parameter decode : V2 (5.70 KB, patch)
2012-07-20 05:38 PDT, Vicamo Yang [:vicamo][:vyang]
no flags Details | Diff | Review

Description Vicamo Yang [:vicamo][:vyang] 2012-07-18 04:07:18 PDT
There are a few bugs/defects in WSP/MMS PDU decoder that have to be fixed/refactored before landing encoder code.
Comment 1 Vicamo Yang [:vicamo][:vyang] 2012-07-18 04:54:07 PDT
Created attachment 643319 [details] [diff] [review]
Part 1: fix minor defects
Comment 2 Vicamo Yang [:vicamo][:vyang] 2012-07-18 04:54:35 PDT
Created attachment 643320 [details] [diff] [review]
Part 2: Refactor X-Mms-Retrieve-Status decoding
Comment 3 Vicamo Yang [:vicamo][:vyang] 2012-07-18 04:55:03 PDT
Created attachment 643321 [details] [diff] [review]
Part 3: fix type parameter decode
Comment 4 Philipp von Weitershausen [:philikon] 2012-07-19 13:28:01 PDT
Comment on attachment 643319 [details] [diff] [review]
Part 1: fix minor defects

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

::: dom/mms/src/ril/MmsPduHelper.jsm
@@ +1026,5 @@
>    },
>  
>    /**
>     * Check existences of all mandatory fields of a MMS message. Also sets `type`
> +   * for convient access.

"convenient"?
Comment 5 Philipp von Weitershausen [:philikon] 2012-07-19 13:30:07 PDT
Comment on attachment 643320 [details] [diff] [review]
Part 2: Refactor X-Mms-Retrieve-Status decoding

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

::: dom/mms/tests/test_mms_pdu_helper.js
@@ +132,5 @@
>                    null, "NotWellKnownEncodingError");
>    // Test for normal header
>    wsp_encode_test(MMS.MmsHeader, {name: "X-Mms-Message-Type",
> +                                  value: /*MMS.MMS_PDU_TYPE_SEND_REQ*/128},
> +                  [0x80 | 0x0C, /*MMS.MMS_PDU_TYPE_SEND_REQ*/128]);

Why not use the consts here?
Comment 6 Vicamo Yang [:vicamo][:vyang] 2012-07-20 05:32:39 PDT
Created attachment 644264 [details] [diff] [review]
Part 1: fix minor defects : V2

1. address review comment #4
2. "Date" is also a mandatory field for M-Retrieve.req PDU.
Comment 7 Vicamo Yang [:vicamo][:vyang] 2012-07-20 05:34:31 PDT
Created attachment 644265 [details] [diff] [review]
Part 2: refactor ContentLocationValue test cases

fix review comment #5 & clean up
Comment 8 Vicamo Yang [:vicamo][:vyang] 2012-07-20 05:35:52 PDT
Created attachment 644266 [details] [diff] [review]
Part 3: Refactor X-Mms-Retrieve-Status decoding : V2

fix review comment #5
Comment 9 Vicamo Yang [:vicamo][:vyang] 2012-07-20 05:38:14 PDT
Created attachment 644267 [details] [diff] [review]
Part 4: fix type parameter decode : V2

Update r=philikon in summary only. Thanks for your review :)
Comment 10 Philipp von Weitershausen [:philikon] 2012-07-20 12:36:21 PDT
Comment on attachment 644264 [details] [diff] [review]
Part 1: fix minor defects : V2

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

I already r+'ed, no need to ask for review again :)

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