908603 - B2G RIL: data connection not workable after flight mode on/off

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[Jessica Jong [:jessica] (inactive)]

Attachment: 20130823_data_unworkable_after_flightmode.log

Tests to reproduce:

1. Enable data and wait for 3G icon
2. Enable flight mode
3. Disable flight mode
4. Wait for 3G icon to re-appear

Expected results:
data workable after flight mode on/off

Actual results:
3G icon shows up but not able to browse the web

---

Looked into the case and found that after flight mode is disabled, we use the old ip and gateway to set the routes, which would fail:

08-23 16:02:06.961 D/RILC    (  147): [0117]< SETUP_DATA_CALL {[status=0,retry=-1,cid=0,up,IP,rmnet0,42.70.76.153/30,168.95.1.1 168.95.192.1,42.70.76.154]}

08-23 16:03:36.000 D/RILC    (  147): [0253]< SETUP_DATA_CALL {[status=0,retry=-1,cid=0,up,IP,rmnet0,100.74.241.84/30,168.95.1.1 168.95.192.1,100.74.241.85]}

08-23 16:03:36.000 I/Gecko   (  143): -*- RadioInterface[0]: Received message from worker: {"status":0,"suggestedRetryTime":-1,"cid":"0","active":2,"type":"IP","ifname":"rmnet0","ipaddr":"42.70.76.153/30","dns":["168.95.1.1","168.95.192.1"],"gw":"42.70.76.154","ip":"42.70.76.153","netmask":"255.255.255.252","broadcast":"42.70.76.155","state":1,"radioTech":3,"apn":"internet","chappap":3,"pdptype":"IP","rilMessageType":"datacallstatechange"}

This is because when flight mode was enabled, we received an unsolicited data call list 
with state down so we updated the data call to state "down" (but not removed from |currentDataCalls|). 
When flight mode was disabled, the new established data call will not be treated as a new one, but a data call state change, hence ip, gw and other info were not updated.

Comment 1

[Jessica Jong [:jessica] (inactive)]

Currently, when user turns airplane mode on, RIL receives "ril.radio.disabled: true" first and "ril.data.enabled: false" after, so the data call is not deactivated because radio state is not in "GECKO_RADIOSTATE_READY" anymore.

We should consider deactivating data call before turning off radio.

Comment 2

[Jessica Jong [:jessica] (inactive)]

Attachment: fix-data-not-workable-after-flightmode.patch

Comment 3

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

Comment on attachment 795348 [details] [diff] [review]
fix-data-not-workable-after-flightmode.patch

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

::: dom/system/gonk/RadioInterfaceLayer.js
@@ +1497,5 @@
>      }
>  
>      if (this.rilContext.radioState == RIL.GECKO_RADIOSTATE_OFF &&
>          this._radioEnabled) {
>        this.setRadioEnabled(true);

During the time we're deactivating data connections, |this._radioPowerOffPending| was set to true but |this._changingRadioPower| remains false.  So it is possible that user may quickly turn on radio again and cause unexpected behaviour then.  Is it possible to re-use |this._changingRadioPower| to guard whole power off process just like power on?

@@ +1522,5 @@
> +   * Clean up existing data call before turning radio off.
> +   */
> +  powerOffRadioSafely: function powerOffRadioSafely() {
> +    if (!this._radioPowerOffPending) {
> +      let defaultDataCallState = this.getDataCallStateByType("default");

Shouldn't we disconnect all data connections?

Comment 4

[Jessica Jong [:jessica] (inactive)]

Attachment: fix-data-not-workable-after-flightmode.patch

Address comments in Comment 3:
1. Reuse |this._changingRadioPower| the guard the whole powering on/off process.
2. Clear all data connections before turning radio off.

Comment 5

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

Comment on attachment 795876 [details] [diff] [review]
fix-data-not-workable-after-flightmode.patch

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

Almost done, but when is |this._changingRadioPower| set to false?

Comment 6

[Jessica Jong [:jessica] (inactive)]

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #5)
> Comment on attachment 795876 [details] [diff] [review]
> fix-data-not-workable-after-flightmode.patch
> 
> Review of attachment 795876 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Almost done, but when is |this._changingRadioPower| set to false?

|this._changingRadioPower| is set to false in handleRadioStateChange() [1], this is called when we actually receive a radio state change event from lower layer, meaning the whole process is finished.

[1] http://mxr.mozilla.org/mozilla-central/source/dom/system/gonk/RadioInterfaceLayer.js#1460

Comment 7

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

Comment on attachment 795876 [details] [diff] [review]
fix-data-not-workable-after-flightmode.patch

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

Thank you :)

Comment 8

[Jessica Jong [:jessica] (inactive)]

Thanks!

Try results: https://hg.mozilla.org/try/rev/62ae3000c961

Comment 9

[Jessica Jong [:jessica] (inactive)]

Sorry, this is the correct link to try results:
https://tbpl.mozilla.org/?tree=Try&rev=2525bea1334e

Comment 10

[Ryan VanderMeulen [:RyanVM]]

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

Comment 11

[Ryan VanderMeulen [:RyanVM]]

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