From 5c408157621ccd5be8fb02027784e945e5283382 Mon Sep 17 00:00:00 2001 From: "smfr%smfr.org" Date: Wed, 13 Apr 2005 04:51:36 +0000 Subject: [PATCH] Fix bug 277923: crash on scrolling in Camino on page load, because we destroy the scrollbars while in a native scrollbar tracking loop. Fixed by posting a fake mouseup event when we detect that the scrollbar is going away. r=pinkerton. Only affects Camino, so no need for approval. Also remove the USE_OPAQUE_VIEWS #ifdefs from nsChildView, and add an explanatory comment now that we understand the issue better. --- widget/src/cocoa/nsChildView.mm | 28 ++++--- widget/src/cocoa/nsNativeScrollbar.h | 7 ++ widget/src/cocoa/nsNativeScrollbar.mm | 105 +++++++++++++++++++++++--- 3 files changed, 114 insertions(+), 26 deletions(-) diff --git a/widget/src/cocoa/nsChildView.mm b/widget/src/cocoa/nsChildView.mm index a1481cc9815..da3a3b86315 100644 --- a/widget/src/cocoa/nsChildView.mm +++ b/widget/src/cocoa/nsChildView.mm @@ -67,8 +67,6 @@ #include #include "nsCursorManager.h" -// #define USE_OPAQUE_VIEWS 1 - #define NSAppKitVersionNumber10_2 663 // category of NSView methods to quiet warnings @@ -2262,6 +2260,11 @@ nsChildView::Idle() // an empty fallback port. } +- (NSString*)description +{ + return [NSString stringWithFormat:@"ChildView %p, gecko child %p, frame %@", self, mGeckoChild, NSStringFromRect([self frame])]; +} + // Find the nearest scrollable view for this ChildView // (recall that views are not refcounted) - (nsIScrollableView*) getScrollableView @@ -2385,16 +2388,17 @@ nsChildView::Idle() // -isOpaque // -// XXXdwh. Quickdraw views are transparent by default. Since Gecko does its own blending if/when -// opacity is specified, we would like to optimize here by turning off the transparency of the view. -// But we can't. :( +// NSQuickDrawViews do not correctly update if opaque, because of a known incompatibility +// between the way that NSQuickDrawView is implemented, and the NSWindow update mechanism. +// This is unlikely to change in future. +// +// It's unfortunate, because it's expensive to redraw every parent view when updating +// a portion of any given NSQDView. However, there is no efficient workaround. See +// bug 166932. +// - (BOOL)isOpaque { -#ifdef USE_OPAQUE_VIEWS - return YES; -#else return mIsPluginView; -#endif } -(void)setIsPluginView:(BOOL)aIsPlugin @@ -2487,9 +2491,6 @@ nsChildView::Idle() ConvertCocoaToGeckoRect(aRect, r); nsCOMPtr rendContext = getter_AddRefs(mGeckoChild->GetRenderingContext()); mGeckoChild->UpdateWidget(r, rendContext); -#ifdef USE_OPAQUE_VIEWS - [self flushRect:aRect]; -#endif } // If >10.3, only paint the sub-rects that need it. This avoids the // nasty coalesced updates that result in big white areas. @@ -2502,9 +2503,6 @@ nsChildView::Idle() ConvertCocoaToGeckoRect(rects[i], r); nsCOMPtr rendContext = getter_AddRefs(mGeckoChild->GetRenderingContext()); mGeckoChild->UpdateWidget(r, rendContext); -#ifdef USE_OPAQUE_VIEWS - [self flushRect:rects[i]]; -#endif } } } diff --git a/widget/src/cocoa/nsNativeScrollbar.h b/widget/src/cocoa/nsNativeScrollbar.h index d8495243a71..645f7744b7b 100644 --- a/widget/src/cocoa/nsNativeScrollbar.h +++ b/widget/src/cocoa/nsNativeScrollbar.h @@ -64,6 +64,8 @@ public: nsNativeScrollbar(); virtual ~nsNativeScrollbar(); + NS_IMETHOD Destroy(); + NS_DECL_ISUPPORTS_INHERITED NS_DECL_NSINATIVESCROLLBAR @@ -110,6 +112,9 @@ private: // the nsNativeScrollbar that created this view. It retains this NSView, so // the link back to it must be weak. [WEAK] nsNativeScrollbar* mGeckoChild; + + // YES when we're in a tracking loop + BOOL mInTracking; } // default initializer @@ -117,6 +122,8 @@ private: // overridden parent class initializer - (id)initWithFrame:(NSRect)frameRect; +- (void)scrollbarDestroyed; + - (IBAction)scroll:(NSScroller*)sender; @end diff --git a/widget/src/cocoa/nsNativeScrollbar.mm b/widget/src/cocoa/nsNativeScrollbar.mm index d9f0dab1035..7d1396eb166 100644 --- a/widget/src/cocoa/nsNativeScrollbar.mm +++ b/widget/src/cocoa/nsNativeScrollbar.mm @@ -76,6 +76,12 @@ nsNativeScrollbar::~nsNativeScrollbar() { } +NS_IMETHODIMP +nsNativeScrollbar::Destroy() +{ + [mView scrollbarDestroyed]; + return NS_OK; +} // // CreateCocoaView @@ -105,7 +111,7 @@ nsNativeScrollbar::GetQuickDrawPort ( ) // pray we're always a child of a NSQuickDrawView if ( [mParentView isKindOfClass: [ChildView class]] ) { NSQuickDrawView* parent = NS_STATIC_CAST(NSQuickDrawView*, mParentView); - return [parent qdPort]; + return (GrafPtr)[parent qdPort]; } return nsnull; @@ -531,15 +537,15 @@ nsNativeScrollbar::IsEnabled(PRBool *aState) // - (id)initWithFrame:(NSRect)frameRect geckoChild:(nsNativeScrollbar*)inChild { - [super initWithFrame:frameRect]; - - NS_ASSERTION(inChild, "Need to provide a tether between this and a nsChildView class"); - mGeckoChild = inChild; - - // make ourselves the target of the scroll and set the action message - [self setTarget:self]; - [self setAction:@selector(scroll:)]; - + if ((self = [super initWithFrame:frameRect])) + { + NS_ASSERTION(inChild, "Need to provide a tether between this and a nsChildView class"); + mGeckoChild = inChild; + + // make ourselves the target of the scroll and set the action message + [self setTarget:self]; + [self setAction:@selector(scroll:)]; + } return self; } @@ -552,7 +558,10 @@ nsNativeScrollbar::IsEnabled(PRBool *aState) - (id)initWithFrame:(NSRect)frameRect { NS_WARNING("You're calling the wrong initializer. You really want -initWithFrame:geckoChild"); - return [self initWithFrame:frameRect geckoChild:nsnull]; + if ((self = [self initWithFrame:frameRect geckoChild:nsnull])) + { + } + return self; } @@ -601,6 +610,34 @@ nsNativeScrollbar::IsEnabled(PRBool *aState) // do nothing } +// +// -scrollbarDestroyed +// +// the gecko nsNativeScrollbar is being destroyed. +// +- (void)scrollbarDestroyed +{ + mGeckoChild = nsnull; + + if (mInTracking) + { + // To get out of the NSScroller tracking loop, we post a fake mouseup event. + // We have to do this here, before we are ripped out of the view hierarchy. + [NSApp postEvent:[NSEvent mouseEventWithType:NSLeftMouseUp + location:[NSEvent mouseLocation] + modifierFlags:0 + timestamp:[[NSApp currentEvent] timestamp] + windowNumber:[[self window] windowNumber] + context:NULL + eventNumber:0 + clickCount:0 + pressure:1.0] + atStart:YES]; + + mInTracking = NO; + } +} + // // -scroll @@ -616,5 +653,51 @@ nsNativeScrollbar::IsEnabled(PRBool *aState) } +// +// -trackKnob: +// +// overridden to toggle mInTracking on and off +// +- (void)trackKnob:(NSEvent *)theEvent +{ + mInTracking = YES; + NS_DURING // be sure we always turn mInTracking off. + [super trackKnob:theEvent]; + NS_HANDLER + NS_ENDHANDLER + mInTracking = NO; +} + +// +// -trackScrollButtons: +// +// overridden to toggle mInTracking on and off +// +- (void)trackScrollButtons:(NSEvent *)theEvent +{ + mInTracking = YES; + NS_DURING // be sure we always turn mInTracking off. + [super trackScrollButtons:theEvent]; + NS_HANDLER + NS_ENDHANDLER + mInTracking = NO; +} + +// +// -trackPagingArea: +// +// this method is not documented, but was seen in sampling. We have +// to override it to toggle mInTracking. +// +- (void)trackPagingArea:(id)theEvent +{ + mInTracking = YES; + NS_DURING // be sure we always turn mInTracking off. + [super trackPagingArea:theEvent]; + NS_HANDLER + NS_ENDHANDLER + mInTracking = NO; +} + @end