Closed
Bug 358723
Opened 18 years ago
Closed 18 years ago
Crashes [@ nsHTMLDocument::GetCookie]
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: smaug, Assigned: smaug)
Details
(Keywords: fixed1.8.1.1, Whiteboard: [need testcase])
Attachments
(1 file)
2.44 KB,
patch
|
bzbarsky
:
review+
bzbarsky
:
superreview+
dveditz
:
approval1.8.1.1+
|
Details | Diff | Splinter Review |
According to TB there are some FF2 crashes @ nsHTMLDocument::GetCookie. nsHTMLDocument::GetCookie [mozilla/content/html/document/src/nsHTMLDocument.cpp, line 1829] XPTC_InvokeByIndex [mozilla/xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp, line 102] XPCWrappedNative::CallMethod [mozilla/js/src/xpconnect/src/xpcwrappednative.cpp, line 2169] XPC_WN_GetterSetter [mozilla/js/src/xpconnect/src/xpcwrappednativejsops.cpp, line 1487] js_Invoke [mozilla/js/src/jsinterp.c, line 1377] I wonder in which case mPrincipal is null? http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/content/html/document/src/nsHTMLDocument.cpp&mark=1829&rev=MOZILLA_1_8_BRANCH#1829
Assignee | ||
Comment 1•18 years ago
|
||
Btw, most of the crash reports mention http://earth.google.com/earth.html or other google earth pages.
Comment 2•18 years ago
|
||
> I wonder in which case mPrincipal is null?
When it feels like -- mPrincipal is lazily allocated on the 1.8 branch. All consumers should be calling GetPrincipal(), then null-checking the result and throwing OOM if it's null.
Flags: blocking1.8.1.1?
Assignee | ||
Comment 3•18 years ago
|
||
(In reply to comment #2) > When it feels like -- mPrincipal is lazily allocated on the 1.8 branch. All > consumers should be calling GetPrincipal(), then null-checking the result and > throwing OOM if it's null. > Ah, ok. I can then take this and fix also other cases when principal isn't used properly.
Assignee: general → Olli.Pettay
Assignee | ||
Comment 4•18 years ago
|
||
(In reply to comment #2)then null-checking the result and > throwing OOM if it's null. > Not necessarily OOM, as far as I see. It may fail also during shutdown.
Assignee | ||
Comment 5•18 years ago
|
||
Like this?
Attachment #244094 -
Flags: superreview?(bzbarsky)
Attachment #244094 -
Flags: review?(bzbarsky)
Comment 6•18 years ago
|
||
Comment on attachment 244094 [details] [diff] [review] use GetPrincipal Looks great. r+sr=bzbarsky
Attachment #244094 -
Flags: superreview?(bzbarsky)
Attachment #244094 -
Flags: superreview+
Attachment #244094 -
Flags: review?(bzbarsky)
Attachment #244094 -
Flags: review+
Assignee | ||
Updated•18 years ago
|
Attachment #244094 -
Flags: approval1.8.1.1?
Assignee | ||
Comment 7•18 years ago
|
||
Note, this is not needed for trunk, because NodePrincipal() is used there.
Updated•18 years ago
|
Flags: blocking1.8.1.1? → blocking1.8.1.1+
Comment 8•18 years ago
|
||
Comment on attachment 244094 [details] [diff] [review] use GetPrincipal approved for 1.8 branch, a=dveditz for drivers
Attachment #244094 -
Flags: approval1.8.1.1? → approval1.8.1.1+
Assignee | ||
Updated•18 years ago
|
Keywords: fixed1.8.1.1
Comment 9•18 years ago
|
||
--> RESOLVED FIXED?
Assignee | ||
Comment 10•18 years ago
|
||
Yes, this should be fixed.
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Comment 11•18 years ago
|
||
Does anyone have a simplified testcase or steps to reproduce this crash? Would be nice to verify the fix...
Whiteboard: [need testcase]
Comment 12•18 years ago
|
||
All we have is talkback data...
You need to log in
before you can comment on or make changes to this bug.
Description
•