Closed Bug 131725 Opened 23 years ago Closed 23 years ago

Javascript and named frames crash bug; M099 [@ LocationImpl::GetProtocol]

Categories

(Core :: DOM: Navigation, defect)

Product:
Core
Component:
DOM: Navigation
Platform:
x86
All
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla1.0

People

(Reporter: abs, Assigned: jst)

References

(
URL
)

Details

(Keywords: crash, topcrash)

Crash Data

Attachments

(5 files)

WinNT stack trace
5.04 KB, text/plain
Details
this is a band aid
505 bytes, patch
Details | Diff | Splinter Review
Patch checks for nsnull
1.26 KB, patch
Brade
: review+
darin.moz
: superreview+
Details | Diff | Splinter Review
Fix crash w/o throwing an exception
8.29 KB, patch
Details | Diff | Splinter Review
diff -w of the above changes
7.50 KB, patch
adamlock
: review+
darin.moz
: superreview+
asa
: approval+
Details | Diff | Splinter Review
We have a dynamo based publishing system which uses frames and javascript. Mozilla has worked fine up to and including 0.9.8. New with 0.9.9 it crashes mozilla on login. This makes mozilla unususable for us. I have distilled down a javascript/frames fragment which reproduces the problem. Its available at http://www.mono.org/abs/bang/, or as a tarfile of the frames at http://www.mono.org/abs/bang/mozilla_bang.tar
Confirmed, build 2002-03-17-21 on Linux SuSE 7.3 Talkback id: TB4176863G
Severity: major → critical
Status: UNCONFIRMED → NEW
Ever confirmed: true
I see this crash with 0.9.9 on Linux. I do _not_ see a crash with the 2002-03-18-06 trunk nightly on Linux.
Severity: critical → major
stephend, could you get the stack? TB4176863G
Here it is: Incident ID 4176863 Stack Signature LocationImpl::GetProtocol() 310c2cd2 Trigger Time 2002-03-18 09:25:03 Email Address mats@symsoft.se URL visited http://www.mono.org/abs/bang/ Build ID 2002031721 Product ID MozillaTrunk Platform Operating System LinuxIntel Module Trigger Reason SIGSEGV: Segmentation Fault: (signal 11) User Comments Bugzilla bug 131725 Stack Trace LocationImpl::GetProtocol() XPTC_InvokeByIndex() XPCWrappedNative::CallMethod() XPC_WN_GetterSetter() js_Invoke() js_InternalInvoke() js_GetProperty() js_Interpret() js_Execute() JS_EvaluateUCScriptForPrincipals() nsJSContext::EvaluateString() nsScriptLoader::EvaluateScript() nsScriptLoader::ProcessRequest() nsScriptLoader::ProcessScriptElement() nsHTMLScriptElement::SetDocument() nsGenericHTMLContainerElement::AppendChildTo() HTMLContentSink::ProcessSCRIPTTag() HTMLContentSink::AddLeaf() CNavDTD::AddLeaf() CNavDTD::HandleScriptToken() CNavDTD::OpenContainer() CNavDTD::HandleDefaultStartToken() CNavDTD::HandleStartToken() CNavDTD::HandleToken() CNavDTD::BuildModel() nsParser::BuildModel() nsParser::ResumeParse() nsParser::OnDataAvailable() nsDocumentOpenInfo::OnDataAvailable() nsStreamListenerTee::OnDataAvailable() nsHttpChannel::OnDataAvailable() nsOnDataAvailableEvent::HandleEvent() nsARequestObserverEvent::HandlePLEvent() PL_HandleEvent() PL_ProcessPendingEvents() nsEventQueueImpl::ProcessPendingEvents() event_processor_callback() our_gdk_io_invoke() libglib-1.2.so.0 + 0xf3b0 (0x403903b0) libglib-1.2.so.0 + 0x10c46 (0x40391c46) libglib-1.2.so.0 + 0x11273 (0x40392273) libglib-1.2.so.0 + 0x1143c (0x4039243c) libgtk-1.2.so.0 + 0x9276c (0x402aa76c) nsAppShell::Run() nsAppShellService::Run() main1() main() libc.so.6 + 0x1d7ee (0x404df7ee)
Severity: major → critical
Keywords: crash
I can reproduce this crash simply by loading the given URL in my debug WinNT build 2002-03-14. Will attach stack trace below. OS: NetBSD ---> All
OS: NetBSD → All
Attached file WinNT stack trace —
Not sure what this stack trace indicates. Provisionally assigning to XPConnect for help; cc'ing jband. This is an easy crash to reproduce. Is this JS Engine or XPConnect-related, do you think? Here is the parent HTML page: <HTML> <FRAMESET rows="*,*"> <FRAME src="broken_js.html"> <FRAME name=STIM_Preview src="blank.html"> </FRAMESET> </HTML> Here is broken_js.html: <HTML> <BODY> <SCRIPT language=JavaScript1.1> parent.top.STIM_Preview.location.href = parent.top.STIM_Preview.location.protocol + "//" + parent.top.STIM_Preview.location.host + parent.top.STIM_Preview.location.pathname; </SCRIPT> </BODY> </HTML>
Assignee: rogerl → dbradley
Component: JavaScript Engine → XPConnect
Assignee: dbradley → adamlock
Component: XPConnect → Embedding: Docshell
QA Contact: pschwartau → adamlock
Summary: Javascript and named frames crash bug → Javascript and named frames crash bug [@LocationImpl::GetProtocol]
###!!! ASSERTION: You can't dereference a NULL nsCOMPtr with operator->().: 'mRawPtr != 0', file ../../../dist/include/xpcom/nsCOMPtr.h, line 650 this assert comes from uri (surprise). Going backwards, i jumped to 645 result = GetURI(getter_AddRefs(uri)); /*where uri is set*/ 647 if (NS_SUCCEEDED(result)) { 648 nsCAutoString protocol; 649 650 result = uri->GetScheme(protocol); /*where the assert is*/ And this is where i ended up: 2482 nsDocShell::GetCurrentURI(nsIURI ** aURI) 2483 { 2484 NS_ENSURE_ARG_POINTER(aURI); 2485 2486 *aURI = mCurrentURI; 2487 NS_IF_ADDREF(*aURI); 2488 2489 return NS_OK; 2490 } 2491 (gdb) p mCurrentURI $10 = {mRawPtr = 0x0} (gdb) where #0 nsDocShell::GetCurrentURI (this=0x9e48600, aURI=0xbfbfd0d0) at /home/timeless/mozilla/docshell/base/nsDocShell.cpp:2487 #1 0x29857d39 in LocationImpl::GetURI (this=0x9ea5b00, aURI=0xbfbfd0d0) at /home/timeless/mozilla/dom/src/base/nsLocation.cpp:242 #2 0x29859c42 in LocationImpl::GetProtocol (this=0x9ea5b00, aProtocol=@0xbfbfd2d4) at /home/timeless/mozilla/dom/src/base/nsLocation.cpp:645 #3 0x282fa68c in XPTC_InvokeByIndex (that=0x9ea5b00, methodIndex=15, paramCount=1, params=0xbfbfd368) at /home/timeless/mozilla/xpcom/reflect/xptcall/src/md/unix/xptcinvoke_unixish_x86.cpp:153 I don't think we should return NS_OK if the getter fails [nor did the LocationImpl coders]
fwiw i'm on freebsd
the solution to this bug depends on the how nsIDocShell is meant to work... if it is OK for currentURI to return NULL, then LocationImpl should work around this behavior, possibly returning an error from GetURI when the URI is NULL.
taking this pill allows me to load the page, this is what venkman gives me: function (null)() in <http://www.mono.org/abs/bang/broken_js.html> line 4 Continuing from thrown exception. 002: <BODY> 003: <SCRIPT language=JavaScript1.1> 004: parent.top.STIM_Preview.location.href = parent.top.STIM_Preview.location.protocol + "// 005: + parent.top.STIM_Preview.location.host 006: + parent.top.STIM_Preview.location.pathname; presumably it's not really what we want for final. i wonder if the actual problem is a race condition where the other frame hasn't loaded yet so location isn't available for reading.
currentURI is a readonly attribute on nsIWebNavigation that *can* be nsnull. For example, a newly created docshell has no current uri. The interface documentation could be a little clearer but I see nothing wrong with the current implementation. Therefore callers should test the value is non-null in addition or instead of looking at the nsresult if they intend to use it. I will do a simple patch for nsLocation.cpp and make the documentation more clear.
Patch adds a check for nsnull inside LocationImpl::GetURI. A null value makes this method return NS_ERROR_FAILURE.
Comment on attachment 74969 [details] [diff] [review] Patch checks for nsnull sr=darin
Attachment #74969 - Flags: superreview+
Comment on attachment 74969 [details] [diff] [review] Patch checks for nsnull r=brade
Attachment #74969 - Flags: review+
Comment on attachment 74969 [details] [diff] [review] Patch checks for nsnull This doesn't seem like the right thing to do to me, just because there's no current URI doesn't mean that we should throw an exception in the callers face, in stead we should just return no URI, or default to about:blank or something in the DOM code. I'm working on a patch for this...
Attachment #74969 - Flags: needs-work+
Comment on attachment 75011 [details] [diff] [review] diff -w of the above changes look great, sr=darin though we should also be sure to land the documentation change to nsIDocShell::currentURI.
Attachment #75011 - Flags: superreview+
topcrash on M099; added M099 to summary for tracking
Keywords: topcrash
Summary: Javascript and named frames crash bug [@LocationImpl::GetProtocol] → Javascript and named frames crash bug; M099 [@LocationImpl::GetProtocol]
Comment on attachment 75011 [details] [diff] [review] diff -w of the above changes r=adamlock I am concerned that GetWritableURI will return NS_OK when it fails, but the logic for handling the return value looks ok.
Comment on attachment 75011 [details] [diff] [review] diff -w of the above changes r=adamlock I am concerned that GetWritableURI will return NS_OK when it fails, but the logic for handling the return value looks ok.
Attachment #75011 - Flags: review+
Adam, wanna land this one? If not, hand it over to me and I'll do it...
Reassigning. I've asked Asa for the a= and I've checked in the tweak to the currentURI comment nsIWebNavigation.idl
Assignee: adamlock → jst
Status: NEW → ASSIGNED
Keywords: mozilla1.0, nsbeta1
Target Milestone: --- → mozilla1.0
Comment on attachment 75011 [details] [diff] [review] diff -w of the above changes a=asa (on behalf of drivers) for checkin to the 1.0 trunk
Attachment #75011 - Flags: approval+
Fixed.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Verifying fixed. This last crashed with build 2002031910...no Talkback incidents since then.
Status: RESOLVED → VERIFIED
Summary: Javascript and named frames crash bug; M099 [@LocationImpl::GetProtocol] → Javascript and named frames crash bug; M099 [@ LocationImpl::GetProtocol]
Crash Signature: [@ LocationImpl::GetProtocol]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: