917163 - [MMS] MMS Data call is not released immediately

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[Leo]

In MMS service, when the mms data connection is no longer required, we are not immediately releasing the connection.
We have a 30s delay for actually release the data call.

const TIME_TO_RELEASE_MMS_CONNECTION = 30000;

Because of this, the PDP connection is not released and power consumption higher than normal.

Comment 1

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

Attachment: bug-917163.patch v1.0

Comment 2

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

Attachment: bug-917163.patch v1.1

Comment 3

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

Comment on attachment 805904 [details] [diff] [review]
bug-917163.patch v1.1

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

::: dom/mobilemessage/src/gonk/MmsService.js
@@ +59,5 @@
>  const CONFIG_SEND_REPORT_DEFAULT_YES = 2;
>  const CONFIG_SEND_REPORT_ALWAYS      = 3;
>  
> +const PREF_TIME_TO_BUFFER_MMS_REQUESTS =
> +  Services.prefs.getIntPref("dom.networkManager.timeToBufferMMSRequest");

At the time we talked about this, I was thinking we'll move connection management to NetworkManager.  But should we move MMS requests management to NetworkManager as well?  Probably not.  MmsService should still hold all MMS requests.  The timeout for MMS requests here is fine.  Let's keep the way it is now.

@@ +61,5 @@
>  
> +const PREF_TIME_TO_BUFFER_MMS_REQUESTS =
> +  Services.prefs.getIntPref("dom.networkManager.timeToBufferMMSRequest");
> +const PREF_TIME_TO_RELEASE_MMS_CONNECTION =
> +  Services.prefs.getIntPref("dom.networkManager.timeToReleaseMMSConnection");

Please use "network.gonk.ms-release-mms-connection" instead.  NetworkManager is not a DOM element and we have already a pref "network.gonk.manage-offline-status".

@@ +274,5 @@
>  
> +        if (PREF_TIME_TO_BUFFER_MMS_REQUESTS < 1000) {
> +          this.flushPendingCallbacks(_HTTP_STATUS_ACQUIRE_TIMEOUT);
> +          return true;
> +        }

So we don't need this.

@@ +298,5 @@
>      release: function release() {
>        this.refCount--;
>        if (this.refCount <= 0) {
>          this.refCount = 0;
> +        // The waiting is too small, just skip the timer creation.

Please insert an empty line before the comment line.

::: modules/libpref/src/init/all.js
@@ +4380,5 @@
>  pref("mms.debugging.enabled", false);
>  
> +// The waiting time in network manager.
> +pref("dom.networkManager.timeToBufferMMSRequest", 30000);
> +pref("dom.networkManager.timeToReleaseMMSConnection", 30000);

Please only define "network.gonk.ms-release-mms-connection" and define it in b2g/app/b2g.js.

Comment 4

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

Attachment: bug-917163.patch v1.2

Comment 5

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

Comment on attachment 806404 [details] [diff] [review]
bug-917163.patch v1.2

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

Thanks for your comments.

Comment 6

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

Leo,
Please overwrite "network.gonk.ms-release-mms-connection" to 0 in user.js if you want to release the data call immdeitaely.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/51237fb5f6e3

Comment 8

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/51237fb5f6e3