B2G RIL: toggling on->off 3G data call quickly caused data call stay in UP state

RESOLVED FIXED in mozilla18

Status

()

RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: swu, Assigned: swu)

Tracking

Trunk
mozilla18
ARM
Gonk (Firefox OS)
Points:
---
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(blocking-basecamp:+)

Details

(Whiteboard: [LOE:S] [WebAPI:P0])

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

6 years ago
I saw this on Otoro device.

STR:
1. Make sure Data Connection is disabled in the beginning.
2. Go to settings, toggle on->off "Data Connection" quickly.
3. Check "netcfg" in adb shell, the rmnet0 stays in UP state forever.  Further on/off no longer working.

When this happened, according to logcat, the data call state was 2 (GECKO_NETWORK_STATE_SUSPENDED)

I/Gecko   (  105): -*- RadioInterfaceLayer: Received message from worker: {"status":0,"suggestedRetryTime":-1,"cid":"0","active":1,"type":"IP","ifname":"rmnet0","ipaddr":"177.144.41.5/30","dns":["200.220.227.56","200.142.130.202"],"gw":"177.144.41.6","ip":"177.144.41.5","netmask":"255.255.255.252","broadcast":"177.144.41.7","state":2,"radioTech":1,"apn":"","user":"","passwd":"","chappap":3,"pdptype":"IP","rilMessageType":"datacallstatechange"}
I think what we should do here is ignore any changes to the 'ril.data.enabled' setting while RILNetworkInterface.state == nsINetworkInterface.NETWORK_STATE_CONNECTING or DISCONNECTING. Then, when the datacall set-up or tear-down is complete and we're changing RILNetworkInterface.state to nsINetworkInterface.NETWORK_STATE_CONNECTED or DISCONNECTED, we check 'ril.data.enabled' once more. If it's now inconsistent with RILNetworkInterface.state, we tear down or set up the data call again.


I think this block basecamp because

(a) it's entirely possible for users to fat-finger the "data enabled" setting and flip it very quickly. Leaving the data call in an inconsistent state in such cases should not happen.

(b) we actually require the UI layer to flip the 'ril.data.enabled' setting programmatically when other data settings change. Fixing this bug would make it easier to implement that reliably.
blocking-basecamp: --- → ?
Assignee: nobody → swu
blocking-basecamp: ? → +
(Assignee)

Comment 2

6 years ago
Created attachment 656953 [details] [diff] [review]
WIP V1

This is a WIP patch.
I'm doing more tests before asking for review.
(Assignee)

Comment 3

6 years ago
Test cases:
1. Toggle on
Expected result: data call stays in CONNECTED state

2. Toggle off
Expected result: data call stays in DISCONNECTED state

3. Quickly toggle on->off
Expected result: data call stays in DISCONNECTED state

4. Quickly toggle off->on
Expected result: data call stays in CONNECTED state

5. Quickly Toggling on/off repeatedly
Expected result: data call stays in correct state according to last on/off status

6. Toggle on and reboot
Expected result: after reboot, data call stays in CONNECTED state
(Assignee)

Updated

6 years ago
Whiteboard: [LOE:S]
Whiteboard: [LOE:S] → [LOE:S] [WebAPI:P0]
(Assignee)

Comment 4

6 years ago
Status update:

The current implementation can work properly by the test cases.
Need to rebase after bug 744700 landed and test again.
Summary: B2G RIL: toggling on->off 3G data call quickly caused data call stay in UP state forever → B2G RIL: toggling on->off 3G data call quickly caused data call stay in UP state
(Assignee)

Comment 5

6 years ago
Created attachment 663373 [details] [diff] [review]
WIP V2

Slightly changed and rebase on bug 782513.
Attachment #656953 - Attachment is obsolete: true
Attachment #663373 - Flags: review?(philipp)
Comment on attachment 663373 [details] [diff] [review]
WIP V2

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

::: dom/system/gonk/RadioInterfaceLayer.js
@@ +1965,5 @@
>      }
>  
>      this.state = datacall.state;
>  
> +    this.mRIL.updateRILNetworkInterface(); 

Nice, this is elegant. Maybe we can add a small comment here to explain why we're doing this, e.g.:

  // In case the data setting changed while the datacall was being started or
  // ended, let's re-check the setting and potentially adjust the datacall
  // state again.

Also, since you mentioned you rebased it on bug 782513: should we make sure we only do this for the `dataNetworkInterface`? E.g.

  if (this == this.mRIL.dataNetworkInterface) {
    this.mRIL.updateRILNetworkInterface();
  }

(Not sure there's a more elegant way of doing this.)

Ahyway, r=me with those. Thanks!
Attachment #663373 - Flags: review?(philipp) → review+
(Assignee)

Comment 7

6 years ago
Created attachment 664924 [details] [diff] [review]
V3 (r=philikon)

Addressed review comments.
Attachment #663373 - Attachment is obsolete: true
(Assignee)

Comment 8

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/ed7f4ba92ab0
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla18
https://hg.mozilla.org/mozilla-central/rev/ed7f4ba92ab0
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.