775038 - B2G MMS: various small defects in WSP/MMS PDU parsers

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[Vicamo Yang [:vicamo][:vyang]]

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]]

Attachment: Part 1: fix minor defects

Comment 2

[Vicamo Yang [:vicamo][:vyang]]

Attachment: Part 2: Refactor X-Mms-Retrieve-Status decoding

Comment 3

[Vicamo Yang [:vicamo][:vyang]]

Attachment: Part 3: fix type parameter decode

Comment 4

[Philipp von Weitershausen [:philikon]]

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]]

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]]

Attachment: 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]]

Attachment: Part 2: refactor ContentLocationValue test cases

fix review comment #5 & clean up

Comment 8

[Vicamo Yang [:vicamo][:vyang]]

Attachment: Part 3: Refactor X-Mms-Retrieve-Status decoding : V2

fix review comment #5

Comment 9

[Vicamo Yang [:vicamo][:vyang]]

Attachment: Part 4: fix type parameter decode : V2

Update r=philikon in summary only. Thanks for your review :)

Comment 10

[Philipp von Weitershausen [:philikon]]

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 :)

Comment 11

[Vicamo Yang [:vicamo][:vyang]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/ebc12b80a45f
http://hg.mozilla.org/integration/mozilla-inbound/rev/0d9e41c02507
http://hg.mozilla.org/integration/mozilla-inbound/rev/5d9f7e30f66a
http://hg.mozilla.org/integration/mozilla-inbound/rev/be49df587b6b

Comment 12

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/ebc12b80a45f
https://hg.mozilla.org/mozilla-central/rev/0d9e41c02507
https://hg.mozilla.org/mozilla-central/rev/5d9f7e30f66a
https://hg.mozilla.org/mozilla-central/rev/be49df587b6b