Closed
Bug 800263
Opened 9 years ago
Closed 8 years ago
[FM Radio] Launch the FM Radio with headphone already plugged-in will cause screen freeze
Categories
(Firefox OS Graveyard :: General, defect, P1)
Tracking
(blocking-basecamp:+, firefox18 fixed, firefox19 fixed)
VERIFIED
FIXED
blocking-basecamp | + |
People
(Reporter: johnshih.bugs, Assigned: slee)
References
Details
(Keywords: smoketest, Whiteboard: [caf:blocking])
Attachments
(2 files, 3 obsolete files)
30.48 KB,
text/plain
|
Details | |
1.84 KB,
patch
|
slee
:
review+
|
Details | Diff | Splinter Review |
## Environment : Otoro phone, build 2012-10-11 Build info: * "gaia" revision= beee2bf29e779b12306b16c2bc2785c1c22ede45 * "gecko" revision= c0ea0728044b2a5f9556ca988635140963cff037 ## Repro : 1. Plug in the headphone 2. Launch the FM Radio app 3. Try to press the buttons ## Expected: * The app works fine ## Actual: * The screen will freeze and can't do anything (even press the sleep button) but put off the battery ## Note: * It works fine if launch the app first, then plug in the headphone
Comment 1•9 years ago
|
||
This is absolutely blocking. I can reproduce this on the Unagi phone, 10-11 build, and my phone completely freezes. i have to reboot the phone. logcat attached, but i dont see anything that logs the freeze.
blocking-basecamp: --- → +
OS: Linux → Gonk
Hardware: x86_64 → ARM
Comment 2•9 years ago
|
||
Comment 3•9 years ago
|
||
I reproduced here, but when the freeze happened, the phone rebooted by itself after a couple of minutes. I guess something was churning in the background that finally crashed us hard.
Comment 4•9 years ago
|
||
I did a couple of runs with gdb but gdb is getting killed also and I could not get a backtrace of anything.
Comment 5•9 years ago
|
||
I updated my Otoro device on 10-09 Beijing time, and there is no such problem, so it is caused by some commits of 10-09, 10-10 or 10-11.
Comment 7•9 years ago
|
||
(In reply to Ray Cheung [:pzhang] from comment #5) > I updated my Otoro device on 10-09 Beijing time, and there is no such > problem, so it is caused by some commits of 10-09, 10-10 or 10-11. I tried to flash the same version to another Otoro device, and it could be reproduced, it's weird ...
Comment 8•9 years ago
|
||
(In reply to Ray Cheung [:pzhang] from comment #7) > (In reply to Ray Cheung [:pzhang] from comment #5) > > I updated my Otoro device on 10-09 Beijing time, and there is no such > > problem, so it is caused by some commits of 10-09, 10-10 or 10-11. > I tried to flash the same version to another Otoro device, and it could be > reproduced, it's weird ... I checked the buildId of the two Otoro devices (cat /system/b2g/application.ini), they are different, the good one was built on 20121009, and the bad one was built on 20121010, both are Beijing time, so the date of the commits which caused the problem should be 10-09.
Comment 9•9 years ago
|
||
Does the kernel update 10 fix this issue?
Comment 10•9 years ago
|
||
John, could you confirm this?
![]() |
||
Comment 11•9 years ago
|
||
I updated my otoro to kernel update 10. It still crashes with below log. E/GeckoConsole( 2480): Content JS LOG at app://fm.gaiamobile.org/js/fm.js:142 in updatePowerUI: Power status: on
Comment 12•9 years ago
|
||
assigning to Ray, feel free to reassign if you don't see fit.
Assignee: nobody → pzhang
Comment 13•9 years ago
|
||
Here is the commit which causes the regression: commit da394578704f4df5b14a6e2ed69e2d204a14b0c5 Author: Dave Hylands <dhylands@mozilla.com> Date: Mon Oct 8 22:46:19 2012 -0600 Bug 797239 - Defer scheduling prelaunch app until child goes idle. r=cjones
Comment 14•9 years ago
|
||
Dave was the owner of Bug 797239, reassign this bug to Dave.
Assignee: pzhang → dhylands
Comment 15•9 years ago
|
||
I'm not sure how bug 797239 can be responsible for this other than to change the timings of things (which would imply that there is a race condition somewhere) and defering the scheduling of the prelaunch app may very well cause the timings to change. So I don't think defering the prelaunch caused the bug, but rather just exposed the bug.
I agree with that analysis. Qualcomm can't reproduce this bug on their development devices, so I think we're just exposing a race condition in the older version of this driver that we're using.
Comment 17•9 years ago
|
||
(In reply to Chris Jones [:cjones] [:warhammer] from comment #16) > I agree with that analysis. Qualcomm can't reproduce this bug on their > development devices, so I think we're just exposing a race condition in the > older version of this driver that we're using. Is it possible that the Bug797698 is the same issue?
Comment 18•9 years ago
|
||
I'm still using one kernel revision behind. I played with various times of dom.ipc.processPrelaunch.delayMs (5, 10, and 20 sec) and the screen freeze occurs very shortly after the prelaunch. And in every case, the phone reboots very shortly afterwards.
Updated•9 years ago
|
Priority: -- → P1
(In reply to Ray Cheung [:pzhang] from comment #17) > (In reply to Chris Jones [:cjones] [:warhammer] from comment #16) > > I agree with that analysis. Qualcomm can't reproduce this bug on their > > development devices, so I think we're just exposing a race condition in the > > older version of this driver that we're using. > Is it possible that the Bug797698 is the same issue? Available evidence suggests a kernel bug, and the bug isn't reproducible in never builds, so we may need an otoro/unagi-specific workaround in gecko :/.
Reporter | ||
Comment 20•9 years ago
|
||
doesn't happen with smoke test today build info gaia master: 98498cb5803349b50637dc35640fc2235fee0499 mozilla-central : bbf233397549175223da3687bc5678d91685e54a
Assignee | ||
Comment 22•9 years ago
|
||
I can reproduce this problem for today's code. ## Environment : Otoro phone, build 2012-10-19 Build info: * "gecko" revision= cc11217f371bbe3f555d4c4f23ec9c817f3171df ## Repro : 1. Plug in the headphone 2. flash.sh 3. Launch the FM Radio app 4. Try to press the buttons
Assignee | ||
Comment 23•9 years ago
|
||
I found that this problem is always reproducible on my device after I flash the whole image. In this patch, I modified the initial function of FM radio. Just follow the initialization method, fm_turn_on, in [1]. It uses |property_set| and checks the property to determine whether the initialization is done. Please help me check whether the patch works on your devices. BTW, should we contact the vendor about this issue? [1] https://bugzilla.mozilla.org/page.cgi?id=splinter.html&bug=749053&attachment=656384
![]() |
||
Comment 24•9 years ago
|
||
Tested with patch, it works fine. ## Environment: # Otoro phone, build 2012-10-22 Build Info: * gecko: 2a4606ceba876a3008f490af2b0fe726e538f27c with patch
Comment 25•9 years ago
|
||
Looks like we need a review and then once r+'ed, we should land this asap. Thanks.
Assignee | ||
Comment 26•9 years ago
|
||
(In reply to Chris Lee [:clee] from comment #25) > Looks like we need a review and then once r+'ed, we should land this asap. > Thanks. Hi clee, This is just a WIP version. I created bug 804455 for discussing if the patch is really solving this problem.
Comment 27•9 years ago
|
||
I can confirm that this patch resolves the issue for me on my unagi.
Reporter | ||
Comment 28•9 years ago
|
||
I met this bug with today build 2012-10-25 gaia master : 4b999a7866c01a61642fa2015df0868d6eb68c88 gecko : 6bb4d66668895b4d24434ce90f483a5246beccd1
Updated•8 years ago
|
Assignee: dhylands → slee
Updated•8 years ago
|
Whiteboard: [caf:blocking]
Comment 30•8 years ago
|
||
smoke-test blockers have the highest priority.
Severity: normal → critical
Keywords: smoketest
Assignee | ||
Comment 32•8 years ago
|
||
Hi Andrew, I have a work around patch and waiting for mwu's response in Bug 804455. BTW, should I mark this bug dup?
Flags: needinfo?(slee)
Assignee | ||
Comment 33•8 years ago
|
||
Attachment #673147 -
Attachment is obsolete: true
Attachment #678216 -
Flags: review?(jones.chris.g)
Comment 35•8 years ago
|
||
Comment on attachment 678216 [details] [diff] [review] patch Review of attachment 678216 [details] [diff] [review]: ----------------------------------------------------------------- All nits but these add up and make the patch look ugly. ::: hal/gonk/GonkFMRadio.cpp @@ +75,5 @@ > + int rc; > + snprintf(version, sizeof(version), "%d", sTavaruaVersion); > + property_set("hw.fm.version", version); > + > + /*Set the mode for soc downloader*/ Add a space after /* and before */. @@ +79,5 @@ > + /*Set the mode for soc downloader*/ > + property_set("hw.fm.mode", "normal"); > + /* start fm_dl service */ > + property_set("ctl.start", "fm_dl"); > + Trailing whitespace. @@ +87,5 @@ > + * work around is from codeaurora > + * (git://codeaurora.org/platform/frameworks/base.git). > + */ > + for (int i = 0; i < 4; ++i) > + { { on the same line as for. @@ +91,5 @@ > + { > + sleep(1); > + char value[PROPERTY_VALUE_MAX]; > + property_get("hw.fm.init", value, "0"); > + if(!strcmp(value, "1")) { Space after if.
Comment on attachment 678216 [details] [diff] [review] patch >diff --git a/hal/gonk/GonkFMRadio.cpp b/hal/gonk/GonkFMRadio.cpp >+ for (int i = 0; i < 4; ++i) >+ { Nit: style is "{" on the same line, for (int i = 0; i < 4; ++i) { Looks good, thanks. r=me with that change.
Attachment #678216 -
Flags: review?(jones.chris.g) → review+
Assignee | ||
Comment 37•8 years ago
|
||
address comment #36
Attachment #678216 -
Attachment is obsolete: true
Attachment #679008 -
Flags: review+
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 38•8 years ago
|
||
Address comment #35, I did not see mwu's comment last time.
Attachment #679008 -
Attachment is obsolete: true
Attachment #679014 -
Flags: review+
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Comment 39•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/d4f04762dc07
Keywords: checkin-needed
Comment 40•8 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/d4f04762dc07
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Updated•8 years ago
|
status-firefox18:
--- → fixed
status-firefox19:
--- → fixed
Reporter | ||
Comment 42•8 years ago
|
||
verified build info 2012-11-09 gaia master : 18107c6961b8c3f559c82fc7be8ba411c4a67c28 mozilla-aurora(hg) : 6735d6d13751
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•