1105598 - [FFOS7715 v2.1] [dolphin] Wifi is enabled after enabling wifi ,wifi hotspot and airplane mode.

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Wifi, defect)

Description

[孟宪茂]

Steps to reproduce:

1.go to settings and enable wifi;
2.go to internet sharing and enable wifi hotspot;
3.then enable airplane mode.

result: wifi is enabled unexpectedly.

Comment 1

[孟宪茂]

The main cause is that the wifi resumed after wifi hotspot is disabled by airplane mode.

The code is:
path:B2G/gecko/dom/wifi/WifiWorker.js

 handleWifiTetheringEnabled: function(enabled) {
    // Make sure Wifi is idle before switching to Wifi hotspot mode.
    if (enabled) {
      this.queueRequest({command: "setWifiEnabled", value: false}, function(data) {
        if (WifiManager.isWifiEnabled(WifiManager.state)) {
          this.disconnectedByWifiTethering = true;
          this._setWifiEnabled(false, this._setWifiEnabledCallback.bind(this));
        } else {
          this.requestDone();
        }
      }.bind(this));
    }

    this.queueRequest({command: "setWifiApEnabled", value: enabled}, function(data) {
      this.setWifiApEnabled(enabled, this.requestDone.bind(this));
    }.bind(this));

    if (!enabled) {
      this.queueRequest({command: "setWifiEnabled", value: true}, function(data) {
        if (this.disconnectedByWifiTethering) {
          this._setWifiEnabled(true, this._setWifiEnabledCallback.bind(this));
        } else {
          this.requestDone();
        }
        this.disconnectedByWifiTethering = false;
      }.bind(this));
    }
  },

We should not resume wifi if airplane mode is enabled.

Comment 2

[孟宪茂]

Attachment: wifi_enabled_after_enabling_hotport_and_airplanemode .patch

Comment 3

[孟宪茂]

Dear shawn,

Please help find the owner to review the patch attached in comment 2 and check if it is the fine way to fix this issue.

Thanks!

Comment 4

[shawn ku [:sku]]

Hi EJ:
 May I have your time to check/review this issue?

Thanks!!
Shawn

Comment 5

[Henry Chang [:hchang]]

(In reply to 孟宪茂 from comment #1)
> The main cause is that the wifi resumed after wifi hotspot is disabled by
> airplane mode.
> 
> The code is:
> path:B2G/gecko/dom/wifi/WifiWorker.js
> 
>  handleWifiTetheringEnabled: function(enabled) {
>     // Make sure Wifi is idle before switching to Wifi hotspot mode.
>     if (enabled) {
>       this.queueRequest({command: "setWifiEnabled", value: false},
> function(data) {
>         if (WifiManager.isWifiEnabled(WifiManager.state)) {
>           this.disconnectedByWifiTethering = true;
>           this._setWifiEnabled(false,
> this._setWifiEnabledCallback.bind(this));
>         } else {
>           this.requestDone();
>         }
>       }.bind(this));
>     }
> 
>     this.queueRequest({command: "setWifiApEnabled", value: enabled},
> function(data) {
>       this.setWifiApEnabled(enabled, this.requestDone.bind(this));
>     }.bind(this));
> 
>     if (!enabled) {
>       this.queueRequest({command: "setWifiEnabled", value: true},
> function(data) {
>         if (this.disconnectedByWifiTethering) {
>           this._setWifiEnabled(true,
> this._setWifiEnabledCallback.bind(this));
>         } else {
>           this.requestDone();
>         }
>         this.disconnectedByWifiTethering = false;
>       }.bind(this));
>     }
>   },
> 
> We should not resume wifi if airplane mode is enabled.

Even though it does fix the bug, the wifi internal behavior 
shouldn't be affected by "airplane mode" since airplane mode
is not a concrete and unchanged concept. It really depends
on the implementation of airplane mode and gecko shouldn't
do any assumption. I would let the application layer to 
handle this. For example, turn off wifi hotspot before
putting into airplane mode.

Comment 6

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

After offline discussion with Arthur & Henry, we don't think Gecko should realize that `AirplaneMode.enabled` does exist because this is a private key used by System & Settings app only. Even for other apps, they should only use AirplaneModeHelper to know current status of APM instead of reading this key directly.

The best solution here is like what Henry said, these logics for handling mutex should be handled in Gaia instead of Gecko because there are some special logics only exist in Gaia (like APM).

For workaround here, Gaia will make a new mozSettings key to tell Gecko this is a SPECIAL timing and don't do any mutex operations here. After Gecko reading the key, Gecko will restore it to original state.

For long-term solution, because this takes time and all 2.2 features has been planed, we will try to report such API change to PM and let's see what is the better timing to more out this mutex logics out of Gecko. So right now, let's focus on workaround here and Henry will help to review Gecko's patch.

Thanks all :)

Comment 7

[Henry Chang [:hchang]]

Comment on attachment 8529459 [details] [diff] [review]
wifi_enabled_after_enabling_hotport_and_airplanemode .patch

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

::: dom/wifi/WifiWorker.js
@@ +3233,5 @@
>      }.bind(this));
>  
> +    let lock = gSettingsService.createLock();
> +    let self = this;
> +    lock.get("airplaneMode.enabled",

From Arthur's suggestion, we prefer more generic settings key such as 'tethering.wifi.handle_mutex' and let gaia to control all the flow. 
It's also backward compatible and easy for gaia to replace with wifi/tethering
API in the future.

Comment 8

[Steven Yang [:styang]]

Learned from Vance, change the project name to "FFOS7715 v2.1" for filtering efficiently.

Comment 9

[Vance Chen [:vchen][skype:vance.lucida][[e-mail]]]

Hi Xianmao -

Do you still need our assistance on this issue? or could we close this one now?

Thanks

Vance

Comment 10

[孟宪茂]

Hi Vance,

From the suggestion of EJ Chen and Henry,this issue should be fixed in gaia. But I still can not find a fine way to handle it in gaia,so our solution now still is the way as my attached patch does.

Please help provide the workaround solution in gaia.
Thanks!

Comment 11

[Vance Chen [:vchen][skype:vance.lucida][[e-mail]]]

(In reply to 孟宪茂 from comment #10)
> Hi Vance,
> 
> From the suggestion of EJ Chen and Henry,this issue should be fixed in gaia.
> But I still can not find a fine way to handle it in gaia,so our solution now
> still is the way as my attached patch does.
> 
> Please help provide the workaround solution in gaia.
> Thanks!

I see...

Hi EJ, you mentioned a possible gaia workaround solution for this bug on Comment#6, is it possible you can help to implement the solution?

Thanks

Vance

Comment 12

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

(In reply to Vance Chen [:vchen][[e-mail]] from comment #11)
> (In reply to 孟宪茂 from comment #10)
> > Hi Vance,
> > 
> > From the suggestion of EJ Chen and Henry,this issue should be fixed in gaia.
> > But I still can not find a fine way to handle it in gaia,so our solution now
> > still is the way as my attached patch does.
> > 
> > Please help provide the workaround solution in gaia.
> > Thanks!
> 
> I see...
> 
> Hi EJ, you mentioned a possible gaia workaround solution for this bug on
> Comment#6, is it possible you can help to implement the solution?
> 
> Thanks
> 
> Vance

[Short version]

We think what Xianmao did is okay,but all we want to change is to use different settings key like "ril.ignore.mutex" when to tell gecko not to do mutex operations instead of reading "airplanemode.enabled"

[TR;DR]

I remembered that when Henry, Arthur and I were discussing this problem one month ago, we were thinking that the way Xianmao did is right but we for gecko layer, it should not realize the existence of "Airplanemode.enabled" because this is the abstract concept only lives in Gaia instead of Gecko.

What we suggested then was to use another special settings key like "ril.ignore.mutex" from gaia and gecko would read that key to know we are right now at the timing to ignore mutex stuffs like how Xianmao did.

Comment 13

[Vance Chen [:vchen][skype:vance.lucida][[e-mail]]]

> 
> [Short version]
> 
> We think what Xianmao did is okay,but all we want to change is to use
> different settings key like "ril.ignore.mutex" when to tell gecko not to do
> mutex operations instead of reading "airplanemode.enabled"
> 
> [TR;DR]
> 
> I remembered that when Henry, Arthur and I were discussing this problem one
> month ago, we were thinking that the way Xianmao did is right but we for
> gecko layer, it should not realize the existence of "Airplanemode.enabled"
> because this is the abstract concept only lives in Gaia instead of Gecko.
> 
> What we suggested then was to use another special settings key like
> "ril.ignore.mutex" from gaia and gecko would read that key to know we are
> right now at the timing to ignore mutex stuffs like how Xianmao did.

Thanks EJ, got it

Hi Xianmao, as EJ mentioned, the concept of your fix is OK, it is just that you shouldn't refer "Airplanemode.enable" directly from Gecko since Airplanemode.enable is a Gaia layer concept. 

So the alternative is to use something else instead of Airplanemode.enable(e.g. something like ril.ignore.mutex). As EJ mentioned in Comment#6:

"For workaround here, Gaia will make a new mozSettings key to tell Gecko this is a SPECIAL timing and don't do any mutex operations here. After Gecko reading the key, Gecko will restore it to original state."

So all you need to do is to refine your patch with some modifications in the Gaia side. Could you work on that?

Thanks

Vance

Comment 14

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

[Gaia]

https://gist.github.com/EragonJ/568a94e39c0f2961dfdb

for

https://github.com/mozilla-b2g/gaia/blob/v2.1/apps/system/js/airplane_mode.js#L122

[Gecko]

https://gist.github.com/EragonJ/9eab4a1eb44f4c15e29b

for 

https://bug1105598.bugzilla.mozilla.org/attachment.cgi?id=8529459

--

These are some conceptual code without testing but I think it's a good start though.

Comment 15

[Vance Chen [:vchen][skype:vance.lucida][[e-mail]]]

(In reply to EJ Chen [:eragonj][:小龍哥][ni? if you need me] from comment #14)
> [Gaia]
> 
> https://gist.github.com/EragonJ/568a94e39c0f2961dfdb
> 
> for
> 
> https://github.com/mozilla-b2g/gaia/blob/v2.1/apps/system/js/airplane_mode.
> js#L122
> 
> [Gecko]
> 
> https://gist.github.com/EragonJ/9eab4a1eb44f4c15e29b
> 
> for 
> 
> https://bug1105598.bugzilla.mozilla.org/attachment.cgi?id=8529459
> 
> --
> 
> These are some conceptual code without testing but I think it's a good start
> though.

EJ, much appreciate your help here!

Vance

Comment 16

[siiaceon]

thanks vance and EJ's support, expect official regular solution on 2.2

Comment 17

[孟宪茂]

CLOSE THE ISSUE

Comment 18

[Derek Harris [:DerekH] - Dont need info. Last day 6/5/15]

This issue is still occuring on the most recent Flame 3.0 and 2.2 builds.

The wifi becomes enabled after enabling airplane mode

Environmental Variables:
Device: Flame 3.0 (319mb)(Kitkat)(Full Flash)
Build ID: 20150128010234
Gaia: 1d53fb07984298253aad64bfa4236b7167ee3d4d
Gecko: b2b10231606b
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 38.0a1 (3.0)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:38.0) Gecko/38.0 Firefox/38.0


Environmental Variables:
Device: Flame 2.2 (319mb)(Kitkat)(Full Flash)
Build ID: 20150127002504
Gaia: 80d5b797fd0497a7e3337b7798a21b2e1219681a
Gecko: 01bf1516a65b
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 37.0a2 (2.2)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0


If a new bug should be filed for this issue, please let me know and I will do so.

Comment 20

[Peter Bylenga [:PBylenga]]

Derek, please write up a new issue and reference it here.

Comment 21

[Derek Harris [:DerekH] - Dont need info. Last day 6/5/15]

A new bug has been written: https://bugzilla.mozilla.org/show_bug.cgi?id=1128010