129622 - Hang loading linamarengines.com

Status: RESOLVED FIXED

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

Description

[Blake Ross]

6.2 user feedback, still exists in current nightlies:

Loading linamarengines.com locks up the browser.

Comment 1

[Adam Hauner]

Attachment: HTML of title page

BTW inside is this HTML tag:
<frameset rows="0,*" frameborder=no border=0>
Should be related to bug 112570?

Comment 2

[Adam Hauner]

Attachment: Reduced testcase

Comment 3

[Adam Hauner]

Changing URL to hanging frame. IMHO something in JS.

Comment 4

[Chris Petersen]

Changing Priority to P2.

Comment 5

[Kevin McCluskey (gone)]

Hangs with 2002031803 build on WinXP

Comment 6

[Håkan Waara]

Here's the stack of the hang. Based on it, I think we can blame Image: Layout.
But Necko is also in there ... CC darin for opinion.

xptiInterfaceEntry::IsFullyResolved() line 555 + 33 bytes
xptiInterfaceEntry::GetMethodInfo(unsigned short 3, const nsXPTMethodInfo * *
0x0012daac) line 317 + 10 bytes
xptiInterfaceInfo::GetMethodInfo(xptiInterfaceInfo * const 0x03a6f3e0, unsigned
short 3, const nsXPTMethodInfo * * 0x0012daac) line 704 + 42 bytes
PrepareAndDispatch(nsXPTCStubBase * 0x0568da70, unsigned int 3, unsigned int *
0x0012db68, unsigned int * 0x0012db58) line 69
SharedStub() line 139
nsHttpHandler::OnModifyRequest(nsIHttpChannel * 0x0568cc30) line 613
nsHttpChannel::AsyncOpen(nsHttpChannel * const 0x0568cc30, nsIStreamListener *
0x0568d670, nsISupports * 0x00000000) line 2374 + 16 bytes
imgLoader::LoadImage(imgLoader * const 0x03110710, nsIURI * 0x0568c840, nsIURI *
0x00000000, nsILoadGroup * 0x032b0f50, imgIDecoderObserver * 0x0568be80,
nsISupports * 0x03c316b8, unsigned int 0, nsISupports * 0x00000000, imgIRequest
* 0x00000000, imgIRequest * * 0x0568be84) line 370 + 44 bytes
nsHTMLImageElement::SetSrcInner(nsIURI * 0x03ea2478, const nsAString & {...})
line 895 + 149 bytes
nsHTMLImageElement::SetSrc(nsHTMLImageElement * const 0x0568be74, const
nsAString & {...}) line 934 + 24 bytes
XPTC_InvokeByIndex(nsISupports * 0x0568be74, unsigned int 71, unsigned int 1,
nsXPTCVariant * 0x0012e2c4) line 106
XPCWrappedNative::CallMethod(XPCCallContext & {...}, XPCWrappedNative::CallMode
CALL_SETTER) line 1994 + 42 bytes
XPCWrappedNative::SetAttribute(XPCCallContext & {...}) line 1849 + 14 bytes
XPC_WN_GetterSetter(JSContext * 0x032ab290, JSObject * 0x055ef2e8, unsigned int
1, long * 0x03d14028, long * 0x0012e588) line 1290 + 12 bytes
js_Invoke(JSContext * 0x032ab290, unsigned int 1, unsigned int 2) line 788 + 23
bytes
js_InternalInvoke(JSContext * 0x032ab290, JSObject * 0x055ef2e8, long 60168224,
unsigned int 0, unsigned int 1, long * 0x0012eeb0, long * 0x0012eeb0) line 880 +
20 bytes
js_SetProperty(JSContext * 0x032ab290, JSObject * 0x055ef2e8, long 12183928,
long * 0x0012eeb0) line 2612 + 47 bytes
js_Interpret(JSContext * 0x032ab290, long * 0x0012f0bc) line 2585 + 1751 bytes
js_Execute(JSContext * 0x032ab290, JSObject * 0x03225f60, JSScript * 0x03cdf248,
JSStackFrame * 0x00000000, unsigned int 0, long * 0x0012f0bc) line 968 + 13 bytes
JS_EvaluateUCScriptForPrincipals(JSContext * 0x032ab290, JSObject * 0x03225f60,
JSPrincipals * 0x03cda8d8, const unsigned short * 0x03d26ef0, unsigned int 130,
const char * 0x0012f1d4, unsigned int 46, long * 0x0012f0bc) line 3379 + 25 bytes
nsJSContext::EvaluateString(nsJSContext * const 0x032ab0a8, const nsAString &
{...}, void * 0x03225f60, nsIPrincipal * 0x03cda8d4, const char * 0x0012f1d4,
unsigned int 46, const char * 0x00c80428, nsAString & {...}, int * 0x0012f120)
line 702 + 85 bytes
nsScriptLoader::EvaluateScript(nsScriptLoadRequest * 0x03cdf078, const
nsAFlatString & {...}) line 570
nsScriptLoader::ProcessRequest(nsScriptLoadRequest * 0x03cdf078) line 477 + 22 bytes
nsScriptLoader::ProcessScriptElement(nsScriptLoader * const 0x03da95a8,
nsIDOMHTMLScriptElement * 0x03d298b8, nsIScriptLoaderObserver * 0x03d298bc) line
420 + 15 bytes
nsHTMLScriptElement::SetDocument(nsHTMLScriptElement * const 0x03d29890,
nsIDocument * 0x03ee09f0, int 0, int 1) line 158
nsGenericHTMLContainerElement::AppendChildTo(nsGenericHTMLContainerElement *
const 0x03c45df8, nsIContent * 0x03d29890, int 0, int 0) line 4082
HTMLContentSink::ProcessSCRIPTTag(const nsIParserNode & {...}) line 4955
HTMLContentSink::AddLeaf(HTMLContentSink * const 0x03daa408, const nsIParserNode
& {...}) line 3257 + 12 bytes
CNavDTD::AddLeaf(const nsIParserNode * 0x03c4ba30) line 3804 + 25 bytes
CNavDTD::HandleScriptToken(const nsIParserNode * 0x03c4ba30) line 2266 + 12 bytes
CNavDTD::OpenContainer(const nsCParserNode * 0x03c4ba30, nsHTMLTag
eHTMLTag_script, int 1, nsEntryStack * 0x00000000) line 3455 + 12 bytes
CNavDTD::HandleDefaultStartToken(CToken * 0x03d3df28, nsHTMLTag eHTMLTag_script,
nsCParserNode * 0x03c4ba30) line 1343 + 20 bytes
CNavDTD::HandleStartToken(CToken * 0x03d3df28) line 1752 + 22 bytes
CNavDTD::HandleToken(CNavDTD * const 0x03e5cd28, CToken * 0x00000000, nsIParser
* 0x030082c8) line 908 + 12 bytes
CNavDTD::BuildModel(CNavDTD * const 0x03e5cd28, nsIParser * 0x030082c8,
nsITokenizer * 0x03b63318, nsITokenObserver * 0x00000000, nsIContentSink *
0x03daa408) line 519 + 20 bytes
nsParser::BuildModel() line 1870 + 34 bytes
nsParser::ResumeParse(int 1, int 0, int 1) line 1737 + 11 bytes
nsParser::OnDataAvailable(nsParser * const 0x030082cc, nsIRequest * 0x03c3a268,
nsISupports * 0x00000000, nsIInputStream * 0x03bb8728, unsigned int 0, unsigned
int 1315) line 2371 + 21 bytes
nsDocumentOpenInfo::OnDataAvailable(nsDocumentOpenInfo * const 0x03ce8628,
nsIRequest * 0x03c3a268, nsISupports * 0x00000000, nsIInputStream * 0x03bb8728,
unsigned int 0, unsigned int 1315) line 243 + 46 bytes
nsStreamListenerTee::OnDataAvailable(nsStreamListenerTee * const 0x03c452a0,
nsIRequest * 0x03c3a268, nsISupports * 0x00000000, nsIInputStream * 0x03cc2f58,
unsigned int 0, unsigned int 1315) line 97 + 51 bytes
nsHttpChannel::OnDataAvailable(nsHttpChannel * const 0x03c3a26c, nsIRequest *
0x03cc923c, nsISupports * 0x00000000, nsIInputStream * 0x03cc2f58, unsigned int
0, unsigned int 1315) line 2982 + 63 bytes
nsOnDataAvailableEvent::HandleEvent() line 193 + 70 bytes
nsARequestObserverEvent::HandlePLEvent(PLEvent * 0x03cdd3bc) line 116
PL_HandleEvent(PLEvent * 0x03cdd3bc) line 596 + 10 bytes
PL_ProcessPendingEvents(PLEventQueue * 0x00ac8c30) line 526 + 9 bytes
_md_EventReceiverProc(HWND__ * 0x00310202, unsigned int 49441, unsigned int 0,
long 11308080) line 1077 + 9 bytes
USER32! 77d33a5f()
USER32! 77d33b2e()
USER32! 77d33d6a()
USER32! 77d341fd()
nsAppShellService::Run(nsAppShellService * const 0x00b66c70) line 458
main1(int 1, char * * 0x002f7508, nsISupports * 0x00000000) line 1456 + 32 bytes
main(int 1, char * * 0x002f7508) line 1805 + 37 bytes
mainCRTStartup() line 338 + 17 bytes
KERNEL32! 77e7eb69()

Comment 7

[Håkan Waara]

Actually, I think the problem is that the executed JS hangs us, because I could
step up in the debugger all the way up to js_Invoke() where I always hung.

Here's the minimal stack I could get:

js_Invoke(JSContext * 0x037b9a88, unsigned int 1, unsigned int 2) line 788 + 23
bytes
js_InternalInvoke(JSContext * 0x037b9a88, JSObject * 0x0671b650, long 60677080,
unsigned int 0, unsigned int 1, long * 0x0012eeb0, long * 0x0012eeb0) line 880 +
20 bytes
js_SetProperty(JSContext * 0x037b9a88, JSObject * 0x0671b650, long 11963568,
long * 0x0012eeb0) line 2612 + 47 bytes
js_Interpret(JSContext * 0x037b9a88, long * 0x0012f0bc) line 2585 + 1751 bytes
js_Execute(JSContext * 0x037b9a88, JSObject * 0x034815a0, JSScript * 0x03d3bc20,
JSStackFrame * 0x00000000, unsigned int 0, long * 0x0012f0bc) line 968 + 13 bytes
JS_EvaluateUCScriptForPrincipals(JSContext * 0x037b9a88, JSObject * 0x034815a0,
JSPrincipals * 0x0391d3d8, const unsigned short * 0x03d3bd10, unsigned int 130,
const char * 0x0012f1d4, unsigned int 46, long * 0x0012f0bc) line 3379 + 25 bytes
nsJSContext::EvaluateString(nsJSContext * const 0x037b98a0, const nsAString &
{...}, void * 0x034815a0, nsIPrincipal * 0x0391d3d4, const char * 0x0012f1d4,
unsigned int 46, const char * 0x00c80428, nsAString & {...}, int * 0x0012f120)
line 702 + 85 bytes
nsScriptLoader::EvaluateScript(nsScriptLoadRequest * 0x03d3ba60, const
nsAFlatString & {...}) line 570
nsScriptLoader::ProcessRequest(nsScriptLoadRequest * 0x03d3ba60) line 477 + 22 bytes
nsScriptLoader::ProcessScriptElement(nsScriptLoader * const 0x03877e80,
nsIDOMHTMLScriptElement * 0x03d3b508, nsIScriptLoaderObserver * 0x03d3b50c) line
420 + 15 bytes
nsHTMLScriptElement::SetDocument(nsHTMLScriptElement * const 0x03d3b4e0,
nsIDocument * 0x03a48978, int 0, int 1) line 158
nsGenericHTMLContainerElement::AppendChildTo(nsGenericHTMLContainerElement *
const 0x03b54bd0, nsIContent * 0x03d3b4e0, int 0, int 0) line 4082
HTMLContentSink::ProcessSCRIPTTag(const nsIParserNode & {...}) line 4955
HTMLContentSink::AddLeaf(HTMLContentSink * const 0x03877c90, const nsIParserNode
& {...}) line 3257 + 12 bytes
CNavDTD::AddLeaf(const nsIParserNode * 0x03b54088) line 3804 + 25 bytes
CNavDTD::HandleScriptToken(const nsIParserNode * 0x03b54088) line 2266 + 12 bytes
CNavDTD::OpenContainer(const nsCParserNode * 0x03b54088, nsHTMLTag
eHTMLTag_script, int 1, nsEntryStack * 0x00000000) line 3455 + 12 bytes
CNavDTD::HandleDefaultStartToken(CToken * 0x0391c0b8, nsHTMLTag eHTMLTag_script,
nsCParserNode * 0x03b54088) line 1343 + 20 bytes
CNavDTD::HandleStartToken(CToken * 0x0391c0b8) line 1752 + 22 bytes
CNavDTD::HandleToken(CNavDTD * const 0x03b916a8, CToken * 0x00000000, nsIParser
* 0x03877988) line 908 + 12 bytes
CNavDTD::BuildModel(CNavDTD * const 0x03b916a8, nsIParser * 0x03877988,
nsITokenizer * 0x03bd0a20, nsITokenObserver * 0x00000000, nsIContentSink *
0x03877c90) line 519 + 20 bytes
nsParser::BuildModel() line 1870 + 34 bytes
nsParser::ResumeParse(int 1, int 0, int 1) line 1737 + 11 bytes
nsParser::OnDataAvailable(nsParser * const 0x0387798c, nsIRequest * 0x03a49850,
nsISupports * 0x00000000, nsIInputStream * 0x03b30800, unsigned int 0, unsigned
int 1315) line 2371 + 21 bytes
nsDocumentOpenInfo::OnDataAvailable(nsDocumentOpenInfo * const 0x03a49d98,
nsIRequest * 0x03a49850, nsISupports * 0x00000000, nsIInputStream * 0x03b30800,
unsigned int 0, unsigned int 1315) line 243 + 46 bytes

Comment 8

[Håkan Waara]

As far as I can see, we're looping forever in js_Interpret() (jsinterp.c) ... 
going to "advance_pc:" (line #3845) all the time.

Comment 9

[Håkan Waara]

The for-loop in the testcase's loadRandomImage() is for some reason in Mozilla 
never exiting (like in IE) ...

Comment 10

[Håkan Waara]

OK, so here's what I think is the problem:

	for(i=1;i<=this.num;i++){
		i = new Image();
		i.src=this.directory+i+".gif";
	}

the (undeclared) variable `i' is used *both* as an iterator for the for-loop, 
and for creating a new Image() in. So first `i' is assigned 1, then a new Image
(), then the second time the loop is evaluated the image ([object 
HTMLImageElement()]) is tested against a number.

So basically the page author screwed up the variables. But IE can cope with 
this, and we should never hang.

Does this make sense to anyone? Maybe some JSEng guru can chime in here.

Comment 11

[Håkan Waara]

Unsetting Future milestone for re-evaluation for the new assignee of this bug.

Comment 12

[Phil Schwartau]

[MID-AIR COLLISION]

The problem is an infinite loop in this function:

function loadRandomImages(){

  for(i=1;i<=this.num;i++) {
    i = new Image();
    i.src=this.directory+i+".gif";
  }
  
  this.images_have_loaded = 1;
}


At the top of the loop, |this.num| is, say, 3. Notice how the body
of the loop keeps setting |i| to a new Image() element. When we test
the condition |i<=3|, this is always evaluating to true in Mozilla.
Therefore the loop continues forever -

Obviously this is a coding error at the site, in their file
view-source:http://www.linamarengines.com/scripts/rotating_menus.js

But reassigning to DOM Level 0, because this site loads fine in IE.
Is it because IE uses a different property of Image elements when
it makes the comparison to the number 3?  Or is this just Evangelism?

I will attach a testcase below that may help. Try it in Mozilla, IE.

Comment 13

[Phil Schwartau]

Attachment: Further reduced testcase: try in NN4.7, IE, vs. Mozilla

Comment 14

[Håkan Waara]

This is not evangelism because we shouldn't hang, regardless of the bad web author.

Comment 15

[Håkan Waara]

Thanks for the testcase.

In IE6:
i = [object]
i <= 2 evaluates to: false

in Mozilla:
i = [object HTMLImageElement]
i <= 2 evaluates to: true

Comment 16

[Tim]

Perhaps appropriate behavior would be for any comparison to null or void to
invalidate the whole comparison (as opposed to assuming the undefined value is
0), thus invariably returning false in a boolean context?

Comment 17

[Tim]

Simple verification procedure: in the JS console, eval the following:
var a = new Image(); [ a < 1, a == 1, a > 1, a <= 1, Number(a) ]
 (returns "false, false, false, true, 1")

That last part is what kills this script: objects always resolve to 1 numerically.

Comment 18

[Johnny Stenback (:jst)]

IIRC an object's default to number conversion returns 1 per ECMA so Mozilla does
the right thing here. But from looking at the code XPConnect is the code that
does the conversion from XPConnect wrapped object to number
(XPC_WN_Shared_Convert()). That, and the fact that new Object() != 1 suggest
that maybe my memory doesn't serve me correctly here. brendan, jband, dbradley,
comments?

Comment 19

[Johnny Stenback (:jst)]

... and:

  javascript:alert(1 + new Number(new Object()));

gives "NaN", and:

  javascript:alert(1 + new Number(new Image()));

gives "2"...

Comment 20

[Håkan Waara]

So basically it boils down to, what should we do about this:

if (NaN < 1)

IMHO, NaN should mark the test invalid and therefore return false.

Comment 21

[Alex Vincent [:WeirdAl]]

ECMAScript 3rd Edition, section 11.8.1:

...
5. Perform the comparison Result(2) < Result(4). (Section 11.8.5.)
6. If Result(5) is undefined, return false. Otherwise, return Result(5).

Section 11.8.5:
1. Call ToPrimitive(x, hint Number).
2. Call ToPrimitive(y, hint Number).
3. If Type(Result(1)) is String and Type(Result(2)) is String, go to step 16.
(Note that this step differs from step 7 in
the algorithm for the addition operator + in using and instead of or.)
4. Call ToNumber(Result(1)).
5. Call ToNumber(Result(2)).
6. If Result(4) is NaN, return undefined.
7. If Result(5) is NaN, return undefined.
...

In this context, either Result(4) or Result(5) in 11.8.5 ends up undefined. 
Therefore, 11.8.1 returns false.  Our browser's behavior on NaN < 1 is correct.

Comment 22

[Tim]

The main issue seems to be that Number( new Image() ) is not NaN, but 1 (try
it).  The proper result of NaN <,<=,==,>=,> (finite number) is undefined and
therefore false.  If our behavior was proper, the loop condition would be
immediately false and the loop would never happen.  Obviously, the place to look
is where Image produces its default value.  Note that Number( new Object() ) is
fine (NaN).

RE: comment 18, ECMA-262 does not mention an Image object.  I'm not sure where
we get that, but my guess is DOM.  Thus ECMA does not define what a primitive
(numerical) representation of Image should be.

Comment 23

[Johnny Stenback (:jst)]

The number one comes from
http://lxr.mozilla.org/seamonkey/source/js/src/xpconnect/src/xpcwrappednativejsops.cpp#528

Comment 24

[Johnny Stenback (:jst)]

Attachment: Make Number(new Image()) convert to NaN

Comment 25

[Johnny Stenback (:jst)]

Comment on attachment 89030 [details] [diff] [review]
Make Number(new Image()) convert to NaN

jband says sr=jband

Comment 26

[Johnny Stenback (:jst)]

dbradley, r=?

Comment 27

[David Bradley]

Comment on attachment 89030 [details] [diff] [review]
Make Number(new Image()) convert to NaN

r=dbradley

Comment 28

[Håkan Waara]

FWIW, you have my r=hwaara

Comment 29

[Johnny Stenback (:jst)]

Fix checked in.