917097 - [Wifi] Wifi is enabled in airplane mode.

Status: RESOLVED DUPLICATE of bug 1105598

(Firefox OS Graveyard :: Wifi, defect)

Description

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Reported in Bug 855513 comment 19.

STR:
1. Enable Wifi.
2. Enable Hotspot.
3. Enable Airplane Mode.

Expect:
Wifi and Hotspot both disabled.

Actual:
Wifi is enabled.

Caused by the behavior introduced in bug 855513, which will try to enable wifi after hotspot is disabled in such STR.

Comment 1

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

The cause for this problem is the "wifi/hotspot mutex" mechanism introduced in bug 855513.

After STR 2, if you disable Hotspot, Wifi will be enabled automatically.
This is what gaia do in STR 3, so Wifi is enabled.

Since gecko can't tell the situation requesting disable, I think it's better to fix this bug in Gaia.

So my proposal is move the "wifi/hotspot mutex" mechanism from gecko to gaia, and let gaia manage the enable/disable/suspend status.

Comment 2

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

After discussion with Arthur, The plan now is move the mutex mechanism from gecko to Gaia.

But for this bug, I prefer just remove the mechanism in WifiManager.

A follow up bug will be issued to create the mechanism in Gaia, in following steps:
1. Add following APIs to WifiManager
   interface nsIDOMWifiManager : nsISupports
   {
     attribute nsIDOMEventListener onwifitetheringenabled;
     attribute nsIDOMEventListener onwifitetheringdisabled;
   }
2. Gaia implement mutex mechanism and listen to tethering enabled/disabled event to check if command succeed.

Comment 3

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

If all device APIs agree to use API to control enable/disable instead of settings, then we'll take following step:
1. Add support enable/disable control by both API and settings for backward compatibility.
   interface nsIDOMWifiManager : nsISupports
   {
     nsIDOMDOMRequest setWifiEnable(bool enabled);
     nsIDOMDOMRequest setWifiTetheringEnable(bool enabled);
   }
2. Gaia switch enable/disable control from settings to API.
3.(optional) WifiManager remove support for enable/disable control by settings.

Comment 4

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0001. Remove auto enable and auto disable mechanism between wifi and hotspot.

Remove the auto enable/disable mechanism between wifi and hotspot, but preserve the request queue to make sure commands are executed as ordered.

Comment 5

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Gaia need to do some modification on the checkbox enable/disable control for this patch, refer to bug 922535.

Comment 6

[Vincent Chang[:vchang][changyihsin]]

> But for this bug, I prefer just remove the mechanism in WifiManager.
> 
> A follow up bug will be issued to create the mechanism in Gaia, in following
> steps:
> 1. Add following APIs to WifiManager
>    interface nsIDOMWifiManager : nsISupports
>    {
>      attribute nsIDOMEventListener onwifitetheringenabled;
>      attribute nsIDOMEventListener onwifitetheringdisabled;
>    }

I filed Bug 927298.

Comment 7

[Vincent Chang[:vchang][changyihsin]]

(In reply to Chuck Lee [:chucklee] from comment #3)
> If all device APIs agree to use API to control enable/disable instead of
> settings, then we'll take following step:
> 1. Add support enable/disable control by both API and settings for backward
> compatibility.
>    interface nsIDOMWifiManager : nsISupports
>    {
>      nsIDOMDOMRequest setWifiEnable(bool enabled);
>      nsIDOMDOMRequest setWifiTetheringEnable(bool enabled);
>    }
I filed bug 930355 for this.

Comment 8

[Vincent Chang[:vchang][changyihsin]]

Comment on attachment 812500 [details] [diff] [review]
0001. Remove auto enable and auto disable mechanism between wifi and hotspot.

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

Give the r+ because the patch looks good for me.
We still need others patch ready so please pause for a second until others patch are ready.