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

RESOLVED FIXED in mozilla17

Status

()

Core
DOM: Device Interfaces
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: vicamo, Assigned: vicamo)

Tracking

unspecified
mozilla17
ARM
Gonk (Firefox OS)
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments, 3 obsolete attachments)

(Assignee)

Description

5 years ago
There are a few bugs/defects in WSP/MMS PDU decoder that have to be fixed/refactored before landing encoder code.
(Assignee)

Comment 1

5 years ago
Created attachment 643319 [details] [diff] [review]
Part 1: fix minor defects
Attachment #643319 - Flags: review?(philipp)
(Assignee)

Comment 2

5 years ago
Created attachment 643320 [details] [diff] [review]
Part 2: Refactor X-Mms-Retrieve-Status decoding
Attachment #643320 - Flags: review?(philipp)
(Assignee)

Comment 3

5 years ago
Created attachment 643321 [details] [diff] [review]
Part 3: fix type parameter decode
Attachment #643321 - Flags: review?(philipp)
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"?
Attachment #643319 - Flags: review?(philipp) → review+
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?
Attachment #643320 - Flags: review?(philipp) → review+
Attachment #643321 - Flags: review?(philipp) → review+
(Assignee)

Comment 6

5 years ago
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.
Attachment #643319 - Attachment is obsolete: true
(Assignee)

Updated

5 years ago
Attachment #644264 - Flags: review?(philipp)
(Assignee)

Comment 7

5 years ago
Created attachment 644265 [details] [diff] [review]
Part 2: refactor ContentLocationValue test cases

fix review comment #5 & clean up
Attachment #644265 - Flags: review?(philipp)
(Assignee)

Comment 8

5 years ago
Created attachment 644266 [details] [diff] [review]
Part 3: Refactor X-Mms-Retrieve-Status decoding : V2

fix review comment #5
Attachment #643320 - Attachment is obsolete: true
Attachment #644266 - Flags: review?(philipp)
(Assignee)

Updated

5 years ago
Attachment #644266 - Attachment description: Part 2: Refactor X-Mms-Retrieve-Status decoding : V2 → Part 3: Refactor X-Mms-Retrieve-Status decoding : V2
(Assignee)

Comment 9

5 years ago
Created attachment 644267 [details] [diff] [review]
Part 4: fix type parameter decode : V2

Update r=philikon in summary only. Thanks for your review :)
Attachment #643321 - Attachment is obsolete: true
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 :)
Attachment #644264 - Flags: review?(philipp) → review+
Attachment #644265 - Flags: review?(philipp) → review+
Attachment #644266 - Flags: review?(philipp) → review+
(Assignee)

Comment 11

5 years ago
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
Target Milestone: --- → mozilla17
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
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.