Bug 682445 - Don't copy a C++ object into the block; instead, hold on to the NSEvent object which will be memory-managed by the block appropriately. r=smichaud
authorMarkus Stange <mstange@themasta.com>
Thu, 08 Sep 2011 15:31:09 +0200
changeset 76729 2bce36a017afe60c242322753f7f3e84fab21bbf
parent 76728 a8a9ade7fefb035cb485ca735ddcd441ea2bb53b
child 76730 8948238c6e6adf1d076cbd315ea5c6f736de298b
push id3
push userfelipc@gmail.com
push dateFri, 30 Sep 2011 20:09:13 +0000
reviewerssmichaud
bugs682445
milestone9.0a1
Bug 682445 - Don't copy a C++ object into the block; instead, hold on to the NSEvent object which will be memory-managed by the block appropriately. r=smichaud
widget/src/cocoa/nsChildView.mm
--- a/widget/src/cocoa/nsChildView.mm
+++ b/widget/src/cocoa/nsChildView.mm
@@ -3066,38 +3066,32 @@ NSEvent* gLastDragMouseDownEvent = nil;
   // problems with vertical scrolls (by minimizing the possibility that they'll
   // be misinterpreted as horizontal swipes), while still tolerating a small
   // vertical element to a true horizontal swipe.  The number '8' was arrived
   // at by trial and error.
   if ((deltaX == 0) || (fabs(deltaX) <= fabs(deltaY) * 8)) {
     return;
   }
 
-  // geckoEvent must be initialized now (while anEvent is still available),
-  // but we also need to access it (and modify it) in the following "block"
-  // (the trackingHandler passed to [NSEvent trackSwipeEventWithOptions:...]).
-  // Normally we'd give it the '__block' keyword, but this makes the compiler
-  // crash :-(  Without the '__block' keyword, it becomes immutable from
-  // trackingHandler.  So trackingHandler must make a copy of it and modify
-  // that.
-  nsSimpleGestureEvent geckoEvent(PR_TRUE, NS_SIMPLE_GESTURE_SWIPE, mGeckoChild, 0, 0.0);
-  [self convertCocoaMouseEvent:anEvent toGeckoEvent:&geckoEvent];
-
   __block BOOL animationCancelled = NO;
   // At this point, anEvent is the first scroll wheel event in a two-finger
   // horizontal gesture that we've decided to treat as a swipe.  When we call
   // [NSEvent trackSwipeEventWithOptions:...], the OS interprets all
   // subsequent scroll wheel events that are part of this gesture as a swipe,
   // and stops sending them to us.  The OS calls the trackingHandler "block"
   // multiple times, asynchronously (sometimes after [NSEvent
   // maybeTrackScrollEventAsSwipe:...] has returned).  The OS determines when
   // the gesture has finished, and whether or not it was "successful" -- this
   // information is passed to trackingHandler.  We must be careful to only
   // call [NSEvent maybeTrackScrollEventAsSwipe:...] on a "real" swipe --
   // otherwise two-finger scrolling performance will suffer significantly.
+  // Note that we use anEvent inside the block. This extends the lifetime of
+  // the anEvent object because it's retained by the block, see bug 682445.
+  // The block will release it when the block goes away at the end of the
+  // animation, or when the animation is canceled.
   [anEvent trackSwipeEventWithOptions:0
              dampenAmountThresholdMin:-1
                                   max:1
                          usingHandler:^(CGFloat gestureAmount, NSEventPhase phase, BOOL isComplete, BOOL *stop) {
       if (animationCancelled) {
         *stop = YES;
         return;
       }
@@ -3107,23 +3101,24 @@ NSEvent* gLastDragMouseDownEvent = nil;
       // gestureAmount is negative when it will be '-1' at isComplete, and
       // positive when it will be '1'.  And phase is never equal to
       // NSEventPhaseEnded when gestureAmount will be '0' at isComplete.
       // Not waiting until isComplete is TRUE substantially reduces the
       // time it takes to change pages after a swipe, and helps resolve
       // bug 678891.
       if (phase == NSEventPhaseEnded) {
         if (gestureAmount) {
-          nsSimpleGestureEvent geckoEventCopy(geckoEvent);
+          nsSimpleGestureEvent geckoEvent(PR_TRUE, NS_SIMPLE_GESTURE_SWIPE, mGeckoChild, 0, 0.0);
+          [self convertCocoaMouseEvent:anEvent toGeckoEvent:&geckoEvent];
           if (gestureAmount > 0) {
-            geckoEventCopy.direction |= nsIDOMSimpleGestureEvent::DIRECTION_LEFT;
+            geckoEvent.direction |= nsIDOMSimpleGestureEvent::DIRECTION_LEFT;
           } else {
-            geckoEventCopy.direction |= nsIDOMSimpleGestureEvent::DIRECTION_RIGHT;
+            geckoEvent.direction |= nsIDOMSimpleGestureEvent::DIRECTION_RIGHT;
           }
-          mGeckoChild->DispatchWindowEvent(geckoEventCopy);
+          mGeckoChild->DispatchWindowEvent(geckoEvent);
         }
         mSwipeAnimationCancelled = nil;
       } else if (phase == NSEventPhaseCancelled) {
         mSwipeAnimationCancelled = nil;
       }
     }];
 
   // We keep a pointer to the __block variable (animationCanceled) so we