Fix for recursive notification postings crashing. Also fixed sizing i…

…ssues with tableview, including crashing when not in a superview. Closes cappuccino#238. Reviewed by me.
280north · Aug 25, 2009 · d1c0237 · d1c0237
1 parent abc1c6f
commit d1c0237
Show file tree

Hide file tree

Showing 3 changed files with 93 additions and 72 deletions.
diff --git a/AppKit/CPTableColumn.j b/AppKit/CPTableColumn.j
@@ -111,10 +111,18 @@ CPTableColumnUserResizingMask   = 2;
     var tableView = [self tableView];
 
     if (tableView)
+    {
+        var index = [[tableView tableColumns] indexOfObjectIdenticalTo:self];
+
+        tableView._dirtyTableColumnRangeIndex = tableView._dirtyTableColumnRangeIndex < 0 ? index : MIN(index,  tableView._dirtyTableColumnRangeIndex);
+
+        [tableView tile];
+
         [[CPNotificationCenter defaultCenter]
             postNotificationName:CPTableViewColumnDidResizeNotification
                           object:tableView
                         userInfo:[CPDictionary dictionaryWithObjects:[self, oldWidth] forKeys:[@"CPTableColumn", "CPOldWidth"]]];
+    }
 }
 
 - (float)width

diff --git a/AppKit/CPTableView.j b/AppKit/CPTableView.j
@@ -76,7 +76,7 @@ CPTableViewSolidHorizontalGridLineMask = 1 << 1;
 
 
 #define NUMBER_OF_COLUMNS() (_tableColumns.length)
-//#define ENSURE_COLUMN_WIDTHS()
+#define UPDATE_COLUMN_RANGES_IF_NECESSARY() if (_dirtyTableColumnRangeIndex !== CPNotFound) [self _recalculateTableColumnRanges];
 
 @implementation _CPTableDrawView : CPView
 {
@@ -118,6 +118,7 @@ CPTableViewSolidHorizontalGridLineMask = 1 << 1;
     CPInteger   _dirtyTableColumnRangeIndex;
     CPInteger   _numberOfHiddenColumns;
 
+    BOOL        _reloadAllRows;
     Object      _objectValues;
     CPRange     _exposedRows;
     CPIndexSet  _exposedColumns;
@@ -262,10 +263,11 @@ CPTableViewSolidHorizontalGridLineMask = 1 << 1;
     if (!_dataSource)
         return;
 
+    _reloadAllRows = YES;
     _objectValues = { };
 
+    // This updates the size too.
     [self noteNumberOfRowsChanged];
-    [self _sizeToParent];
 
     [self setNeedsLayout];
     [self setNeedsDisplay:YES];
@@ -721,8 +723,7 @@ CPTableViewSolidHorizontalGridLineMask = 1 << 1;
     if (aColumnIndex < 0 || aColumnIndex >= NUMBER_OF_COLUMNS())
         return _CGRectMakeZero();
 
-    if (_dirtyTableColumnRangeIndex !== CPNotFound)
-        [self _recalculateTableColumnRanges];
+    UPDATE_COLUMN_RANGES_IF_NECESSARY();
 
     var range = _tableColumnRanges[aColumnIndex];
 
@@ -806,8 +807,7 @@ CPTableViewSolidHorizontalGridLineMask = 1 << 1;
     if (!_CGRectContainsPoint(bounds, aPoint))
         return CPNotFound;
 
-    if (_dirtyTableColumnRangeIndex !== CPNotFound)
-        [self _recalculateTableColumnRanges];
+    UPDATE_COLUMN_RANGES_IF_NECESSARY();
 
     var x = aPoint.x,
         low = 0,
@@ -877,8 +877,7 @@ CPTableViewSolidHorizontalGridLineMask = 1 << 1;
 
     var superviewSize = [superview bounds].size;
 
-    if (_dirtyTableColumnRangeIndex !== CPNotFound)
-        [self _recalculateTableColumnRanges];
+    UPDATE_COLUMN_RANGES_IF_NECESSARY();
 
     var count = NUMBER_OF_COLUMNS();
 
@@ -894,8 +893,32 @@ CPTableViewSolidHorizontalGridLineMask = 1 << 1;
 {
     _numberOfRows = [_dataSource numberOfRowsInTableView:self];
 
+    [self tile];
+}
+
+- (void)tile
+{
+    UPDATE_COLUMN_RANGES_IF_NECESSARY();
+
+    // FIXME: variable row heights.
+    var width = _tableColumnRanges.length > 0 ? CPMaxRange([_tableColumnRanges lastObject]) : 0.0,
+        height = (_rowHeight + _intercellSpacing.height) * _numberOfRows,
+        superview = [self superview];
+
+    if ([superview isKindOfClass:[CPClipView class]])
+    {
+        var superviewSize = [superview bounds].size;
+
+        width = MAX(superviewSize.width, width);
+        height = MAX(superviewSize.height, height);
+    }
+
+    [self setFrameSize:_CGSizeMake(width, height)];
+
     [self setNeedsLayout];
+    [self setNeedsDisplay:YES];
 }
+
 /*
     * - tile
     * - sizeToFit
@@ -914,23 +937,6 @@ CPTableViewSolidHorizontalGridLineMask = 1 << 1;
     * - setAutosaveTableColumns:
 */
 
-- (void)_sizeToParent
-{
-    var superviewSize = [[self superview] bounds].size;
-
-    if (_dirtyTableColumnRangeIndex !== CPNotFound)
-        [self _recalculateTableColumnRanges];
-
-    if (_tableColumnRanges.length > 0)
-        var naturalWidth = CPMaxRange([_tableColumnRanges lastObject]);
-
-    else
-        var naturalWidth = 0.0;
-
-    [self setFrameSize:_CGSizeMake( MAX(superviewSize.width, naturalWidth),
-                                    MAX(superviewSize.height, (_rowHeight + _intercellSpacing.height) * _numberOfRows))];
-}
-
 //Setting the Delegate:(id)aDelegate
 
 - (void)setDelegate:(id)aDelegate
@@ -1125,18 +1131,22 @@ CPTableViewSolidHorizontalGridLineMask = 1 << 1;
 
 - (void)load
 {
-    if (!_dataSource)
-    {
-        // remove?
-        return;
-    }
-
 //    if (!window.blah)
 //        return window.setTimeout(function() { window.blah = true; [self load]; window.blah = false}, 0.0);
 
  //   if (window.console && window.console.profile)
  //       console.profile("cell-load");
 
+    if (_reloadAllRows)
+    {
+        [self _unloadDataViewsInRows:_exposedRows columns:_exposedColumns];
+
+        _exposedRows = [CPIndexSet indexSet];
+        _exposedColumns = [CPIndexSet indexSet];
+
+        _reloadAllRows = NO;
+    }
+
     var exposedRect = [self _exposedRect],
         exposedRows = [CPIndexSet indexSetWithIndexesInRange:[self rowsInRect:exposedRect]],
         exposedColumns = [self columnIndexesInRect:exposedRect],
@@ -1179,6 +1189,17 @@ CPTableViewSolidHorizontalGridLineMask = 1 << 1;
 //    [_tableDrawView setBounds:exposedRect];
     [_tableDrawView display];
 
+    // Now clear all the leftovers
+    // FIXME: this could be faster!
+    for (identifier in _cachedDataViews)
+    {
+        var dataViews = _cachedDataViews[identifier],
+            count = dataViews.length;
+
+        while (count--)
+            [dataViews[count] removeFromSuperview];
+    }
+
   //  if (window.console && window.console.profile)
 //        console.profileEnd("cell-load");
 }
@@ -1232,6 +1253,8 @@ _cachedDataViews[dataView.identifier].push(dataView);
     [rows getIndexes:rowArray maxCount:-1 inIndexRange:nil];
     [columns getIndexes:columnArray maxCount:-1 inIndexRange:nil];
 
+    UPDATE_COLUMN_RANGES_IF_NECESSARY();
+
     var columnIndex = 0,
         columnsCount = columnArray.length;
 
@@ -1318,7 +1341,8 @@ _cachedDataViews[dataView.identifier].push(dataView);
     var exposedRows = [self rowsInRect:aRect],
         firstRow = exposedRows.location,
         lastRow = CPMaxRange(exposedRows) - 1,
-        colorIndex = MIN(exposedRows.length, colorCount);
+        colorIndex = MIN(exposedRows.length, colorCount),
+        heightFilled = 0.0;
 
     while (colorIndex--)
     {
@@ -1526,11 +1550,7 @@ _cachedDataViews[dataView.identifier].push(dataView);
 
 - (void)superviewFrameChanged:(CPNotification)aNotification
 {
-    [self _sizeToParent];
-
-    // Call this explicitly because *our* size may not change, but the exposedRect may change.
-    [self setNeedsDisplay:YES];
-    [self setNeedsLayout];
+    [self tile];
 }
 
 //

diff --git a/Foundation/CPNotificationCenter.j b/Foundation/CPNotificationCenter.j
@@ -181,16 +181,20 @@ var _CPNotificationCenterPostNotification = function(/* CPNotificationCenter */
 @implementation _CPNotificationRegistry : CPObject
 {
     CPDictionary    _objectObservers;
-    BOOL            _observerRemoval;
-    CPArray         _postingObservers;
+    BOOL            _observerRemovalCount;
 }
 
 - (id)init
 {
+    self = [super init];
+
     if (self)
+    {
+        _observerRemovalCount = 0;
         _objectObservers = [CPDictionary dictionary];
+    }
 
-   return self;
+    return self;
 }
 
 -(void)addObserver:(_CPNotificationObserver)anObserver object:(id)anObject
@@ -208,9 +212,6 @@ var _CPNotificationCenterPostNotification = function(/* CPNotificationCenter */
         observers = [];
         [_objectObservers setObject:observers forKey:[anObject UID]];
     }
-
-    if (observers == _postingObservers)
-        _postingObservers = [observers copy];
 
     // Add this observer.    
     observers.push(anObserver);
@@ -235,10 +236,7 @@ var _CPNotificationCenterPostNotification = function(/* CPNotificationCenter */
             while (count--)
                 if ([observers[count] observer] == anObserver)
                 {
-                    _observerRemoval = YES;
-                    if (observers == _postingObservers)
-                        _postingObservers = [observers copy];
-
+                    ++_observerRemovalCount;
                     observers.splice(count, 1);
                 }
 
@@ -255,10 +253,7 @@ var _CPNotificationCenterPostNotification = function(/* CPNotificationCenter */
         while (count--)
             if ([observers[count] observer] == anObserver)
             {
-                _observerRemoval = YES;
-                if (observers == _postingObservers)
-                    _postingObservers = [observers copy];
-
+                ++_observerRemovalCount;
                 observers.splice(count, 1)
             }
 
@@ -282,47 +277,45 @@ var _CPNotificationCenterPostNotification = function(/* CPNotificationCenter */
     // However, this is a very expensive operation (O(N) => O(N^2)), so to avoid it,
     // we keep track of whether observers are added or removed, and only do our 
     // rigorous testing in those cases.
-    var object = [aNotification object];
-
-    if (object != nil && (_postingObservers = [_objectObservers objectForKey:[object UID]]))
+    var observerRemovalCount = _observerRemovalCount,
+        object = [aNotification object],
+        observers = nil;
+
+    if (object != nil && (observers = [[_objectObservers objectForKey:[object UID]] copy]))
     {
-        var observers = _postingObservers,
+        var currentObservers = observers,
             count = observers.length;
 
-        _observerRemoval = NO;
         while (count--)
         {
-            var observer = _postingObservers[count];
+            var observer = observers[count];
 
             // if there wasn't removal of an observer during this posting, or there 
             // was but we are still in the observer list... 
-            if (!_observerRemoval || [observers indexOfObjectIdenticalTo:observer] != CPNotFound)
+            if ((observerRemovalCount === _observerRemovalCount) || [currentObservers indexOfObjectIdenticalTo:observer] !== CPNotFound)
                 [observer postNotification:aNotification];
-
         }
     }
 
     // Now do the same for the nil object observers...
-    _postingObservers = [_objectObservers objectForKey:[[CPNull null] UID]];
-    
-    if (!_postingObservers)
+    observers = [[_objectObservers objectForKey:[[CPNull null] UID]] copy];
+
+    if (!observers)
         return;
-    
-    var observers = _postingObservers,
-        count = observers.length;
-
-    _observerRemoval = NO;
+
+    var observerRemovalCount = _observerRemovalCount,
+        count = observers.length,
+        currentObservers = observers;
+
     while (count--)
     {
-        var observer = _postingObservers[count];
-        
+        var observer = observers[count];
+
         // if there wasn't removal of an observer during this posting, or there 
         // was but we are still in the observer list... 
-        if (!_observerRemoval || [observers indexOfObjectIdenticalTo:observer] != CPNotFound)
+        if ((observerRemovalCount === _observerRemovalCount) || [currentObservers indexOfObjectIdenticalTo:observer] !== CPNotFound)
             [observer postNotification:aNotification];
     }
-
-    _postingObservers = nil;
 }
 
 - (unsigned)count