236313 - Mozilla crashes after clicking "SKYPASS" at Korean Air site

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect)

Description

[Victor Kazmirenko]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6) Gecko/20040113
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6) Gecko/20040113

Open http://www.koreanair.com/
At top of page there is link "SKYPASS". Click it. After opening this page
Mozilla crashes.


Reproducible: Always
Steps to Reproduce:
1.
2.
3.

Comment 1

[Olivier Cahagne]

confirming crash FF 20040302 Win2k, I get a popup R0625, Pure Virtual Call.

Comment 2

[Matthieu Beaumel]

Confirming "R0625, Pure Virtual Call".
This is on Gecko/20040113, Windows 2000.

However, the behaviour isn't very reproductible here. The amount of time before
the crash seems quit unconsistent.

Comment 3

[Rene Pronk]

Confirming. Could be a dupe of 236196 but it seems to be slightly different.

Comment 4

[PeteVine]

Also crashes on linux:
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7b) Gecko/20040227 Firefox/0.8.0+

Comment 5

[Rene Pronk]

Target All platforms based on #4

Comment 6

[Mats Palmgren (inactive)]

Attachment: stack

Comment 7

[Mats Palmgren (inactive)]

Bug occurs in 2003-09-03-05 trunk Linux, which is the oldest nightly build
I could find.  Bug does not occur in Mozilla 1.4 on Linux.

Comment 8

[Andrew Schultz]

Attachment: testcase

load the testcase.  reload a few times.  crash
tested trunk build 2004030308

<img src="javascript:SKchoosePhoto()" name="SKbnrPhoto">

SKchoosePhoto is:
  document.SKbnrPhoto.src="resource:///res/samples/Anieyes.gif";

Comment 9

[Andrew Schultz]

regrsession between linux trunk builds 2003071005 and 2003071122
bug 70030 or bug 212133?
I'll try backing stuff out.

Comment 10

[Andrew Schultz]

further testing just shows that bug 212133 just got the DOM working used in the
testcase, so no regression.

Comment 11

[Xanthor Sccas]

I can't reproduce it on linux with
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.6) Gecko/20040122 Epiphany/1.0.7
and
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7b) Gecko/20040301 Firefox/0.8.0+

(Neither the original page nor the testcase...)

Comment 12

[Matthias Versen [:Matti]]

*** Bug 236420 has been marked as a duplicate of this bug. ***

Comment 13

[Andrew Schultz]

*** Bug 236196 has been marked as a duplicate of this bug. ***

Comment 14

[tor]

Attachment: prevent img src collision

What appears to be happening here is that the src="javascript:..."
load comes in first and kicks off a load.  Deeper down we notice that
it's javascript and run it, which kicks off another load on the same
image, but now with the real URI.  But since the previous load is
still ongoing we pick the wrong request to use (current instead of
pending), which gets overwritten when processing returns to the first
load.

Comment 15

[tor]

Attachment: prevent img src collision

Ok, let's attach the patch and not the testcase this time...

Comment 16

[tor]

Attachment: modify both places with test

Comment 17

[Boris Zbarsky [:bzbarsky]]

Hmm.. so why is this actually crashing?  Are we double-releasing an
imgRequestProxy or something?

With that patch, won't

function foo() {
  img.src = "javascript:bar()";
}

function bar() {
  img.src = "http://whatever";
}

<img src="javascript:foo()">

still crash?  This time because mPendingRequest is clobbered?

Comment 18

[Boris Zbarsky [:bzbarsky]]

Oh, and no matter what that should be a PRPackedBool and live with the other
PRPackedBools.

Comment 19

[tor]

Attachment: double-indirect testcase

Comment 20

[tor]

Comment on attachment 143722 [details] [diff] [review]
modify both places with test

Yes, the double-indirect thing kills it.

Comment 21

[Isaac Hwak Han]

*** Bug 235843 has been marked as a duplicate of this bug. ***

Comment 22

[tor]

Attachment: different fix - need to discuss with pavlov

Comment 23

[Tom Graham]

*** Bug 240184 has been marked as a duplicate of this bug. ***

Comment 24

[Asa Dotzler [:asa]]

Tor, any progress here? This one seems like a good one to try to get for 1.7.

Comment 25

[Bernard Alleysson]

*** Bug 242334 has been marked as a duplicate of this bug. ***

Comment 26

[Stuart Parmenter]

Comment on attachment 145337 [details] [diff] [review]
different fix - need to discuss with pavlov

i ran with this patch and i can't find anything that breaks.. go for it

Comment 27

[Darin Fisher]

Comment on attachment 145337 [details] [diff] [review]
different fix - need to discuss with pavlov

you seem to be transforming the retval of imgILoader::LoadImage into an inout
param.	is that right?	if so, shouldn't you change the IDL accordingly?

Comment 28

[tor]

Attachment: add [inout] to IDL

Comment 29

[Darin Fisher]

I think we decided on IRC that there is no point to supporting [inout] on the
result of LoadImage (that all callers anyways just want the resulting value and
never pass anything but null).  so, how about just making sure that the
implementation of LoadImage sets *_retval to nsnull before doing anything else.
 that way, the fact that CreateNewProxyForRequest treats _retval as an inout
param will be inconsequential to the caller of LoadImage or LoadImageWithChannel.

Comment 30

[Darin Fisher]

To clarify my last comment: whatever we do for LoadImage needs to be done for
LoadImageWithChannel as well.

Comment 31

[tor]

Attachment: tweak per irc discussion

Comment 32

[Darin Fisher]

Comment on attachment 147664 [details] [diff] [review]
tweak per irc discussion

sr=darin

might be good to indicate with a comment the reason why it is significant that
*_retval be nulled out.

Comment 33

[tor]

Checked in on trunk with comments added.  Leaving open for baking.

Comment 34

[Michael Moy]

I noticed some strange image loading behaviour in a build that I did this morning.
My last build was on 5/3 and I didn't see the bahviour from that build.

I have a homepage with several small images on it which link to larger versions
of the images and they are all local to my hard drive. When I click on one of the
smaller images, the page area is blanked out (white) for a moment before the
image is displayed. Sometimes, this blank time is a fraction of a second and
sometimes it's as long as three seconds.

The image paints very quickly after the pause so it appears that something's going
on that's not related to image painting.

I also noticed this with the MozillaZine png at the top of the page though it
is very short there as the PNG is probably fairly small.

Someone pointed me to this checkin as a possibility.

Comment 35

[Tim Meader]

Note: the behavior described in #34 is not limited to pages with images. On an
internal site I'm developing (which uses only stylesheets) I've noticed that
between the 20040506 zip build and the 20040505 zip build (Windows Firefox),
many pages are MUCH slower on initial load. It seems once they're cached it's fine.

In particular, the pages w/ form controls of any kind, be it text entry boxes,
or form buttons, are noticeable... you can easily see the form component being
drawn before all other page content.

Should another bug be opened about the slow rendering?

Comment 36

[Michael Moy]

This checkin wasn't the cause. I backed it out, did a build and it's still there.
I'll file a bug for now.

Comment 37

[Michael Moy]

Filed bug http://bugzilla.mozilla.org/show_bug.cgi?id=242856 for the other problem that is NOT related to this bug.

Comment 38

[Mike Kaply [:mkaply]]

Comment on attachment 147664 [details] [diff] [review]
tweak per irc discussion

a=mkaply

Comment 39

[tor]

Checked in on branch.

Comment 40

[Karen Huang]

Verified as fix on latest 1.7 branch Win06-24, Mac06-30 & Linux06-29 builds.
Changing keywords from fixed1.7 to verified1.7.
Leave this bug status "as is" until this bug be verified on trunk again...