Closed Bug 922334 Opened 11 years ago Closed 11 years ago

crash in sysconf [B2G][Browser][Youtube] Playing youtube videos crash the browser tab

Categories

(Core :: Audio/Video, defect)

ARM
Gonk (Firefox OS)
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla27
blocking-b2g koi+
Tracking Status
firefox25 --- wontfix
firefox26 --- fixed
firefox27 --- fixed
b2g-v1.2 --- fixed

People

(Reporter: gbennett, Assigned: sotaro)

References

()

Details

(4 keywords)

Crash Data

Attachments

(2 files, 6 obsolete files)

Attached file YoutubeCrashLOG.txt (obsolete) (deleted) —
This bug was filed from the Socorro interface and is report bp-2c4cabe2-5bd4-4494-9bd2-0d8b62130930. ============================================================= Description: After launching browser and tapping to play a youtube video the loading wheel freeses and the browser tab crashes with a user friendly error message. Repro Steps: 1) Updated Buri 1.3 mozRIL to Build ID: 20130930071202 2) Open browser app 3) Go to youtube.com 4) Click and play any video Actual: Youtube browser tab crashes. Expected: Youtube does not crash and video plays properly. Environmental Variables Device: Buri 1.3 mozRIL Build ID: 20130930071202 Gecko: http://hg.mozilla.org/mozilla-central/rev/5a49762ee832 Gaia: e7c011371aa6af696033d4b867fb9152a6985efa Platform Version: 27.0a1 Firmware Version: 20130912 Repro frequency: 100%, 8/8 Test Suite Name: Firefox OS Daily Smoketest UCID: N/A Link to failed test case: https://moztrap.mozilla.org/manage/case/6073/ See attached: YoutubeCrashLOG.txt
Component: Gaia::Browser → Video/Audio
Product: Boot2Gecko → Core
Version: unspecified → Trunk
blocking-b2g: koi? → 1.3?
reseting the phone causes the crash to not appear. It appears that there may be some residue data that's causing the crashing to occur. To note, when I didn't reset the phone I also did see the crash.
I was mistaken. This happen even with the reset, I hit it without going to youtube: https://crash-stats.mozilla.com/report/index/48493bd8-6012-4164-9a1e-b59ba2131001
Analyzing the push log, the only patch candidates that landed since 9/27 that look related to this crash were implemented by kinetik.
From the investigation by using git log, the following change seems make the regression. ----------------------------------------------------------- commit 5ce2711c7a26f148c840ccdfa8bd4c239d4b4fc5 Author: Nathan Froyd <froydnj@mozilla.com> Date: Tue Sep 10 15:55:39 2013 -0400 Bug 914826 - part 6 - don't include basictypes.h or nscore.h in ipdl headers; r=ehsan
I narrow downed the bug until around Bug 914826. The following the result of the youtube playback check result and related git log. ------------------------------------------------------------------- commit dda25a1f898331a454f1983e24ac8381ea761bab < youtube failed ************************************> Author: Andrew McCreight <amccreight@mozilla.com> Date: Thu Sep 26 16:46:42 2013 -0700 Bug 920840 - Crash when JS-implemented WebIDL's init method returns a value other than undefined. r=bz commit 5ce2711c7a26f148c840ccdfa8bd4c239d4b4fc5 < youtube failed ************************************> Author: Nathan Froyd <froydnj@mozilla.com> Date: Tue Sep 10 15:55:39 2013 -0400 Bug 914826 - part 6 - don't include basictypes.h or nscore.h in ipdl headers; r=ehsan commit 12fa756043d7c69cbacc8dc17f253d8607017280 < youtube works ************************************> Author: Nathan Froyd <froydnj@mozilla.com> Date: Tue Sep 10 15:45:16 2013 -0400 Bug 914826 - part 5 - provide for cpp-only include files, starting with nsIFile.h and GeckoProfiler.h; r=ehsan commit e8316ed759446e66efd6bf727ddc2b287b140d4d Author: Nathan Froyd <froydnj@mozilla.com> Date: Tue Sep 10 16:56:05 2013 -0400 Bug 914826 - part 4 - fix source files that were bootlegging XPCOM do_* functions via generated ipdl headers; r=ehsan commit 44d376b745ac49dff7c492c44d6eddabbc90004c Author: Nathan Froyd <froydnj@mozilla.com> Date: Tue Sep 10 15:35:51 2013 -0400 Bug 914826 - part 3 - forward-declare nsIFile for GetMinidump() declaration; r=ehsan commit a4908f9bf19c9800966ec4afeafea98ab71d294b Author: Nathan Froyd <froydnj@mozilla.com> Date: Tue Sep 10 15:34:44 2013 -0400 Bug 914826 - part 2 - rename builtinIncludes to builtinHeaderIncludes; r=ehsan commit 515c39f6fad0cc10afd0a338656ca195a056f6c6 Author: Nathan Froyd <froydnj@mozilla.com> Date: Tue Sep 10 15:07:10 2013 -0400 Bug 914826 - part 1 - use static_assert instead of COMPILE_ASSERT in IPCMessageStart.h; r=ehsan commit 8aa87ed7e6b980179dad08ce520c7c1654145508 < youtube works ****************************************> Author: Eitan Isaacson <eitan@monotonous.org> Date: Fri Sep 27 10:03:05 2013 -0700 Bug 918388 - Enable Synth Web Speech API pref in b2g. r=fabrice r=smaug
Depends on: 914826
Just reverting only the following from recent code did not fix the problem. - Bug 914826 - part 6 - don't include basictypes.h or nscore.h in ipdl headers; r=ehsan
(In reply to Sotaro Ikeda [:sotaro] from comment #6) > Just reverting only the following from recent code did not fix the problem. > - Bug 914826 - part 6 - don't include basictypes.h or nscore.h in ipdl > headers; r=ehsan Looks like bug 920840 is the problem, then, no?
Depends on: 920840
No longer depends on: 914826
I see this in the log: 09-30 13:37:53.020: I/Gecko(3409): No permission to use the keyboard API for http://m.youtube.com 09-30 13:37:53.200: I/Gecko(3409): No permission to use the keyboard API for http://m.youtube.com It looks like the page is trying to use a privileged API (?!?) which now fails more aggressively due to a combination of bug 920831 and bug 920840. I think bug 920831 is a more likely regressor, given that it landed on m-c on 9-30, and bug 920840 hasn't landed on m-c yet. But I'd guess the real problem here is whatever the page is doing that is trying to use the keyboard without permissions.
I doubt that YouTube's mobile site is attempting to use privileged B2G APIs, so I'd guess this is a Gaia problem somehow.
Depends on: 920831
No longer depends on: 920840
(In reply to Nathan Froyd (:froydnj) from comment #7) > (In reply to Sotaro Ikeda [:sotaro] from comment #6) > > Just reverting only the following from recent code did not fix the problem. > > - Bug 914826 - part 6 - don't include basictypes.h or nscore.h in ipdl > > headers; r=ehsan > > Looks like bug 920840 is the problem, then, no? I am going to re-check.
Sotaro is quite busy these days - can we get somebody else to find the regression range, especially since it seems to be outside the graphics work? I can't actually tell, I don't have access to either of the two bugs mentioned as possible culprits.
(In reply to Milan Sreckovic [:milan] from comment #11) > Sotaro is quite busy these days - can we get somebody else to find the > regression range, especially since it seems to be outside the graphics work? > I can't actually tell, I don't have access to either of the two bugs > mentioned as possible culprits. Not suggesting others aren't busy - just that Sotaro is on a couple of critical paths, and it'd be good if he wasn't on one more.
Attached file stackinfo of the crash (obsolete) (deleted) —
When the crash happen, stack overflow seems to happen.
(In reply to Sotaro Ikeda [:sotaro] from comment #13) > Created attachment 812649 [details] > stackinfo of the crash > > When the crash happen, stack overflow seems to happen. Having the full stack and not just a truncated stack would be good (keep hitting <Enter> in GDB until you get back to a (gdb) prompt). But since this is infinite recursion in the runtime, I'm going to guess that somebody's doing a division that has undefined results. Providing a full stack will point the finger at who's doing that.
If this doesn't reproduce with the dom.mozInputMethod.testing pref set to true, then that suggests bug 920831 isn't at fault.
Attached file stackinfo of the crash on nexus4 (obsolete) (deleted) —
On nexus4, I could get meaningful stacktrace.
Attached file original regression rage of the git log (obsolete) (deleted) —
At first, I got a change list by uisng git log. I used it to check which change made regression.
In attachment 812706 [details], there is a change around mp3. ---------------------------------------------------- commit 9772d4c7cfb0801219b16c780d4caf73b4404394 Author: Edwin Flores <eflores@mozilla.com> Date: Sat Sep 28 16:33:32 2013 +1200 Bug 919572 - Refactor the MP3 frame parser r=cpearce
Attachment #812649 - Attachment is obsolete: true
It is not clear why master hamachi did not crash around the change in Comment 18.
Does the patch from bug 920831 fix this?
Sorry, this bug should block that bug, on the theory that it is a regression from it. Comment 15 is one way we could test if it really is or not. (Also, I've cc'ed you on it, Milan.)
Blocks: 920831
No longer depends on: 920831
(In reply to Andrew McCreight [:mccr8] from comment #21) > Sorry, this bug should block that bug, on the theory that it is a regression > from it. Comment 15 is one way we could test if it really is or not. (Also, > I've cc'ed you on it, Milan.) I don't think that's right. bug 902831 landed on early afternoon on 9/30, but this bug was filed on a build before that patch landed.
Following code does 0 division when the file is not mp3. It is also a problem that MP3FrameParser is called even when the file is not mp3. By changing the code locally, I confirmed the crash is fixed. And the mp3's change also causes Bug 912829. ----------------------------------------------------------------- > // The duration of each frame is constant over a given stream. > double usPerFrame = USECS_PER_S * SAMPLES_PER_FRAME / mSampleRate; http://mxr.mozilla.org/mozilla-central/source/content/media/MP3FrameParser.cpp#377
Depends on: 919572
(In reply to Sotaro Ikeda [:sotaro] from comment #23) > Following code does 0 division when the file is not mp3. It is also a > problem that MP3FrameParser is called even when the file is not mp3. By > changing the code locally, I confirmed the crash is fixed. And the mp3's > change also causes Bug 912829. Bug 919572 also causes youtube playback failure. But it seems different one from Bug 912829.
No longer blocks: 920831
Attached patch patch - Add zero division check (obsolete) (deleted) — Splinter Review
The patch fixes the crash, but youtube still can not be playbacked. mp3 parser doing parsing even when youtube is not mp3.
Assignee: nobody → sotaro.ikeda.g
Blocks: 919572
No longer depends on: 919572
Attachment #812759 - Attachment is obsolete: true
(In reply to Sotaro Ikeda [:sotaro] from comment #26) > Created attachment 812799 [details] [diff] [review] > patch - Add zero division check and prevent unncecessary mp3 parsing By the patch, youtube can be playbacked on master hamachi.
Comment on attachment 812799 [details] [diff] [review] patch - Add zero division check and prevent unncecessary mp3 parsing doublec, can you review the patch soon? This bug is top crash. Thanks.
Attachment #812799 - Flags: review?(chris.double)
blocking-b2g: 1.3? → 1.3+
Attachment #812799 - Flags: review?(chris.double) → review+
Attachment #812260 - Attachment is obsolete: true
Attachment #812702 - Attachment is obsolete: true
Attachment #812706 - Attachment is obsolete: true
Commitable patch. Carry "chris.double: review+".
Attachment #812799 - Attachment is obsolete: true
Attachment #812945 - Flags: review+
Attachment #812945 - Attachment description: patch v2 - Add zero division check and prevent unncecessary mp3 parsingpatch_922334_3 → patch v2 - Add zero division check and prevent unncecessary mp3 parsing
Keywords: checkin-needed
Status: NEW → RESOLVED
Closed: 11 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
Blocks: 919596
Committable patch for b2g v1.2.
Attachment #813618 - Flags: review+
This bug blocks ko1+ Bug 919596. Need to uplift of b2g v1.2.
blocking-b2g: 1.3+ → koi?
blocking-b2g: koi? → koi+
Keywords: checkin-needed
Blocks: 927884
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: