794767 - B2G RIL: Handle data call error without APN name

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[Shian-Yow Wu [:swu]]

Attachment: Error Log: _processDataCallList without 'newDataCallOptions'

In some cases we may receive _processDataCallList() without 'newDataCallOptions', this will cause problem for data call error handling.  Without APN name, data call error handling cannot point to right APN connection.

This can be reproduced in latest gecko, by enabling data call, then go to airplane mode. 

To fix it, we can get APN name from currentDataCalls{} by comparing cid.

Comment 1

[Shian-Yow Wu [:swu]]

This bug also stops to resume data call from airplane mode off, must be blocking-basecamp.

Comment 2

[Andrew Overholt [:overholt]]

We have to resume data after airplane mode is shut off so agreed, it's a blocker.

Comment 3

[Andrew Overholt [:overholt]]

Shian-Yow, can you take this?

Comment 4

[Vincent Liu[:vliu]]

Hi Shian-Yow,
When I tried to reproduce this issue, I found something from log.
When the data call was built and then enabled Airplane mode, I could see the below parcel in worker.

10-01 13:29:14.356 I/Gecko   (  106): RIL Worker: Handling parcel as REQUEST_DEACTIVATE_DATA_CALL

But when I disabled Airplane mode, I couldn't see REQUEST_SETUP_DATA_CALL was shown in the log. The reason why I expected to see this parcel is that it was a pair when I tried to enable/disable Data Connection. Furthermore, When I saw the the definition RIL_REQUEST_SETUP_DATA_CALL in ril.h in /hardware/ril/include/telephony/, it was also written "See also: RIL_REQUEST_DEACTIVATE_DATA_CALL".

Please correct me if I have anything wrong. I will try more on this issue.

Comment 5

[Shian-Yow Wu [:swu]]

Attachment: WIP V1

Add null checking on 'newDataCallOptions'.

Comment 6

[Shian-Yow Wu [:swu]]

(In reply to vliu from comment #4)
> Hi Shian-Yow,
> When I tried to reproduce this issue, I found something from log.
> When the data call was built and then enabled Airplane mode, I could see the
> below parcel in worker.
> 
> 10-01 13:29:14.356 I/Gecko   (  106): RIL Worker: Handling parcel as
> REQUEST_DEACTIVATE_DATA_CALL
> 
> But when I disabled Airplane mode, I couldn't see REQUEST_SETUP_DATA_CALL
> was shown in the log. The reason why I expected to see this parcel is that
> it was a pair when I tried to enable/disable Data Connection. Furthermore,
> When I saw the the definition RIL_REQUEST_SETUP_DATA_CALL in ril.h in
> /hardware/ril/include/telephony/, it was also written "See also:
> RIL_REQUEST_DEACTIVATE_DATA_CALL".
> 
> Please correct me if I have anything wrong. I will try more on this issue.

When enabled airplane mode, we first got UNSOLICITED_DATA_CALL_LIST_CHANGE followed by calling _processDataCallList().  The previous connected data call in the data call list will be status:65535, therefore it will be handled this._sendDataCallError().  However due to 'newDataCallOptions' is null, it will cause JavaScript error and .  We should fix this issue first.

The second problem is, when REQUEST_DEACTIVATE_DATA_CALL been handled, it never get response when the phone went into airplance mode.  So the data call will be staying in DISCONNECTING state, which blocked further RIL_REQUEST_SETUP_DATA_CALL after airplane mode disabled.

Is it same as your testing?

Comment 7

[Vincent Liu[:vliu]]

(In reply to Shian-Yow Wu from comment #6)
> (In reply to vliu from comment #4)
> > Hi Shian-Yow,
> > When I tried to reproduce this issue, I found something from log.
> > When the data call was built and then enabled Airplane mode, I could see the
> > below parcel in worker.
> > 
> > 10-01 13:29:14.356 I/Gecko   (  106): RIL Worker: Handling parcel as
> > REQUEST_DEACTIVATE_DATA_CALL
> > 
> > But when I disabled Airplane mode, I couldn't see REQUEST_SETUP_DATA_CALL
> > was shown in the log. The reason why I expected to see this parcel is that
> > it was a pair when I tried to enable/disable Data Connection. Furthermore,
> > When I saw the the definition RIL_REQUEST_SETUP_DATA_CALL in ril.h in
> > /hardware/ril/include/telephony/, it was also written "See also:
> > RIL_REQUEST_DEACTIVATE_DATA_CALL".
> > 
> > Please correct me if I have anything wrong. I will try more on this issue.
> 
> When enabled airplane mode, we first got UNSOLICITED_DATA_CALL_LIST_CHANGE
> followed by calling _processDataCallList().  The previous connected data
> call in the data call list will be status:65535, therefore it will be
> handled this._sendDataCallError().  However due to 'newDataCallOptions' is
> null, it will cause JavaScript error and .  We should fix this issue first.
> 
> The second problem is, when REQUEST_DEACTIVATE_DATA_CALL been handled, it
> never get response when the phone went into airplYance mode.  So the data
> call will be staying in DISCONNECTING state, which blocked further
> RIL_REQUEST_SETUP_DATA_CALL after airplane mode disabled.
> 
> Is it same as your testing?

Yes, I saw the second case. What I saw is the this.dataNetworkInterface.state kept in GECKO_NETWORK_STATE_DISCONNECTING state and then it couldn't issue RIL_REQUEST_SETUP_DATA_CALL again after airplane mode disabled. I thought it is the reason why the data call couldn't back to alive. I think the problem is this.dataNetworkInterface.state couldn't change to GECKO_NETWORK_STATE_DISCONNECTED when Airplane mode is enabled. Is it another issue we should file?

Comment 8

[Shian-Yow Wu [:swu]]

After problem 1 ('newDataCallOptions' is null) addressed, data call can be resumed after airplane mode off.  

But there's another issue we should fix.

When airplane mode disabled, device will:
1. Changing radio power to off
2. Receive data call list update with status:65535 for current data call, then it changed the state of the current data call from GECKO_NETWORK_STATE_CONNECTED to GECKO_NETWORK_STATE_UNKNOWN.
4. Gaia system app apps/system/js/airplane_mode.js turns off "ril.data.enabled", which triggers an deactivate data call in Gecko side.
5. Due to data call debouncing function (bug 785982), device will try to setup date call again and got error (because radio power is turning off).
6. Receive radio power state changed to off.

IMHO, during radio changing, we should not allow any data call setup/deactivate, because it either got error or no response.

Vincent, could you file another issue?

Comment 9

[Vincent Liu[:vliu]]

Hi Shian-Yow,

Sure!! I will file another issue for Comment 8.

Comment 10

[Shian-Yow Wu [:swu]]

Attachment: V1

We can get APN name from currentDataCalls{} by comparing cid, but if the error case is not for setup data call, it's better to simply leave newDataCall.apn empty.  Otherwise it will trigger unwanted data call setup by reset() in RILNetworkInterface.

Only add null checking on 'newDataCallOptions' should be better.

Comment 11

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

Comment on attachment 666875 [details] [diff] [review]
V1

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

Since there is similar expression just below the code chunk modified here, I assume the check was just missed before. But I really want to have a complete study about how Android handles all these data call events. It seems we keep having small troubles like APN name is null, newDataCallOptions is null, blahblah. Can we have a full review on this?

Comment 12

[Shian-Yow Wu [:swu]]

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #11)
> Since there is similar expression just below the code chunk modified here, I
> assume the check was just missed before. But I really want to have a
> complete study about how Android handles all these data call events. It
> seems we keep having small troubles like APN name is null,
> newDataCallOptions is null, blahblah. Can we have a full review on this?

Surely we need to have a full review.  I've created a follow up bug 796868 to review null checking for 3G data call.

Comment 13

[Shian-Yow Wu [:swu]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7897fb9e2563

Comment 14

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/7897fb9e2563