53501 - nsPresShell:EndLoad() called on wrong presShell loading a document

Status: NEW

(Core :: Layout, defect, P3)

Description

[Mike Pinkerton (not reading bugmail)]

- go to a long page, say your buglist
- click a link on there

nsScrollBoxFrame::SaveState() is called to save the scroll position. Note the 
presShell that is on the stack.

then nsScrollBoxFrame::RestoreState() is mysteriously called from
  nsDocument::EndLoad
  PresShell::EndLoad
  FrameMananager::RestoreFrameStateFor
and the PresShell on which the EndLoad is called is _the same_ as for the 
document you just left. Now that really can't be good. It appears EndLoad is 
being called for the wrong document/presShell

Who knows what bad things are caused because of this. It's mac-only, BTW.

Comment 1

[Mike Pinkerton (not reading bugmail)]

attn pdt: this should be a stop-ship. It's bad bad bad.

Comment 2

[Mike Pinkerton (not reading bugmail)]

this could also mean that we're leaking presShells/documents on mac. have you 
seen this beard?

Comment 3

[Mike Pinkerton (not reading bugmail)]

clayton shouldn't have this, i give it to.....nisheeth!

Comment 4

[Simon Fraser [no longer active]]

But why would this be mac-only? Does it happen in Modern skin too?

Comment 5

[Nisheeth Ranjan]

Nominating for rtm, marking nsbeta3+ and rtm+.  I doubt if this will get a PDT
nsbeta3++ marker, but it should hopefully qualify for a rtm++ marker.

Comment 6

[Michael La Guardia]

marking nsbeta3- as we won't be holding pr3 for this.

Comment 7

[Michael La Guardia]

Marking "needinfo". This seems like a bad thing, but we need a patch, review and
super review to clear for checkin.

Comment 8

[Nisheeth Ranjan]

Vidur checked in a fix to bugscape bug 2533 that fixed the timing of when end
load handlers are called.  Mike, please check if this is still happening in the
latest Mac rtm branch build.  If it is, please re-assign it to me and I'll take
a look.  Thanks.

Comment 9

[Mike Pinkerton (not reading bugmail)]

nope, it still calls endLoad on the presShell of the page you just came from
with a branch build from 10/2.

Comment 10

[Mike Pinkerton (not reading bugmail)]

since i seem to see this with bugzilla bug pages, and they have some strange
multi-part mime troubles, perhaps this is just a dupe of those bugs...

Comment 11

[Mike Pinkerton (not reading bugmail)]

yes, nisheeth and I verified this is only a problem with Bugzilla query pages
(probably because of the weird "multipart/x-mixed-replace", see bug #47350). We
also verified that the EndLoad is called on the correct presContext, in additon
to the wrong one, so nothing breaks, per se.

Since it's not as bad as we first thought, removing rtm nomination and
downgrading priority. We can ship with this.

Comment 12

[Nisheeth Ranjan]

Futuring.

Comment 13

[Steve Dagley]

Can we ship Mozilla0.9 with this?

Comment 14

[Nisheeth Ranjan]

Based on the analysis that pink and I did and our current bug lists, I think 
this bug will need to stay futured.  In what context are you seeing this bug, 
Steve?  If you have data that this bug is higher impact, we should re-evaluate.

Comment 15

[Boris Zbarsky [:bzbarsky]]

Is this still a problem?

Comment 16

[Nisheeth Ranjan]

I haven't looked this on a Mac for a long time.  Also, not sure if we've ever
seen whether this behavior happens on OSX.  I think pink was running OS 9 when
we tested for this bug back in early 2001.

I'll try to reproduce this on a recent OSX build and report back...

Comment 17

[Boris Zbarsky [:bzbarsky]]

Peter, can you reproduce this on OSX?