186509 - LocationImpl::GetURI should check nsDocShell::GetCurrentURI against null

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[timeless]

cvs build (lots of patches) from today

Ok, so here's the pair of outputs that i got:
WARNING: NS_ENSURE_TRUE(aURI) failed, file
i:/build/mozilla/docshell/base/nsDefaultURIFixup.cpp, line 58
************************************************************
* Call to xpconnect wrapped JSObject produced this error:  *
[Exception... "Component returned failure code: 0x80004003
(NS_ERROR_INVALID_POINTER) [nsIDOMLocation.href]"  nsresult: "0x80004003
(NS_ERROR_INVALID_POINTER)"  location: "JS frame ::
chrome://venkman/content/venkman-handlers.js :: con_winopen :: line 89"  data: no]

I decided to track down the null uri that was fed to nsDefaultURIFixup, here's
the stack:
nsDocShell::GetCurrentURI(nsDocShell * const 0x132f06e8, nsIURI * * 0x00128e84)
line 2800
LocationImpl::GetURI(nsIURI * * 0x00128f80) line 217 + 47 bytes
LocationImpl::GetHref(LocationImpl * const 0x1333db08, nsAString & {...}) line
400 + 32 bytes
XPTC_InvokeByIndex(nsISupports * 0x1333db08, unsigned int 9, unsigned int 1,
nsXPTCVariant * 0x0012912c) line 106
XPCWrappedNative::CallMethod(XPCCallContext & {...}, XPCWrappedNative::CallMode
CALL_GETTER) line 2016 + 42 bytes
XPCWrappedNative::GetAttribute(XPCCallContext & {...}) line 1880 + 14 bytes
XPC_WN_GetterSetter(JSContext * 0x11e37480, JSObject * 0x1740ea60, unsigned int
0, long * 0x174262b8, long * 0x001293e4) line 1324 + 12 bytes
js_Invoke(JSContext * 0x11e37480, unsigned int 0, unsigned int 2) line 839 + 23
bytes
js_InternalInvoke(JSContext * 0x11e37480, JSObject * 0x1740ea60, long 390130296,
unsigned int 0, unsigned int 0, long * 0x00000000, long * 0x00129cc8) line 931 +
20 bytes
js_GetProperty(JSContext * 0x11e37480, JSObject * 0x1740ea60, long 62771152,
long * 0x00129cc8) line 2548 + 45 bytes
js_Interpret(JSContext * 0x11e37480, long * 0x00129e48) line 2634 + 1785 bytes
js_Invoke(JSContext * 0x11e37480, unsigned int 3, unsigned int 2) line 856 + 13
bytes
nsXPCWrappedJSClass::CallMethod(nsXPCWrappedJSClass * const 0x0b7fe750,
nsXPCWrappedJS * 0x10d4e988, unsigned short 3, const nsXPTMethodInfo *
0x0b80d9c8, nsXPTCMiniVariant * 0x0012a38c) line 1200 + 22 bytes

mCurrentURI was null, and this is legal per
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/docshell/base/nsIWebNavigation.idl&rev=1.16&mark=190,192#189

Comment 1

[Harshal Pradhan]

Mine. Fallout of bug 179269.

Comment 2

[Harshal Pradhan]

Attachment: restore things to how they were for the null uri case

Simple null check. Make GetURI treal null like it did before 179269.

Comment 3

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment on attachment 109993 [details] [diff] [review]
restore things to how they were for the null uri case

>+  // Its valid for docshell to return a null uri, don't try fixup if
>+  // this happens.

For reference, the grammar nits I had were:

- "Its" as written is wrong.  It should be "It is" or the contracted form of
"It's" (with an apostrophy).
- URI is an acronym and should be capitalized.
- You have two complete statements and they should not run-on with a comma. 
Break them up with a period.
- Don't try _to_.



I think the comment would be better written as:

  // It's valid for docshell to not have a URI.  If that's the case,
  // don't worry about running it through the fixup object.

Comment 4

[Johnny Stenback (:jst)]

Comment on attachment 109993 [details] [diff] [review]
restore things to how they were for the null uri case

sr=jst

Comment 5

[Harshal Pradhan]

Checked in. (With comment wording changed as caillon suggested.)