177369 - 10.2: crash after accessing a downloadable link [QDIsPortBuffered]

Status: VERIFIED FIXED

(Camino Graveyard :: Downloading, defect)

Description

[sairuh (rarely reading bugmail)]

found using 2002.10.29.04 on 10.2.1. i'm not sure if this is really a dup of bug
172381 (which was dup'd in favor of bug 157157), as the stack is a bit different.

anyhow, steps:

1. go to a site which has downloadable links, such as
ftp://ftp.mozilla.org/pub/chimera/nightly/2002-10-29-04/

2. single-click the Chimera.dmg.gz link.

results: crash --the file picker doesn't even appear.

note, however, that this is NOT a problem if i bring up the context menu for the
link and select "download link target".

crash report coming up.

Comment 1

[sairuh (rarely reading bugmail)]

Attachment: crash report

Comment 2

[sairuh (rarely reading bugmail)]

fwiw, i'm unable to repro this with 2002.10.25.14 on 10.2.1. gonna see if this
is a more recent regression.

Comment 3

[sairuh (rarely reading bugmail)]

argh, i take back my comment 2.

the second attempt at accessing a downloadable link resulted in a crash --but at
a somewhat different step: i got the file picker (unlike comment 0), but the
crash occurred as soon as i selected a different folder from the droplist.

the stack trace looks the same, too.

Comment 4

[Simon Fraser [no longer active]]

I can repro this.

Comment 5

[sairuh (rarely reading bugmail)]

this regressed btwn the 10.25.04 (works fine) and 10.25.14 (crashes) builds.

also, i cannot repro this on 10.1.5, with the same steps and builds.

Comment 6

[Simon Fraser [no longer active]]

Things I have found(tm)
1. nsChildView::GetNativeData() sometimes passes back NULL GrafPtrs for views
   that have not been placed in the window yet (but this seems unrelated
   to the crash).

2. The GetParent() that was hacked to always return nil, except for plugins, is
   called mostly from 2 places: SetZIndex() calls that deal with child widget
   layering, and event handling code. Putting that hack back makes the crash
   go away, as before.

3. When the crash occurs, GetQDGlobalsThePort() returns the address of a port
   that belongs to a view that has gone away. We've done lots of other port-
   setting in the meantime, so it's not obvious how QD held onto this port.

Comment 7

[Simon Fraser [no longer active]]

Hrm. So NSQuickDraw views have _qdPort and _savePort members. When they are
deleted, they call SetPort() with the _savePort. Every time gecko loads a new
page, two views go away (at autorelease time) in this order:

view 1 dealloc (port A, saveport B); SetPort(B)
view 2 dealloc (port C, saveport A): SetPort(A) // whoops

I've yet to determine where _savePort gets set.

Comment 8

[Simon Fraser [no longer active]]

Attachment: Patch to nsChildView.mm

This patch fixes the bug by setting the _savePort member of the NSQuickDraw
view (our superclass) to a valid port, when the view is about to be removed
from the window. Then, at dealloc time, the superclass will set the current
port to the window's port, which is safe.

Comment 9

[Simon Fraser [no longer active]]

Attachment: Same patch with more context

This patch shows the location of the main change, in [NSView
viewWillMoveToWindow], which called with newWindow == NULL when a view is
removed from the visible hierarchy.

Comment 10

[Simon Fraser [no longer active]]

This fix may also fix bug 165929, bug 177338, bug 163464, and bug 167312. I
checked it in.

Comment 11

[Mike Pinkerton (not reading bugmail)]

+    WindowRef nativeWindow = windowToWindowRef([self window]);
+    _savePort = ::GetWindowPort(nativeWindow);

i'm worried that windowToWindowRef doesn't exist on 10.1. does it?

Comment 12

[Simon Fraser [no longer active]]

windowToWindowRef is our function that wraps the jaguar/10.1 specific calls, so
it works on both.

Comment 13

[Chris Petersen]

I can't reproduce issue using the 2002-10-30-04 build under 10.2.1. Tested with
two different macs : Power Mac Dual 800mhz (g4) and iBook 500mhz. Sarah, do you
still see this problem ?

Comment 14

[sairuh (rarely reading bugmail)]

i cannot repro this using 2002.10.30.04 on 10.2.1 (500mhz G4). marking verified.

Comment 15

[Chris Petersen]

Based on Comment #10, I can still reporoduce 165929, 163464, and 167312 in the
latest build (2002-10-30-04)