761057 - B2G MMS: support sending M-Send.req PDUs

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

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

This is the protocol part of MMS outgoing messages support. It should open an API that accepts a pre-contructed message object, encode it into a M-Send.req PDU, send it out and process returning M-Send.conf message as well.

Comment 1

[JP Rosevear [:jpr]]

MMS is desired to be ready for basecamp, but does not actually block.

Comment 2

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

Attachment: Part 1: Add required encoders for M-Send.req/M-Send.conf PDU : WIP

Comment 3

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

Attachment: Part 2: send M-Send.req : WIP

Comment 4

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

Attachment: [Debug] DO NOT SUBMIT

Comment 5

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

Attachment: Part 1: disable some v1.4 parameters for compatibility

Comment 6

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

Attachment: Part 2: Add required encoders for M-Send.req/M-Send.conf PDU

1. support email addressing model, see bug 761069
2. refactor MmsPduHelper.encodeHeader* and add test cases
3. decoded msg.content is an array, so refactor encoder to accept the same type
4. do not use magic pdu type numbers in test script

Comment 7

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

Attachment: Part 3: send M-Send.req

use msg.content rather than msg.contentStream

Comment 8

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

Attachment: [Debug] DO NOT SUBMIT : V2

updates for changes made in patch 2, 3

Comment 9

[Philipp von Weitershausen [:philikon]]

Comment on attachment 645715 [details] [diff] [review]
Part 2: Add required encoders for M-Send.req/M-Send.conf PDU

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

::: dom/mms/src/ril/MmsPduHelper.jsm
@@ +115,5 @@
> +      throw new WSP.CodeError("Address: invalid value");
> +    }
> +
> +
> +    let [type, addr] = [value.type, value.address];

Sadly I think this creates at least one array object which seems a bit excessive just to save one line of code.

@@ +117,5 @@
> +
> +
> +    let [type, addr] = [value.type, value.address];
> +    let str;
> +    if (type == "email") {

It seems like a `switch` statement would fit better here.

@@ +122,5 @@
> +      if (addr.indexOf("@") > 0) {
> +        str = addr;
> +      }
> +    } else if (type == "num") {
> +      if (addr.match(/^[\+*#]\d+$/)) {

Please define these RegExps globally. You can `const` them or make them lazy getters on the global scope, e.g.:

  function defineLazyRegExp(obj, name, pattern) {
    obj.__defineGetter__(name, function() {
      delete obj[name];
      return obj[name] = new RegExp(pattern);
    });
  }

  defineLazyRegExp(this, "MMS_ADDR_TYPE_NUM", "^[\+*#]\d+$");
  defineLazyRegExp(this, "MMS_ADDR_TYPE_ALPHANUM", "^\w+$");
  etc.

Comment 10

[Philipp von Weitershausen [:philikon]]

Comment on attachment 645716 [details] [diff] [review]
Part 3: send M-Send.req

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

::: dom/mms/src/ril/MmsService.js
@@ +234,5 @@
> +    msg.headers["x-mms-message-type"] = MMS.MMS_PDU_TYPE_SEND_REQ;
> +    if (!msg.headers["x-mms-transaction-id"]) {
> +      // Create an unique transaction id
> +      let uuidgen = Cc["@mozilla.org/uuid-generator;1"]
> +                    .getService(Ci.nsIUUIDGenerator);

Please use a lazy getter for this in the top scope:

  XPCOMUtils.defineLazyServiceGetter(this, gUUIDGenerator, "@mozilla.org/uuid-generator;1", "nsIUUIDGenerator");

Comment 11

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

Attachment: Part 1: disable some v1.4 parameters for compatibility : V2

Update r=philikon only. Since these patches were previously reviewed+, won't set review flag here.

Comment 12

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

Attachment: Part 2: Add required encoders for M-Send.req/M-Send.conf PDU : V2

Accommodate to comment #9. Thanks Philipp!

Comment 13

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

Attachment: Part 3: send M-Send.req : V2

Accommodate to comment #10. Thanks Philipp!

Comment 14

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

Attachment: [Debug] DO NOT SUBMIT : V3

add text/plain for easier debug

Comment 15

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

http://hg.mozilla.org/integration/mozilla-inbound/rev/83254a09a38f
http://hg.mozilla.org/integration/mozilla-inbound/rev/7001e01f26ec
http://hg.mozilla.org/integration/mozilla-inbound/rev/13a837f5cba4

Comment 16

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/83254a09a38f
https://hg.mozilla.org/mozilla-central/rev/7001e01f26ec
https://hg.mozilla.org/mozilla-central/rev/13a837f5cba4