1042478 - [Gaia] AirplaneModeHelper.ready is invoked several times when enabling airplane mode.

Status: RESOLVED FIXED

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

Description

[Pin Zhang [:pzhang] (inactive)]

+++ This bug was initially created as a clone of Bug #1042175 +++
|AirplaneModeHelper.ready| is supposed to be invoked only once, but when airplane mode is enabled, it will be called multiple times.

Comment 1

[Pin Zhang [:pzhang] (inactive)]

 38         this.addEventListener(kEventName, function onChangeEvent() {
 39           // make sure _cachedStatus is definitely not ''
 40           if (this._cachedStatus !== '') {
 41             this.removeEventListener(kEventName, onChangeEvent);
 42             cb();   
 43           }     
 44         }.bind(this));
             ^^^^^^^^^^^^ I think we need to move context binding here which causes removeEventListener failed in line 41.

Comment 2

[Pin Zhang [:pzhang] (inactive)]

Attachment: PR 22056

Comment 4

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

Comment on attachment 8460719 [details] [review]
PR 22056

Thanks for finding this scope problem. Can we make a unit test for this case ?

Comment 5

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

BTW, You can create one test file named airplane_mode_helper_test.js under sharedtest/test/ .

Comment 6

[Pin Zhang [:pzhang] (inactive)]

(In reply to EJ Chen [:eragonj][:小龍哥] from comment #5)
> BTW, You can create one test file named airplane_mode_helper_test.js under
> sharedtest/test/ .

I didn't find any tests for shared js, any doc for this?

Comment 7

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

No, but you can check the others for reference. Like this https://github.com/mozilla-b2g/gaia/blob/master/apps/sharedtest/test/unit/simslot_test.js

Comment 8

[Pin Zhang [:pzhang] (inactive)]

(In reply to EJ Chen [:eragonj][:小龍哥] from comment #7)
> No, but you can check the others for reference. Like this
> https://github.com/mozilla-b2g/gaia/blob/master/apps/sharedtest/test/unit/
> simslot_test.js

OK, thanks.

Comment 9

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

Comment on attachment 8460719 [details] [review]
PR 22056

Please set the review flag on me later when test is done, thanks for the help Pin Zhang !

Comment 10

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

Comment on attachment 8460719 [details] [review]
PR 22056

Hi PinZhang,

I just left some comments on Github and I think the patch is good enough !

Thanks again for your hard work and please set r? again after comments are addressed.

Comment 11

[Pin Zhang [:pzhang] (inactive)]

(In reply to EJ Chen [:eragonj][:小龍哥] from comment #10)
> Comment on attachment 8460719 [details] [review]
> PR 22056
> 
> Hi PinZhang,
> 
> I just left some comments on Github and I think the patch is good enough !
> 
> Thanks again for your hard work and please set r? again after comments are
> addressed.

Oops .. didn't configure gjslint well.

Comment 12

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

Comment on attachment 8460719 [details] [review]
PR 22056

Thanks PinZhang !!

r+ and please land the patch after passing tbpl tests.

Comment 13

[Pin Zhang [:pzhang] (inactive)]

Gaia Build Test opt on B2G Desktop Linux x64 is burning, but i don't think it's caused by this PR, let's land it.

TEST-UNEXPECTED-FAIL | /builds/slave/test/gaia/build/test/integration/build.test.js | Build GAIA from differece app list GAIA_DEVICE_TYPE=tablet make
Bug 1022192 - Intermittent build.test.js | Build GAIA from differece app list GAIA_DEVICE_TYPE=tablet make
Bug 1024047 - Intermittent build.test.js | Node modules tests make node_modules from github or git mirror
Bug 1025111 - Intermittent build.test.js | Build Integration tests make with HAIDA=1 (and more)
Bug 1022196 - Intermittent build.test.js | Build Integration tests make with DEBUG=1 TEST-UNEXPECTED-FAIL | /builds/slave/test/gaia/build/test/integration/build.test.js | Build GAIA from differece app list "after each" hook
Bug 1022192 - Intermittent build.test.js | Build GAIA from differece app list GAIA_DEVICE_TYPE=tablet make
Bug 1024047 - Intermittent build.test.js | Node modules tests make node_modules from github or git mirror
Bug 1025111 - Intermittent build.test.js | Build Integration tests make with HAIDA=1 (and more)
Bug 1022196 - Intermittent build.test.js | Build Integration tests make with DEBUG=1 TEST-UNEXPECTED-FAIL | /builds/slave/test/gaia/build/test/integration/build.test.js | Build Integration tests make with GAIA_OPTIMIZE=1 BUILD_DEBUG=1
Bug 1022192 - Intermittent build.test.js | Build GAIA from differece app list GAIA_DEVICE_TYPE=tablet make
Bug 1024047 - Intermittent build.test.js | Node modules tests make node_modules from github or git mirror
Bug 1025111 - Intermittent build.test.js | Build Integration tests make with HAIDA=1 (and more)
Bug 1022196 - Intermittent build.test.js | Build Integration tests make with DEBUG=1 Exception: [Exception... "Component returned failure code: 0x80520006 (NS_ERROR_FILE_TARGET_DOES_NOT_EXIST) [nsILocalFile.isFile]" nsresult: "0x80520006 (NS_ERROR_FILE_TARGET_DOES_NOT_EXIST)" location: "JS frame :: resource://gre/modules/commonjs/toolkit/loader.js -> file:///builds/slave/test/gaia/build/webapp-zip.js :: WebappZip.prototype.addToZip :: line 118" data: no]
TEST-UNEXPECTED-FAIL | /builds/slave/test/gaia/build/test/integration/build.test.js | Build Integration tests "after each" hook
Bug 1022192 - Intermittent build.test.js | Build GAIA from differece app list GAIA_DEVICE_TYPE=tablet make
Bug 1024047 - Intermittent build.test.js | Node modules tests make node_modules from github or git mirror
Bug 1025111 - Intermittent build.test.js | Build Integration tests make with HAIDA=1 (and more)
Bug 1022196 - Intermittent build.test.js | Build Integration tests make with DEBUG=1 make: *** [build-test-integration] Error 4
Return code: 2
Tests exited with return code 2: harness failures
# TBPL FAILURE #

Comment 14

[Pin Zhang [:pzhang] (inactive)]

https://github.com/mozilla-b2g/gaia/commit/3a0c96131210cfa7d2473a8f8e1979183547f691

Comment 16

[Pin Zhang [:pzhang] (inactive)]

Here is the PR to land this in v1.4:
  https://github.com/mozilla-b2g/gaia/pull/22288

Comment 17

[Pin Zhang [:pzhang] (inactive)]

(In reply to Pin Zhang [:pzhang] from comment #16)
> Here is the PR to land this in v1.4:
>   https://github.com/mozilla-b2g/gaia/pull/22288

TBPL tests passed, landed it:
  https://github.com/mozilla-b2g/gaia/commit/142953262d2cb031e3db217206edc3507580b0df

Comment 18

[Ryan VanderMeulen [:RyanVM]]

Does this need to land on v2.0 as well? If so, please request approval (1.4 blockers don't have auto-approval anymore).

Comment 19

[Pin Zhang [:pzhang] (inactive)]

Attachment: PR 22463

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 #): Bug 988445
[User impact] if declined:
  - Bug 1042175: Favorite stations duplicated after toggling airplane mode on then off while FM radio with favorite stations is open. 
  - Bug 1043854: FmRadio is turned on while turning on/off airplay mode.
[Testing completed]: I tried on Flame. Also passed TBPL tests: https://tbpl.mozilla.org/?rev=a8fa2a19ecb64da96184ed7dd26498d17cf75f32&tree=Gaia-Try
[Risk to taking this patch] (and alternatives if risky): Low.
[String changes made]: None.

Comment 20

[bhavana bajaj [:bajaj]]

Comment on attachment 8466870 [details] [review]
PR 22463

Approving as this is a regression from 1.4 landing.

Comment 21

[Ryan VanderMeulen [:RyanVM]]

v2.0: https://github.com/mozilla-b2g/gaia/commit/67cec9b3ead974911cd689752b8fe2fc88294c54