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

@@ -67,8 +67,6 @@
 #include <unistd.h>
 #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<nsIRenderingContext> 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<nsIRenderingContext> rendContext = getter_AddRefs(mGeckoChild->GetRenderingContext());
       mGeckoChild->UpdateWidget(r, rendContext);
-#ifdef USE_OPAQUE_VIEWS
-      [self flushRect:rects[i]];
-#endif
     }
   }
 }

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

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