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

RESOLVED FIXED in Firefox OS v1.4

Status

Firefox OS
Gaia::Settings
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: pzhang, Assigned: pzhang)

Tracking

({regression})

unspecified
2.1 S1 (1aug)
ARM
Gonk (Firefox OS)
regression

Firefox Tracking Flags

(blocking-b2g:1.4+, b2g-v1.4 fixed, b2g-v2.0 fixed, b2g-v2.1 fixed)

Details

(Whiteboard: [sprd333713][partner-blocker])

Attachments

(2 attachments)

(Assignee)

Description

4 years ago
+++ 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.
(Assignee)

Comment 1

4 years ago
 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.
(Assignee)

Comment 2

4 years ago
Created attachment 8460719 [details] [review]
PR 22056
Attachment #8460719 - Flags: review?(ejchen)
(Assignee)

Updated

4 years ago
Duplicate of this bug: 1042175
Comment on attachment 8460719 [details] [review]
PR 22056

Thanks for finding this scope problem. Can we make a unit test for this case ?
BTW, You can create one test file named airplane_mode_helper_test.js under sharedtest/test/ .
(Assignee)

Comment 6

4 years ago
(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?
(Assignee)

Comment 8

4 years ago
(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 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 !
Attachment #8460719 - Flags: review?(ejchen)
(Assignee)

Updated

4 years ago
Attachment #8460719 - Flags: review?(ejchen)
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.
Attachment #8460719 - Flags: review?(ejchen)
(Assignee)

Comment 11

4 years ago
(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.
(Assignee)

Updated

4 years ago
Attachment #8460719 - Flags: review?(ejchen)
Comment on attachment 8460719 [details] [review]
PR 22056

Thanks PinZhang !!

r+ and please land the patch after passing tbpl tests.
Attachment #8460719 - Flags: review?(ejchen) → review+
(Assignee)

Comment 13

4 years ago
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 #
(Assignee)

Comment 14

4 years ago
https://github.com/mozilla-b2g/gaia/commit/3a0c96131210cfa7d2473a8f8e1979183547f691
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Duplicate of this bug: 1043854
status-b2g-v1.4: --- → affected
(Assignee)

Comment 16

4 years ago
Here is the PR to land this in v1.4:
  https://github.com/mozilla-b2g/gaia/pull/22288
Whiteboard: [sprd333713][partner-blocker]
(Assignee)

Comment 17

4 years ago
(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
blocking-b2g: --- → 1.4+
Does this need to land on v2.0 as well? If so, please request approval (1.4 blockers don't have auto-approval anymore).
Assignee: nobody → pzhang
status-b2g-v1.4: affected → fixed
status-b2g-v2.0: --- → ?
status-b2g-v2.1: --- → fixed
Flags: needinfo?(pzhang)
Target Milestone: --- → 2.1 S1 (1aug)
(Assignee)

Comment 19

4 years ago
Created attachment 8466870 [details] [review]
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.
Attachment #8466870 - Flags: approval-gaia-v2.0?(fabrice)
Flags: needinfo?(pzhang)
Attachment #8466870 - Flags: approval-gaia-v2.0?(fabrice) → approval-gaia-v2.0?(release-mgmt)

Updated

4 years ago
Keywords: regression
Comment on attachment 8466870 [details] [review]
PR 22463

Approving as this is a regression from 1.4 landing.
Attachment #8466870 - Flags: approval-gaia-v2.0?(release-mgmt) → approval-gaia-v2.0+
You need to log in before you can comment on or make changes to this bug.