1034455 - [tarako][dolphin] Switch airplane mode over and over again, the airplane mode will be out of commission.

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::System, defect)

Description

[Wei Gao (Spreadtrum)]

OS version
---------------------------------------------
FireFoxOS v1.3t v1.4

Reproduce steps:
---------------------------------------------
Switch airplane mode over and over again.

Expected result:
---------------------------------------------
The airplane mode should be turned on and off all the time.

Actual result:
---------------------------------------------
The airplane mode will not be turned on or off.

Probability:
---------------------------------------------
Occasionally Recurrence

Comment 1

[Wei Gao (Spreadtrum)]

Attachment: radio-enabled.patch

As for its cause, I think that's because the modem's slow response. That's to say, when dispatching 'radio-enabled' or 'radio-disabled', the radio has not been turned on or off yet. 
Then user turn airplane mode again, it will make radio and airplane-gaia chaotic.

https://github.com/mozilla-b2g/gaia/blob/v1.4/apps/system/js/radio.js#L139-149
https://github.com/mozilla-b2g/gaia/blob/v1.4/apps/system/js/radio.js#L164-172

I wonder can this patch be granted.
It will make "window.dispatchEvent" wait untill the radio is turned on or off thoroughly.

Hi Evelyn

Can you help to transmit this bug to the right owner?
Thanks a great.

Comment 2

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

Comment on attachment 8450764 [details] [diff] [review]
radio-enabled.patch

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

Thanks Wei.gao, it makes sense to me and because radio.js resides in System app, you may need its owner's approval as well.

::: apps/system/js/radio.js
@@ +143,5 @@
>          self._setRadioAfterReqsCalled(enabled);
>        };
>  
>        req.onerror = function() {
> +        self._setRadioOpCount++;

Can you put this line below to make code more consistent? thanks :]

Comment 3

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

By the way, Wei.gao, please make a PR for it and ask for approval to land.

Thanks !

Comment 4

[Wei Gao (Spreadtrum)]

(In reply to EJ Chen [:eragonj][:小龍哥] from comment #3)
> By the way, Wei.gao, please make a PR for it and ask for approval to land.

Hi EJ
Thanks for your review.
I will make a PR later. I want to know, is this needed for master also or v1.3t and v1.4 only?

Thanks.

Comment 5

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

Based on your title, this patch is for 1.3t right ? You can ask for approval for 1.3t first to let the team decide whether this should be taken or not. And also, you can create another PR for master (2.1). Thanks.

Comment 6

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

WeiGao, I think you can try to ask for approval for 1.3t, 1.4, 2.0 because this patch is valuable in some edge cases. Thanks for your help !!

Comment 7

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8450764 [details] [diff] [review]
radio-enabled.patch

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

Better having tests.
Best using semaphore. (asyncSemaphore)

Comment 8

[Wei Gao (Spreadtrum)]

Attachment: [v1.3t] pull request

Hi EJ and Alive

This is a PR for v1.3t.
Please help to reviw it.
Thanks a great.

Comment 9

[Wei Gao (Spreadtrum)]

Attachment: [v1.4] pull request

Hi EJ and Alive

This is a PR for v1.4.
Please help to reviw it.
Thanks a great again.

Comment 10

[Wei Gao (Spreadtrum)]

(In reply to Alive Kuo [:alive][NEEDINFO!] from comment #7)
> Better having tests.
> Best using semaphore. (asyncSemaphore)

Hi Alive

Sorry for I can't get your mean?
Is there anything I can perfect it.
Thanks.

Comment 11

[Wei Gao (Spreadtrum)]

Attachment: [v1.3t] pull request

I am sorry for some mistakes in that PR.
I recommit it again.
Please help to review it.

Comment 12

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

Comment on attachment 8452216 [details] [review]
[v1.3t] pull request

Please check my comment on github.

Comment 13

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

Wei.gao, from AriplaneMode side, I am ok with your patch. But from System aspect, please follow Alive's comments.

If you get all r+ from Alive, I will automatically r+ after that. Thanks :)

Comment 14

[Wei Gao (Spreadtrum)]

(In reply to EJ Chen [:eragonj][:小龍哥] from comment #12)
> Comment on attachment 8452216 [details] [review]
> [v1.3t] pull request
> 
> Please check my comment on github.

Thank EJ(龙哥) for your suggestion. I have modified it.

(In reply to EJ Chen [:eragonj][:小龍哥] from comment #13)
> Wei.gao, from AriplaneMode side, I am ok with your patch. But from System
> aspect, please follow Alive's comments.
> 
> If you get all r+ from Alive, I will automatically r+ after that. Thanks :)

Oh, I see.
Thank EJ(龙哥) again.

Comment 15

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8452216 [details] [review]
[v1.3t] pull request

Squash two comments into one please.

Comment 16

[Wei Gao (Spreadtrum)]

(In reply to Alive Kuo [:alive][NEEDINFO!] from comment #15)
> Comment on attachment 8452216 [details] [review]
> [v1.3t] pull request
> 
> Squash two comments into one please.

Thank Alive very much.
Could it be landed on now or a blocking-b2g flag should be added first?

Comment 17

[(inactive after 6/18) Alive Kuo [:alive]]

(In reply to Wei Gao (Spreadtrum) from comment #16)
> (In reply to Alive Kuo [:alive][NEEDINFO!] from comment #15)
> > Comment on attachment 8452216 [details] [review]
> > [v1.3t] pull request
> > 
> > Squash two comments into one please.
> 
> Thank Alive very much.
> Could it be landed on now or a blocking-b2g flag should be added first?

I don't know. For v1.3t I guess it's fine, but I don't know the rule of v1.4 is changed or not.
If not, we need approval-gaia-v1.4

Comment 18

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

Wei.gao please check this : https://wiki.mozilla.org/Release_Management/B2G_Landing

Comment 19

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

Can't find 1.3t?, so let me help you flag 1.4? first and ask later.

Comment 20

[Wei Gao (Spreadtrum)]

(In reply to EJ Chen [:eragonj][:小龍哥] from comment #18)
> Wei.gao please check this :
> https://wiki.mozilla.org/Release_Management/B2G_Landing

Thank EJ(龍哥), that's usefull, I will check it.

Comment 21

[Ivan Tsay (:ITsay)]

Take in v1.4 but ni? Bhavana to see if it needs to be uplifted to 2.0.

Comment 22

[Wei Gao (Spreadtrum)]

Dear EJ

I don't know why there is nobody land PR on v1.3t and v1.4?
Is there anything I forgot to do?
Thanks.

Comment 23

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

(In reply to Wei Gao (Spreadtrum) from comment #22)
> Dear EJ
> 
> I don't know why there is nobody land PR on v1.3t and v1.4?
> Is there anything I forgot to do?
> Thanks.

You may missed my comment from comment #5

> Based on your title, this patch is for 1.3t right ? You can ask for approval
> for 1.3t first to let the team decide whether this should be taken or not.
> And also, you can create another PR for master (2.1). Thanks.

you have to click on the details of attachment and write reasons why you need 
approval-gaia-v1.4 and approval-gaia-v2.0.

And by the way, you have to create another PR for master as what I mentioned above.

Comment 24

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

And also, I think there may not be possible to land code to 1.3t because I can't find any approval flag for it. 

For more details, please check comemnt #18 and this would definitely help.

> Wei.gao please check this :
> https://wiki.mozilla.org/Release_Management/B2G_Landing

Comment 25

[Wei Gao (Spreadtrum)]

Attachment: [master] pull request

Hi Alive and EJ 

This is a PR on master.
Please help to review again.
Thanks a great.

Comment 26

[Wei Gao (Spreadtrum)]

Comment on attachment 8452143 [details] [review]
[v1.4] pull request

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):-
[User impact] if declined:Switch airplane mode over and over again, the airplane mode will be out of commission. That's because the modem's slow response, when dispatching 'radio-enabled' or 'radio-disabled', the radio has not been turned on or off yet. Then user turn airplane mode again, it will make radio and airplane-gaia chaotic.
[Testing completed]:pass on tarako and dolphin
[Risk to taking this patch] (and alternatives if risky):I think there is no risk.
[String changes made]:no

Comment 27

[Ryan VanderMeulen [:RyanVM]]

(In reply to EJ Chen [:eragonj][:小龍哥] from comment #24)
> And also, I think there may not be possible to land code to 1.3t because I
> can't find any approval flag for it. 

Sorry, yeah, our 1.3T uplift situation is a bit of a mess :(. I think the way you want to go is get 1.3T+ blocking status for this bug to take care of that approval. You already have a v1.4 approval request on attachment 8452143 [details] [review], so that'll cover the 1.4 uplift. Not sure if this is something wanted for 2.0 or not, but you'll need to request approval for that as well if wanted.

Note that per our usual tree rules, the master patch still needs to land first before the branch uplifts. AFAICT, that's still waiting on reviews, so I've removed the checkin-needed keyword for now. Sorry again for all the confusion :(

Comment 28

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8452143 [details] [review]
[v1.4] pull request

Clearing the approval request as this is already marked as 1.4+, which means that you're clear to land when ready.

Comment 29

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

Comment on attachment 8456096 [details] [review]
[master] pull request

Thanks Wei.gao !

Comment 30

[Wei Gao (Spreadtrum)]

I think it can be added "checkin-needed" now.

Comment 31

[Ryan VanderMeulen [:RyanVM]]

Thanks! I'm not sure what the procedure is for getting this on to 1.3T is at this point, but I'll get this on to v1.4 once this is confirmed OK on master.

Master: https://github.com/mozilla-b2g/gaia/commit/36d0be8b165f8b4b3ba73622a2d620a7bf52d5be

Comment 32

[Ryan VanderMeulen [:RyanVM]]

v1.4: https://github.com/mozilla-b2g/gaia/commit/e0a732cf172d125eadfad59f0faf8f73b62012b8

Comment 33

[Ryan VanderMeulen [:RyanVM]]

v2.0: https://github.com/mozilla-b2g/gaia/commit/34ebedaaea0bd00a8344851edea843810e5ab900

Comment 34

[Wei Gao (Spreadtrum)]

Dear Julien

I am sorry to trouble.
Do you know how to set a flag of 1.3t approval on this bug, as we all can't find this flag.
Thank you very much.

Comment 35

[Julien Wajsberg [:julienw]]

You have all necessary information in [1]. For v1.3t, you need to have the v1.3t+ blocker flag, so you need to request it. It looks like I don't find the flag either, NI Bhavana on this.

[1] https://wiki.mozilla.org/Release_Management/B2G_Landing#v1.3T

Comment 36

[bhavana bajaj [:bajaj]]

NI : wayne to set 1.3T+ if this is a blocker as requested in comment #34.

Comment 37

[Wayne Chang [:wchang]]

Not taking this on 1.3t now given it's at final stage. 
Edge case here, a user wouldn't hit airplane mode over and over again.

Comment 38

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

Found Test Case: https://moztrap.mozilla.org/manage/case/1295/

STR needs to be added to verify this bug in the existing test case.

Comment 39

[KTucker [:KTucker][Inactive 3/4/2016]]

Steps have been added to cover this issue in this test case:

https://moztrap.mozilla.org/manage/case/1295/

Comment 40

[Sue]

Attachment: verify_video.MP4

According to steps of comment 38, this issue has been verified successfully on Flame v2.1 & v2.0
See attachment: verify_video.MP4
Reproducing rate: 0/5
Flame 2.1 versions:
Gaia-Rev        38e17b0219cbc50a4ad6f51101898f89e513a552
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/8b92c4b8f59a
Build-ID        20141205001201
Version         34.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141205.035305
FW-Date         Fri Dec  5 03:53:16 EST 2014
Bootloader      L1TC00011880

Flame 2.0 versions:
Gaia-Rev        856863962362030174bae4e03d59c3ebbc182473
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/e40fe21e37f1
Build-ID        20141207000206
Version         32.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141207.034341
FW-Date         Sun Dec  7 03:43:52 EST 2014
Bootloader      L1TC00011880