Chia-hung Tai [:ctai :ctai_mozilla :cht] Assignee
	Description • 11 years ago

Looks like something on the retrying send MMS.

	Chia-hung Tai [:ctai :ctai_mozilla :cht] Assignee
	Comment 1 • 11 years ago

Block an leo+ bug.

	Chia-hung Tai [:ctai :ctai_mozilla :cht] Assignee
	Comment 6 • 11 years ago

Root cause:
Users of instances of nsITimer should keep a reference to the timer until it is no longer needed in order to assure the timer is fired.

https://developer.mozilla.org/en-US/docs/XPCOM_Interface_Reference/nsITimer

	Vicamo Yang [:vicamo][:vyang]
	Comment 7 • 11 years ago

Comment on attachment 755860 [details] [diff] [review]
Patch v1.3

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

::: dom/mobilemessage/src/ril/MmsService.js
@@ +625,5 @@
>  function RetrieveTransaction(contentLocation) {
>    this.contentLocation = contentLocation;
> +
> +  // We need to keep a reference to the timer to assure the timer is fired.
> +  this.timer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);

Please only create a timer when necessary.

@@ +748,5 @@
>      msg.headers["content-type"] = contentType;
>    }
>  
> +  // We need to keep a reference to the timer to assure the timer is fired.
> +  this.timer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);

ditto

::: modules/libpref/src/init/all.js
@@ +4207,5 @@
>  // never: Never retrieval mode.
>  pref("dom.mms.retrieval_mode", "manual");
>  
>  pref("dom.mms.sendRetryCount", 3);
> +pref("dom.mms.sendRetryInterval", 180000);

Any reason to shorten the retry interval here?

	Vicamo Yang [:vicamo][:vyang]
	Comment 9 • 11 years ago

Comment on attachment 756467 [details] [diff] [review]
Patch v1.4

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

::: dom/mobilemessage/src/ril/MmsService.js
@@ +625,5 @@
>  function RetrieveTransaction(contentLocation) {
>    this.contentLocation = contentLocation;
> +
> +  // We need to keep a reference to the timer to assure the timer is fired.
> +  this.timer = null;

nit: you can simply move this line into |RetrieveTransaction.prototype|.

@@ +639,5 @@
>      let that = this;
>      this.retrieve((function retryCallback(mmsStatus, msg) {
>        if (MMS.MMS_PDU_STATUS_DEFERRED == mmsStatus &&
>            that.retryCount < PREF_RETRIEVAL_RETRY_COUNT) {
> +        if (that.retryCount === 0) {

nit: please check nullity of |that.timer| instead.  We don't need an extra pre-condition "timer is null when retryCount is zero" even it's true.

@@ +752,5 @@
>      msg.headers["content-type"] = contentType;
>    }
>  
> +  // We need to keep a reference to the timer to assure the timer is fired.
> +  this.timer = null;

ditto

@@ +836,5 @@
>      let retryCallback = (function (mmsStatus, msg) {
>        if ((MMS.MMS_PDU_ERROR_TRANSIENT_FAILURE == mmsStatus ||
>              MMS.MMS_PDU_ERROR_PERMANENT_FAILURE == mmsStatus) &&
>            this.retryCount < PREF_SEND_RETRY_COUNT) {
> +        if (this.retryCount === 0) {

ditto.

	Chia-hung Tai [:ctai :ctai_mozilla :cht] Assignee
	Comment 10 • 11 years ago

Can you mark this bug as leo+?
This bug will cause the sending icon spin forever.
Thanks.

	Ryan VanderMeulen [:RyanVM]
	Comment 12 • 11 years ago

https://hg.mozilla.org/projects/birch/rev/f8a0aa9350c9

	Ryan VanderMeulen [:RyanVM]
	Comment 13 • 11 years ago

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

	Ryan VanderMeulen [:RyanVM]
	Comment 14 • 11 years ago

https://hg.mozilla.org/releases/mozilla-b2g18/rev/9436c8582f92

	Ryan VanderMeulen [:RyanVM]
	Comment 15 • 11 years ago

https://hg.mozilla.org/releases/mozilla-b2g18_v1_1_0_hd/rev/9436c8582f92