Closed
Bug 1108721
Opened 9 years ago
Closed 9 years ago
xul!mozilla::dom::WrapNewBindingObjectHelper<nsRefPtr<mozilla::dom::SpeechGrammarList>,1>::Wrap (NULL Pointer Dereference)
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
VERIFIED
FIXED
mozilla37
People
(Reporter: vulnerable.zappa, Assigned: smaug)
References
Details
(4 keywords)
Crash Data
Attachments
(3 files)
4.14 KB,
application/x-zip-compressed
|
Details | |
1.70 KB,
text/html
|
Details | |
6.27 KB,
patch
|
peterv
:
review+
Sylvestre
:
approval-mozilla-aurora-
Sylvestre
:
approval-mozilla-beta+
bkerensa
:
approval-mozilla-esr31+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:34.0) Gecko/20100101 Firefox/34.0 Build ID: 20141125180439 Steps to reproduce: 1. Open ff33.html as local file (remote was not check) Actual results: Firefox crash Expected results: Nothing
Updated•9 years ago
|
Component: Untriaged → DOM
Product: Firefox → Core
Comment 2•9 years ago
|
||
This reproduces on Nightly as well.
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows 7 → All
Hardware: x86_64 → All
Updated•9 years ago
|
Crash Signature: [@ mozilla::dom::WrapNewBindingObjectHelper<nsRefPtr<nsSVGElement>, int>::Wrap(JSContext*, nsRefPtr<nsSVGElement> const&, JS::MutableHandle<JS::Value>)]
A null pointer where it shouldn't be, perhaps?
Possibly from the early return at http://mxr.mozilla.org/mozilla-central/source/dom/html/TextTrackManager.cpp#98.
Comment 6•9 years ago
|
||
Comment 7•9 years ago
|
||
Comment 4 is spot on: WARNING: NS_ENSURE_TRUE(scriptObject || !hasHadScriptObject) failed: file ../../../mozilla/dom/html/TextTrackManager.cpp, line 98 So the TextTrackManager ctor never sets mTextTracks, which ends up null, and then HTMLMediaElement:::TextTracks() returns null but the IDL for it promises it will never do that so the bindings don't null-check. Looks like a regression from bug 950308.
Blocks: 950308
Flags: needinfo?(rick.eyre)
Flags: needinfo?(bzbarsky)
Flags: needinfo?(bugs)
Keywords: regression
Assignee | ||
Comment 8•9 years ago
|
||
Hmm, I guess we could use Document::GetScopeObject here. Still a bit fragile, since GetScopeObject can in theory return null.
Assignee: nobody → bugs
Flags: needinfo?(bugs)
Assignee | ||
Comment 9•9 years ago
|
||
yeah, that is not enough. Need to make some things nullable in .webidl
Assignee | ||
Comment 10•9 years ago
|
||
I think we should do this for now. Making GetScopeObject() to never return null if it has once returned non-null would be way more risky change, not anything for branches.
Attachment #8545315 -
Flags: review?(peterv)
Comment 11•9 years ago
|
||
Comment on attachment 8545315 [details] [diff] [review] patch Review of attachment 8545315 [details] [diff] [review]: ----------------------------------------------------------------- I'm a bit torn on whether this should actually throw or return null, but this is less riskier. Eventually we should be able to undo this, hopefully we will remember to when GetParentObject never returns null on nodes.
Attachment #8545315 -
Flags: review?(peterv) → review+
Assignee | ||
Comment 12•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/fc33533af56c
Flags: needinfo?(rick.eyre)
Assignee | ||
Comment 13•9 years ago
|
||
Comment on attachment 8545315 [details] [diff] [review] patch [Approval Request Comment] If this is not a sec:{high,crit} bug, please state case for ESR consideration: User impact if declined: Crashes Fix Landed on Version: 37 Risk to taking this patch (and alternatives if risky): Shouldn't be risky String or UUID changes made by this patch: NA See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info. Approval Request Comment [Feature/regressing bug #]: [User impact if declined]: bug 950308 [Describe test coverage new/current, TBPL]: the feature itself is tested by several mochitests, but this crash corner case indeed would require a crashtest. [Risks and why]: Shouldn't be risky, effectively a null check [String/UUID change made/needed]: NA
Attachment #8545315 -
Flags: approval-mozilla-esr31?
Attachment #8545315 -
Flags: approval-mozilla-beta?
Attachment #8545315 -
Flags: approval-mozilla-aurora?
Comment 14•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/fc33533af56c
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
Updated•9 years ago
|
status-firefox36:
--- → affected
status-firefox37:
--- → fixed
status-firefox38:
--- → fixed
status-firefox-esr31:
--- → affected
Comment 15•9 years ago
|
||
Comment on attachment 8545315 [details] [diff] [review] patch aurora is 37, beta is 36. So, aurora is already fixed.
Attachment #8545315 -
Flags: approval-mozilla-beta?
Attachment #8545315 -
Flags: approval-mozilla-beta+
Attachment #8545315 -
Flags: approval-mozilla-aurora?
Attachment #8545315 -
Flags: approval-mozilla-aurora-
Assignee | ||
Comment 16•9 years ago
|
||
Sheriff, the patch should apply to branches with fuzz and the usual dom/foo -> content/bar rename.
Comment 17•9 years ago
|
||
https://hg.mozilla.org/releases/mozilla-beta/rev/5fba52895751
status-firefox38:
fixed → ---
Comment 18•9 years ago
|
||
Is there a bug already about making GetScopeObject not return null?
Flags: needinfo?(bugs)
Assignee | ||
Comment 19•9 years ago
|
||
I thought I had filed that, but apparently not. Bug 1122041.
Flags: needinfo?(bugs)
Updated•9 years ago
|
tracking-firefox-esr31:
--- → 36+
Updated•9 years ago
|
Attachment #8545315 -
Flags: approval-mozilla-esr31? → approval-mozilla-esr31+
Comment 21•9 years ago
|
||
I was able to reproduce the issue in Firefox 34.0.5 using scenario from comment 0 on Windows 7 x64. Firefox no longer crashes in the same scenario in Firefox 36 Beta 8, latest Aurora and latest Nightly.
Status: RESOLVED → VERIFIED
status-firefox38:
--- → verified
Comment 22•9 years ago
|
||
Reproduced the original issue using the following build: * http://ftp.mozilla.org/pub/mozilla.org/firefox/releases/34.0.5/win32/en-US/ ** https://crash-stats.mozilla.com/report/index/36a1ae14-8489-48bb-8be5-61e062150218 Went through verification using the following build: * http://ftp.mozilla.org/pub/mozilla.org/firefox/candidates/31.5.0esr-candidates/build2/ Test Cases: * Opened the POC from comment #0 several different times in normal tabs/windows * Opened the POC from comment #0 several different times in private tabs/windows
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•