842235 - B2G MMS: retry send on error

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Comment 1

[Joe Cheng [:jcheng] (please needinfo)]

leo+, this bug is needed to fulfill MMS user stories for v1.1

Comment 2

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Attachment: WIP-Retry send on error

Comment 3

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Attachment: Retry Send MMS

Comment 4

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

Comment on attachment 722665 [details] [diff] [review]
Retry Send MMS

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

::: dom/mms/src/ril/MmsService.js
@@ +687,5 @@
>        return;
>      }
>  
> +    this.retryCount = 0;
> +    let that = this;

You don't need this.

@@ +696,5 @@
> +        that.retryCount++;
> +        let timer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
> +        timer.initWithCallback((function (){
> +                                 this.send(retryCallback);
> +                               }).bind(that),

let retryCallback = (function (mmsStatus, msg) {
  if ((MMS.MMS_PDU_ERROR_TRANSIENT_FAILURE == mmsStatus ||
        MMS.MMS_PDU_ERROR_PERMANENT_FAILURE == mmsStatus) &&
      this.retryCount < MAX_RETRY_COUNT) {
    this.retryCount++;

    let timer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
    timer.initWithCallback(this.send.bind(this, retryCallback),
                           DELAY_TIME_TO_RETRY,
                           Ci.nsITimer.TYPE_ONE_SHOT);
    return;
  }

  callbackIfValid(mmsStatus, msg);
}).bind(this);

this.send(retryCallback);

@@ +702,5 @@
> +                               Ci.nsITimer.TYPE_ONE_SHOT);
> +        return;
> +      }
> +      if (callback) {
> +        callback(mmsStatus, msg);

callbackIfValid instead.

@@ +713,5 @@
> +   *        A callback function that takes two arguments: one for
> +   *        X-Mms-Response-Status, the other for the parsed M-Send.conf message.
> +   */
> +  send: function send(callback) {
> +    let callbackIfValid = function callbackIfValid(mmsStatus, msg) {

You actually pass |retryCallback| to |send|, so the |callback| here is always valid.

Comment 5

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Attachment: Retry Send MMS

Comment 6

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

Comment on attachment 722707 [details] [diff] [review]
Retry Send MMS

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

::: dom/mms/src/ril/MmsService.js
@@ +702,5 @@
> +      }
> +
> +      callbackIfValid(mmsStatus, msg);
> +    }).bind(this);
> +  },

Have you tried it? You missed send() here.

Comment 7

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Attachment: Retry Send MMS

Comment 8

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #6)
> Comment on attachment 722707 [details] [diff] [review]
> Retry Send MMS
> 
> Review of attachment 722707 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/mms/src/ril/MmsService.js
> @@ +702,5 @@
> > +      }
> > +
> > +      callbackIfValid(mmsStatus, msg);
> > +    }).bind(this);
> > +  },
> 
> Have you tried it? You missed send() here.

My bad...orz

Comment 9

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Attachment: Retry Send MMS

Comment 10

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Attachment: Retry Send MMS

Comment 11

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

Comment on attachment 723326 [details] [diff] [review]
Retry Send MMS

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

Thank you :)

Comment 12

[Joe Cheng [:jcheng] (please needinfo)]

expect to land 3/11

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d46a482ab8c0

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/d46a482ab8c0

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/3090ffe00713