Closed Bug 2535 Opened 26 years ago Closed 26 years ago

Widgets don't stop repeating when disposed->crash

Categories

(Core :: Layout: Form Controls, defect, P2)

PowerPC
All
defect

Tracking

()

VERIFIED FIXED

People

(Reporter: sfraser_bugs, Assigned: mikepinkerton)

Details

Reported by Stewart Rubenstein <sdr@camsoft.com>:

You get a repeatable crash if you close the window in viewer_debug (I
pulled this version down Tuesday evening).

I looked into this, and found that the nsTextWidgets for the location
and status were not being disposed of, so they never call
StopRepeating(), but the window and all the controls have gone away, so
when nsTextWidget::RepeatAction calls StartDraw(), Bad Things Happen.

I guess someone is holding a reference too long, or perhaps when the
window goes away, it has to make sure all the widgets have been
destroyed.  My next move would be to turn on MOZ_TRACE_XPCOM_REFCNT, but
I'm not sure where or how to do that.
OS: All
Funny: it confirms what I saw just 5 minutes ago (!) when switching from a page
to another one. It doesn't happen very often, though. Maybe you are already
running with the 0xEFEF stuff...

Mike, a thing to investigate is whether a widget should delete its children when
it's deleted itself. I'm not sure... Maybe.
I have found two problems, one I've got a fix for, and one I don't.
First is around line 430 of mozilla/gfx/src/mac/nsRenderingContextMac.cpp

      if (NS_SUCCEEDED(children->CurrentItem((nsISupports **)&child)))
      {
      	nsRect childRect;
      	Rect macRect;
      	child->GetBounds(childRect);
      	NS_RELEASE(child); // SDR 1/22/1999 Add missing release
      	::SetRect(&macRect, childRect.x, childRect.y, childRect.x +
childRect.width, childRect.y + childRect.height);
      	::RectRgn(childRgn, &macRect);
      	::DiffRgn(myRgn, childRgn, myRgn);
      }
	delete children;

The second is that the destructor for nsBaseWidget::Enumerator is not being
called in the last line of that snippet.  I added a missing "virtual" keyword to
its definition, but for some reason (maybe I need to do a more thorough
rebuilding) it still isn't called.

-Stew <sdr@camsoft.com>
I have found two problems, one I've got a fix for, and one I don't.
First is around line 430 of mozilla/gfx/src/mac/nsRenderingContextMac.cpp

      if (NS_SUCCEEDED(children->CurrentItem((nsISupports **)&child)))
      {
      	nsRect childRect;
      	Rect macRect;
      	child->GetBounds(childRect);
      	NS_RELEASE(child); // SDR 1/22/1999 Add missing release
      	::SetRect(&macRect, childRect.x, childRect.y, childRect.x +
childRect.width, childRect.y + childRect.height);
      	::RectRgn(childRgn, &macRect);
      	::DiffRgn(myRgn, childRgn, myRgn);
      }
	delete children;

The second is that the destructor for nsBaseWidget::Enumerator is not being
called in the last line of that snippet.  I added a missing "virtual" keyword to
its definition, but for some reason (maybe I need to do a more thorough
rebuilding) it still isn't called.

-Stew <sdr@camsoft.com>
Don't forget: we have 3 similar cases of scanning through the children in
nsWindow.cpp
Status: NEW → RESOLVED
Closed: 26 years ago
Resolution: --- → FIXED
Fixed the one here, and the three in nsWindow by using nsCOMPtr. Added a comment
thanking Stewart in nsRenderingContextMac.cpp
QA Contact: 3819
will mark verified assuming reporter agrees with fix
Status: RESOLVED → VERIFIED
didn't hear any complaining, so marking verified
Moving all Widget Set bugs, past and present, to new HTML Form Controls
component per request from karnaze.  Widget Set component will be retired
shortly.
You need to log in before you can comment on or make changes to this bug.