Closed Bug 853715 Opened 11 years ago Closed 11 years ago

Support WAP-Push-SI and WAP-Push-SL

Categories

(Core :: DOM: Device Interfaces, defect)

Product:
Core
Component:
DOM: Device Interfaces
Platform:
All
Gonk (Firefox OS)
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla24
Project Flags:
blocking-b2g koi+
Tracking Flags:
Tracking Status
firefox26 --- fixed

People

(Reporter: chucklee, Assigned: chucklee)

References

Details

(Whiteboard: [fixed-in-birch])

Attachments

(6 files, 22 obsolete files)

0003. Add PDU helper for decoding SI messages. V1.2
6.46 KB, patch
Details | Diff | Splinter Review
0004. Add PDU helper for decoding SL messages. V1.2
4.58 KB, patch
Details | Diff | Splinter Review
0006. Xpcshell test cases for SI and SL PDU helper. V1.3
10.21 KB, patch
Details | Diff | Splinter Review
0002. Add PDU helper for decoding messages in WBXML format. V1.5
14.09 KB, patch
Details | Diff | Splinter Review
0001. Create DOM skeletion for WAP Push. V1.4
7.11 KB, patch
Details | Diff | Splinter Review
0005. Handle incoming WAP Push messages. V2.3
5.93 KB, patch
Details | Diff | Splinter Review 
Handle WAP Push System Indication and System Loading on receiving.
Create new dom directory and basic files for WAP Push DOM.
Attachment #728068 - Flags: review?(vyang)
Add WBXML PDU helper to provide common decoding routine.

Application-wise definition, with respect to the Public ID, of tag and attribute token must be provided to WBXML PDU helper, or the decoder can't work correctly.
Attachment #728069 - Flags: review?(vyang)
PDU helper for System Indication, based on WBXML PDU helper.
Attachment #728071 - Flags: review?(vyang)
PDU helper for System Loading, based on WBXML PDU helper.
Attachment #728072 - Flags: review?(vyang)
Service handling all WAP Push messages.
Attachment #728074 - Flags: review?(vyang)
Treat incoming SMS with application id "x-wap-application:wml.ua" as WAP Push, and handled by WAP Push service.
Attachment #728075 - Flags: review?(vyang)
Test SI/SL pdu helper with message in plain text/WBXML format
Attachment #728088 - Flags: review?(vyang)
Test command for emulator(Enable debug on WapPushService.js to show result on logcat)

1. SI with href and date
sms pdu 07911326040000F0400B911346610089F6000420806291731408580605040B8423F0010603AEAF8202056A0045C60D0378797A008503656D61696C2F3132332F6162632E776D6C000AC3071999062515231510C304199906300103596F7520686176652034206E657720656D61696C73000101
2. SL with href
sms pdu 07911326040000F0400B911346610089F60004208062917314081E0605040B8423F0010603B0AF8203066A00850A036F7265696C6C79008501
3. Empty SL
sms pdu 07911326040000F0400B911346610089F6000420806291731408120605040B8423F0010603B0AF8203066A0005
4. SI in plain text
sms pdu 07911326040000F0400B911346610089F6000420806291731408750605040B8423F0010603ADAF823C3F786D6C2076657273696F6E3D27312E30273F3E0A3C73693E3C696E6469636174696F6E20687265663D27687474703A2F2F7777772E6F7265696C6C792E636F6D273E436865636B207468697320776562736974653C2F696E6469636174696F6E3E3C2F73693E
We still need input from product here, in order to determine if the proposed solution will meet the partner requirements.

Without that we won't know if we need a new API and if so what it would look like, or if we can add a new backend for the SimplePush API, or if we can simply use the SimplePush API.
Flags: needinfo?(pdolanjski)
Hi,

I need some information, in B2g Gaia, how I can receive wap push message.

Like in Android 
<action android:name="android.provider.Telephony.WAP_PUSH_RECEIVED" />
  <data android:mimeType="application/vnd.syncml.notification"/>
Similar in b2g What I have to do here to process OMADM Server notification.

Regards
Laxman
(In reply to Laxman from comment #10)
> Hi,
> 
> I need some information, in B2g Gaia, how I can receive wap push message.
> 
> Like in Android 
> <action android:name="android.provider.Telephony.WAP_PUSH_RECEIVED" />
>   <data android:mimeType="application/vnd.syncml.notification"/>
> Similar in b2g What I have to do here to process OMADM Server notification.
> 
> Regards
> Laxman

There is not platform support for receiving those messages on Gaia yet. BTW, are you trying to implement OMA DM specs?
Hi Peter, we do need some details about what do we actually need for SI and SL.  Could you help providing related information or introduce us to correct partners?
Rebase.
Attachment #728068 - Attachment is obsolete: true
Attachment #728068 - Flags: review?(vyang)
Attachment #751523 - Flags: review?(vyang)
Add support for string table
Attachment #728069 - Attachment is obsolete: true
Attachment #728069 - Flags: review?(vyang)
Attachment #751524 - Flags: review?(vyang)
Add test for string table
Attachment #728088 - Attachment is obsolete: true
Attachment #728088 - Flags: review?(vyang)
Attachment #751525 - Flags: review?(vyang)
Comment on attachment 751523 [details] [diff] [review]
0001. Create DOM skeletion for WAP Push. V1.1

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

::: dom/wappush/Makefile.in
@@ +12,5 @@
> +include $(DEPTH)/config/autoconf.mk
> +
> +ifdef MOZ_B2G_RIL
> +ifdef ENABLE_TESTS
> +XPCSHELL_TESTS = tests

shall be moved into moz.build, use XPCSHELL_TESTS_MANIFESTS instead.  See https://mxr.mozilla.org/mozilla-central/source/dom/mms/moz.build
Attachment #751523 - Flags: review?(vyang) → review+
Comment on attachment 751524 [details] [diff] [review]
0002. Add PDU helper for decoding messages in WBXML format. V1.1

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

::: dom/wappush/src/Makefile.in
@@ +15,5 @@
>  EXTRA_COMPONENTS = \
>    $(NULL)
>  
>  EXTRA_JS_MODULES = \
> +  ril/WbxmlPduHelper.jsm \

Please rename it to 'gonk'.  We abused the term 'ril' and it should really be 'gonk' to represent the platform that features WAP Push.

::: dom/wappush/src/ril/WbxmlPduHelper.jsm
@@ +10,5 @@
> +Cu.import("resource://gre/modules/WspPduHelper.jsm", WSP);
> +
> +let DEBUG = false; // set to true to see debug messages
> +
> +const TAG_TOKEN_ATTR_MASK     = 0x80;

Please document the origin of these magic numbers.

@@ +24,5 @@
> +this.tagTokenList = null;
> +this.attrTokenList = null;
> +this.globalTokenOverrideList = null;
> +this.charset = "";
> +this.stringTable = null;

Please avoid these global variables as possible.  They don't seem to me some global settings that apply to further parsing transactions.

@@ +77,5 @@
> + *
> + */
> +this.WbxmlInlineString = {
> +  decode: function decode_wbxml_inline_string(data) {
> +    let startOffset = data.offset, endOffset = data.offset;

nit: line break here will be a little bit more clear.

  let startOffset = data.offset,
      endOffset = data.offset;

@@ +84,5 @@
> +      charCode = WSP.Octet.decode(data);
> +      endOffset++;
> +    }
> +    data.offset = startOffset;
> +    let stringData = WSP.Octet.decodeMultiple(data, endOffset);

nit: can we simply push decoded |charCode| into an array, and skip decodeMultiple(), but call decodeStringContent() directly?

@@ +101,5 @@
> + *
> + */
> +this.WbxmlOpaque = {
> +  decode: function decode_wbxml_inline_opaque(data) {
> +    // Default behavior : skip

Please throw an exception here instead.  We don't want to support WAP Push other than SI and SL now, so in theory we should not ever have OPAQUE data here.  If we do, an explicit exception will send a more clear signal than have a dummy implementation here.

@@ +253,5 @@
> +    stringTable = msg.header.stringTable;
> +
> +    // Only parse messages of currect public ID.
> +    if (msg.header.publicId != appToken.publicId) {
> +      return null;

Please bail out early, i.e. move upward right after |msg.header.publicId| is decoded.

@@ +305,5 @@
> +  return names;
> +})();
> +
> +let debug;
> +if (DEBUG) {

I see no debug lines come in this patch.  Besides, since we'd like to wrap all debug lines with |if (DEBUG)| instead, we don't really need two debug function implementations.
Attachment #751524 - Flags: review?(vyang)
Comment on attachment 728071 [details] [diff] [review]
0003. Add PDU helper for decoding SI messages.

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

::: dom/wappush/src/ril/SiPduHelper.jsm
@@ +10,5 @@
> +Cu.import("resource://gre/modules/WspPduHelper.jsm", WSP);
> +let WBXML = {};
> +Cu.import("resource://gre/modules/WbxmlPduHelper.jsm", WBXML);
> +
> +let DEBUG = false; // set to true to see debug messages

We'd better have some universal debug setting for all WAP Push source entities.
Attachment #728071 - Flags: review?(vyang) → review+
Comment on attachment 728072 [details] [diff] [review]
0004. Add PDU helper for decoding SL messages.

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

::: dom/wappush/src/ril/SlPduHelper.jsm
@@ +12,5 @@
> +Cu.import("resource://gre/modules/WbxmlPduHelper.jsm", WBXML);
> +
> +let DEBUG = false; // set to true to see debug messages
> +
> +const PUBLIC_IDENTIFIER_SL    = 0x06;

Please document the origin of these magic numbers.
Attachment #728072 - Flags: review?(vyang) → review+
Comment on attachment 728074 [details] [diff] [review]
0005. Add service handling WAP Push messages.

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

We don't really need a service here and we have already a WapPushManager in dom/mms.  Please 1) move WapPushManager into dom/wappush, 2) add necessary code into WapPushManager.js only.
Attachment #728074 - Flags: review?(vyang)
Comment on attachment 728075 [details] [diff] [review]
0006. Handle incoming WAP Push messages.

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

See comment 20.
Attachment #728075 - Flags: review?(vyang)
Address comment 20 and comment 21
Attachment #728074 - Attachment is obsolete: true
Attachment #728075 - Attachment is obsolete: true
Attachment #753627 - Flags: review?(vyang)
Attachment #751525 - Attachment is obsolete: true
Attachment #751525 - Flags: review?(vyang)
Replace global variables with an local object
Attachment #753623 - Attachment is obsolete: true
Attachment #753623 - Flags: review?(vyang)
Attachment #753691 - Flags: review?(vyang)
Comment on attachment 753627 [details] [diff] [review]
0005. Handle incoming WAP Push messages. V2

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

We'll need another round for system message emitting. Thank you :)

::: dom/wappush/src/gonk/WapPushManager.js
@@ +20,5 @@
> +  let SI = {};
> +  Cu.import("resource://gre/modules/SiPduHelper.jsm", SI);
> +  return SI;
> +});
> + 

nit: trailing ws.

@@ +59,5 @@
> +    if (appid == "x-wap-application:mms.ua") {
> +      let mmsService = Cc["@mozilla.org/mms/rilmmsservice;1"]
> +                       .getService(Ci.nsIMmsService);
> +      mmsService.QueryInterface(Ci.nsIWapPushApplication)
> +                .receiveWapPush(data.array, data.array.length, data.offset, options);

Let's bail out early and put a return statement here. Shall we?

@@ +61,5 @@
> +                       .getService(Ci.nsIMmsService);
> +      mmsService.QueryInterface(Ci.nsIWapPushApplication)
> +                .receiveWapPush(data.array, data.array.length, data.offset, options);
> +    } else if (appid === "x-wap-application:wml.ua") {
> +      let contentType = options.headers["content-type"].media;

Then we don't need "else" here.

@@ +86,5 @@
> +      } else if (contentType === "text/vnd.wap.sl" || contentType === "application/vnd.wap.slc") {
> +        SL.PduHelper.parse(data, contentType, msg);
> +      }
> +
> +      // TODO: Send system message

Whenever you place a TODO in your source code, it must be followed by a bug number.  Besides, bug 854930 was marked as WONTFIX, so we definitely have to do something here or reopen bug 854930.
Attachment #753627 - Flags: review?(vyang)
Comment on attachment 753624 [details] [diff] [review]
0003. Add PDU helper for decoding SI messages. V1.1

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

::: dom/wappush/src/gonk/SiPduHelper.jsm
@@ +85,5 @@
> +     * @see WAP-192-WBXML-20010725-A
> +     */
> +    if (!contentType || contentType === "application/vnd.wap.sic") {
> +      let globalTokenOverride = {};
> +      globalTokenOverride[0xC3] = {coder: OpaqueAsDate};

Should be:

  globalTokenOverride[0xC3] = {
    number: 0xC3,
    coder: OpaqueAsDate
  };

as those defined in WBXML_GLOBAL_TOKENS.
Comment on attachment 753691 [details] [diff] [review]
0002. Add PDU helper for decoding messages in WBXML format. V1.3

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

::: dom/wappush/src/gonk/WbxmlPduHelper.jsm
@@ +256,5 @@
> +    }
> +
> +    /**
> +     * Read WBXML header.
> +     */

Please comment document origin "WAP-192-WBXML-20010725-a section 5.3. BNF for Document Structure" or the one you have.

@@ +259,5 @@
> +     * Read WBXML header.
> +     */
> +    msg.header = {};
> +    msg.header.version = WSP.Octet.decode(data);
> +    msg.header.publicId = WSP.UintVar.decode(data);

According to the WBXML spec, we have "publicid = mb_u_int32 | (zero index)".  Maybe we should make pubicId a string for better compatibility?

@@ +266,5 @@
> +    if (msg.header.publicId != appToken.publicId) {
> +      return null;
> +    }
> +
> +    msg.header.charset = WSP.WSP_WELL_KNOWN_CHARSETS[WSP.UintVar.decode(data)].converter;

That's a long long line :(
Besides, having `charset` pointed to a converter here is really confusing.  The `charset` field is only used in `decodeInfo` in parseWbxml().  I'd rather have:

  let charsetCode = WSP.UintVar.decode(data);
  msg.header.charset = WSP.WSP_WELL_KNOWN_CHARSETS[charsetCode].name;

`charsetCode` could also be zero, and |WSP.WSP_WELL_KNOWN_CHARSETS[charsetCode]| could also be undefined in our implementation.  We'd need better error handling of these things.

@@ +270,5 @@
> +    msg.header.charset = WSP.WSP_WELL_KNOWN_CHARSETS[WSP.UintVar.decode(data)].converter;
> +    let stringTableLen = WSP.UintVar.decode(data);
> +    msg.header.stringTable = [];
> +    for (let i = 0; i < stringTableLen; i++) {
> +      msg.header.stringTable.push(WSP.Octet.decode(data));

msg.header.stringTable =
  WSP.Octet.decodeMultiple(data, data.offset + stringTableLen);
Attachment #753691 - Flags: review?(vyang)
Comment on attachment 753628 [details] [diff] [review]
0006. Xpcshell test cases for SI and SL PDU helper. V1.2

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

Nice! We all love test cases!
Attachment #753628 - Flags: review?(vyang) → review+
Whiteboard: RN5/29
Address comment 31, including support public id in string form.
Attachment #753691 - Attachment is obsolete: true
Attachment #755796 - Flags: review?(vyang)
Address comment 30, and change public ID from binary form to string form.
Attachment #753624 - Attachment is obsolete: true
Change public id from binary form to string form.
Attachment #753625 - Attachment is obsolete: true
Address comment 29.
Attachment #753627 - Attachment is obsolete: true
Attachment #755799 - Flags: review?(vyang)
Whiteboard: RN5/29 → RN6/7
Comment on attachment 755796 [details] [diff] [review]
0002. Add PDU helper for decoding messages in WBXML format. V1.4

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

::: dom/wappush/src/gonk/WbxmlPduHelper.jsm
@@ +279,5 @@
> +    msg.charset = WSP.WSP_WELL_KNOWN_CHARSETS[headerRaw.charset];
> +    if (!msg.charset) {
> +      throw new Error("Charset is not supported.");
> +    }
> +    msg.charset = msg.charset.name;

Please don't do such reassignments.  Keep the meaning of a variable throughout your code.

  let entry = ...
  if (!entry) { throw ... }
  msg.charset = entry.name;
Attachment #755796 - Flags: review?(vyang) → review+
Comment on attachment 755799 [details] [diff] [review]
0005. Handle incoming WAP Push messages. V2.1

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

There are two WapPushManager instances.
Attachment #755799 - Flags: review?(vyang) → review-
Comment on attachment 757752 [details] [diff] [review]
0005. Handle incoming WAP Push messages. V2.2

obsolete due to build error
Attachment #757752 - Attachment is obsolete: true
Attachment #757752 - Flags: review?(vyang)
Fix build error caused by patch 0005
Attachment #755794 - Attachment is obsolete: true
Comment on attachment 757752 [details] [diff] [review]
0005. Handle incoming WAP Push messages. V2.2

Build error is fixed in patch 0001, this one does no wrong.
Attachment #757752 - Attachment is obsolete: false
Attachment #757752 - Flags: review?(vyang)
Comment on attachment 757752 [details] [diff] [review]
0005. Handle incoming WAP Push messages. V2.2

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

Nice! Would you also help re-do this patch with `hg mv`? That would slim this patch down even more.
Attachment #757752 - Flags: review?(vyang) → review+
Blocks: 891762
Blocks: 838062
Blocks: 891248
blocking-b2g: --- → koi+
Flags: needinfo?(pdolanjski)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: