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)
Tracking
()
People
(Reporter: gbennett, Assigned: sotaro)
References
()
Details
(4 keywords)
Crash Data
Attachments
(2 files, 6 obsolete files)
(deleted),
patch
|
sotaro
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
sotaro
:
review+
|
Details | Diff | Splinter Review |
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
Updated•11 years ago
|
Component: Gaia::Browser → Video/Audio
Product: Boot2Gecko → Core
Version: unspecified → Trunk
Updated•11 years ago
|
blocking-b2g: koi? → 1.3?
Updated•11 years ago
|
Blocks: b2g-central-dogfood
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
Comment 3•11 years ago
|
||
Analyzing the push log, the only patch candidates that landed since 9/27 that look related to this crash were implemented by kinetik.
Assignee | ||
Comment 4•11 years ago
|
||
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
Assignee | ||
Comment 5•11 years ago
|
||
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
Assignee | ||
Comment 6•11 years ago
|
||
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
Updated•11 years ago
|
Keywords: regressionwindow-wanted
Comment 7•11 years ago
|
||
(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?
Comment 8•11 years ago
|
||
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.
Comment 9•11 years ago
|
||
I doubt that YouTube's mobile site is attempting to use privileged B2G APIs, so I'd guess this is a Gaia problem somehow.
Updated•11 years ago
|
Assignee | ||
Comment 10•11 years ago
|
||
(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.
Comment 11•11 years ago
|
||
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.
Comment 12•11 years ago
|
||
(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.
Assignee | ||
Comment 13•11 years ago
|
||
When the crash happen, stack overflow seems to happen.
Comment 14•11 years ago
|
||
(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.
Comment 15•11 years ago
|
||
If this doesn't reproduce with the dom.mozInputMethod.testing pref set to true, then that suggests bug 920831 isn't at fault.
Assignee | ||
Comment 16•11 years ago
|
||
On nexus4, I could get meaningful stacktrace.
Assignee | ||
Comment 17•11 years ago
|
||
At first, I got a change list by uisng git log. I used it to check which change made regression.
Assignee | ||
Comment 18•11 years ago
|
||
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
Assignee | ||
Updated•11 years ago
|
Attachment #812649 -
Attachment is obsolete: true
Assignee | ||
Comment 19•11 years ago
|
||
It is not clear why master hamachi did not crash around the change in Comment 18.
Comment 20•11 years ago
|
||
Does the patch from bug 920831 fix this?
Comment 21•11 years ago
|
||
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.)
Comment 22•11 years ago
|
||
(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.
Assignee | ||
Comment 23•11 years ago
|
||
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
Assignee | ||
Comment 24•11 years ago
|
||
(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.
Assignee | ||
Comment 25•11 years ago
|
||
The patch fixes the crash, but youtube still can not be playbacked. mp3 parser doing parsing even when youtube is not mp3.
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → sotaro.ikeda.g
Updated•11 years ago
|
Assignee | ||
Comment 26•11 years ago
|
||
Attachment #812759 -
Attachment is obsolete: true
Assignee | ||
Comment 27•11 years ago
|
||
(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.
Assignee | ||
Comment 28•11 years ago
|
||
Assignee | ||
Comment 29•11 years ago
|
||
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)
Updated•11 years ago
|
blocking-b2g: 1.3? → 1.3+
Updated•11 years ago
|
Attachment #812799 -
Flags: review?(chris.double) → review+
Assignee | ||
Updated•11 years ago
|
Attachment #812260 -
Attachment is obsolete: true
Assignee | ||
Updated•11 years ago
|
Attachment #812702 -
Attachment is obsolete: true
Assignee | ||
Updated•11 years ago
|
Attachment #812706 -
Attachment is obsolete: true
Assignee | ||
Comment 30•11 years ago
|
||
Commitable patch. Carry "chris.double: review+".
Attachment #812799 -
Attachment is obsolete: true
Attachment #812945 -
Flags: review+
Assignee | ||
Updated•11 years ago
|
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
Assignee | ||
Updated•11 years ago
|
Keywords: checkin-needed
Comment 31•11 years ago
|
||
Comment 32•11 years ago
|
||
Status: NEW → RESOLVED
Closed: 11 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
Assignee | ||
Comment 33•11 years ago
|
||
Committable patch for b2g v1.2.
Attachment #813618 -
Flags: review+
Assignee | ||
Comment 34•11 years ago
|
||
This bug blocks ko1+ Bug 919596. Need to uplift of b2g v1.2.
blocking-b2g: 1.3+ → koi?
Updated•11 years ago
|
blocking-b2g: koi? → koi+
Assignee | ||
Updated•11 years ago
|
status-b2g-v1.2:
--- → affected
status-firefox26:
--- → affected
Assignee | ||
Updated•11 years ago
|
Assignee | ||
Updated•11 years ago
|
Keywords: checkin-needed
Comment 35•11 years ago
|
||
status-firefox25:
--- → wontfix
Keywords: checkin-needed
You need to log in
before you can comment on or make changes to this bug.
Description
•