From 9ed30f563adc97943117754806001badf4314f43 Mon Sep 17 00:00:00 2001 From: Alexandre Wilhelm Date: Tue, 19 Aug 2014 14:26:26 -0700 Subject: [PATCH 01/11] Fixed: Methods viewDidMoveToSuperview viewDidMoveToWindow viewWillMoveToSuperview viewWillMoveToWindow not called as in Cocoa Previously, when adding, removing, replacing a CPView, the framework didn't call the methods viewDidMoveToSuperview, viewDidMoveToWindow, viewWillMoveToSuperview and viewWillMoveToWindow as in Cocoa. Now it does. The main change is that these methods will be called when removing a CPView. Added unit-test in Tests/AppKit/CPViewTest.j --- AppKit/CPView.j | 49 +++++---- Tests/AppKit/CPViewTest.j | 212 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 243 insertions(+), 18 deletions(-) diff --git a/AppKit/CPView.j b/AppKit/CPView.j index 5e1357ccf..d0517df4b 100644 --- a/AppKit/CPView.j +++ b/AppKit/CPView.j @@ -508,6 +508,9 @@ var CPViewFlags = { }, [CPException raise:CPInvalidArgumentException reason:"can't insert a subview in duplicate (probably partially decoded)"]; #endif + // Notify the subview that it will be moving. + [aSubview viewWillMoveToSuperview:self]; + // We will have to adjust the z-index of all views starting at this index. var count = _subviews.length; @@ -537,14 +540,13 @@ var CPViewFlags = { }, } else { + var superview = aSubview._superview; + // Remove the view from its previous superview. - [aSubview removeFromSuperview]; + [aSubview _removeFromSuperview]; - // Set the subview's window to our own. - [aSubview _setWindow:_window]; - - // Notify the subview that it will be moving. - [aSubview viewWillMoveToSuperview:self]; + if (superview) + [aSubview _setWindow:nil]; // Set ourselves as the superview. aSubview._superview = self; @@ -577,6 +579,12 @@ var CPViewFlags = { }, if (![aSubview isHidden] && [self isHiddenOrHasHiddenAncestor]) [aSubview _notifyViewDidHide]; + [aSubview viewDidMoveToSuperview]; + + // Set the subview's window to our own. + if (_window) + [aSubview _setWindow:_window]; + // This method might be called before we are fully unarchived, in which case the theme state isn't set up yet // and none of the below matters anyhow. if (_themeState) @@ -592,8 +600,6 @@ var CPViewFlags = { }, [aSubview _notifyWindowDidResignKey]; } - [aSubview viewDidMoveToSuperview]; - [self didAddSubview:aSubview]; } @@ -610,6 +616,18 @@ var CPViewFlags = { }, Does nothing if there's no container view. */ - (void)removeFromSuperview +{ + var superview = _superview; + + [self viewWillMoveToSuperview:nil]; + [self _removeFromSuperview]; + [self viewDidMoveToSuperview]; + + if (superview) + [self _setWindow:nil]; +} + +- (void)_removeFromSuperview { if (!_superview) return; @@ -634,8 +652,6 @@ var CPViewFlags = { }, [self _notifyViewDidResignFirstResponder]; _superview = nil; - - [self _setWindow:nil]; } /*! @@ -645,14 +661,14 @@ var CPViewFlags = { }, */ - (void)replaceSubview:(CPView)aSubview with:(CPView)aView { - if (aSubview._superview !== self) + if (aSubview._superview !== self || aSubview === aView) return; var index = [_subviews indexOfObjectIdenticalTo:aSubview]; - [aSubview removeFromSuperview]; - [self _insertSubview:aView atIndex:index]; + + [aSubview removeFromSuperview]; } - (void)setSubviews:(CPArray)newSubviews @@ -727,9 +743,6 @@ var CPViewFlags = { }, /* @ignore */ - (void)_setWindow:(CPWindow)aWindow { - if (_window === aWindow) - return; - [[self window] _dirtyKeyViewLoop]; // Clear out first responder if we're the first responder and leaving. @@ -2936,10 +2949,10 @@ setBoundsOrigin: - (BOOL)unsetThemeState:(ThemeState)aState { - if (aState && aState.isa && [aState isKindOfClass:CPArray]) + if (aState && aState.isa && [aState isKindOfClass:CPArray]) aState = CPThemeState.apply(null, aState); - var oldThemeState = _themeState + var oldThemeState = _themeState; _themeState = _themeState.without(aState); if (oldThemeState === _themeState) diff --git a/Tests/AppKit/CPViewTest.j b/Tests/AppKit/CPViewTest.j index 509664c4b..49857032c 100644 --- a/Tests/AppKit/CPViewTest.j +++ b/Tests/AppKit/CPViewTest.j @@ -4,14 +4,33 @@ [CPApplication sharedApplication] +var methodCalled; + @implementation CPViewTest : OJTestCase { CPView view; + CPView view1; + CPView view2; + CPView view3; + + CPWindow window; } - (void)setUp { + window = [[CPWindow alloc] initWithContentRect:CGRectMake(0.0, 0.0, 1000.0, 1000.0) styleMask:CPWindowNotSizable]; + view = [[CPView alloc] initWithFrame:CGRectMakeZero()]; + view1 = [[CPResponderView alloc] initWithFrame:CGRectMakeZero()]; + view2 = [[CPResponderView alloc] initWithFrame:CGRectMakeZero()], + view3 = [[CPResponderView alloc] initWithFrame:CGRectMakeZero()], + + [view1 setIdentifier:@"view1"]; + [view2 setIdentifier:@"view2"]; + [view3 setIdentifier:@"view3"]; + + methodCalled = []; + [super setUp]; } @@ -446,10 +465,203 @@ [self assertFalse:[subview hasThemeState:CPThemeStateFirstResponder]]; } +- (void)testWhenAddedSubviewMethodCalled +{ + var expectedRestult = [@"viewWillMoveToSuperview_view1", @"viewDidMoveToSuperview_view1", "viewWillMoveToWindow_view1", "viewDidMoveToWindow_view1"]; + + [[window contentView] addSubview:view1]; + + [self assert:expectedRestult equals:methodCalled]; +} + +- (void)testWhenAddedSubviewWithoutWindowMethodCalled +{ + var expectedRestult = [@"viewWillMoveToSuperview_view2", @"viewDidMoveToSuperview_view2"]; + + [view1 addSubview:view2]; + + [self assert:expectedRestult equals:methodCalled]; +} + +- (void)testWhenAddedSubviewTwiceMethodCalled +{ + var expectedRestult = [@"viewWillMoveToSuperview_view1", @"viewDidMoveToSuperview_view1", "viewWillMoveToWindow_view1", "viewDidMoveToWindow_view1", @"viewWillMoveToSuperview_view1", @"viewDidMoveToSuperview_view1", "viewWillMoveToWindow_view1", "viewDidMoveToWindow_view1"]; + + [[window contentView] addSubview:view1]; + [[window contentView] addSubview:view1]; + + [self assert:expectedRestult equals:methodCalled]; +} + +- (void)testWhenRemovedSubviewMethodCalled +{ + var expectedRestult = [@"viewWillMoveToSuperview_view1", @"viewDidMoveToSuperview_view1"]; + + [view1 removeFromSuperview]; + + [self assert:expectedRestult equals:methodCalled]; +} + +- (void)testWhenAddedSubviewThenRemovedSubviewMethodCalled +{ + var expectedRestult = [@"viewWillMoveToSuperview_view1", @"viewDidMoveToSuperview_view1", "viewWillMoveToWindow_view1", "viewDidMoveToWindow_view1"]; + + [[window contentView] addSubview:view1]; + + methodCalled = []; + [view1 removeFromSuperview]; + + [self assert:expectedRestult equals:methodCalled]; +} + +- (void)testWhenAddedSubviewThenRemovedSubviewWithoutWindowMethodCalled +{ + var expectedRestult = [@"viewWillMoveToSuperview_view1", @"viewDidMoveToSuperview_view1"]; + + [view1 addSubview:view2]; + + methodCalled = []; + [view1 removeFromSuperview]; + + [self assert:expectedRestult equals:methodCalled]; +} + +- (void)testWhenAddedTwoSubviewsMethodCalled +{ + var expectedRestult = [@"viewWillMoveToSuperview_view2", @"viewDidMoveToSuperview_view2",@"viewWillMoveToSuperview_view1", @"viewDidMoveToSuperview_view1", "viewWillMoveToWindow_view1", "viewWillMoveToWindow_view2", "viewDidMoveToWindow_view2", "viewDidMoveToWindow_view1"]; + + [view1 addSubview:view2]; + [[window contentView] addSubview:view1]; + + [self assert:expectedRestult equals:methodCalled]; +} + +- (void)testWhenAddedTwoSubviewsThenRemovedMethodCalled +{ + var expectedRestult = [@"viewWillMoveToSuperview_view1", @"viewDidMoveToSuperview_view1", "viewWillMoveToWindow_view1", "viewWillMoveToWindow_view2", "viewDidMoveToWindow_view2", "viewDidMoveToWindow_view1"]; + + [view1 addSubview:view2]; + [[window contentView] addSubview:view1]; + + methodCalled = []; + [view1 removeFromSuperview]; + + [self assert:expectedRestult equals:methodCalled]; +} + +- (void)testWhenAddedOneSubviewsWithSetSubviewsMethodCalled +{ + var expectedRestult = [@"viewWillMoveToSuperview_view1", @"viewDidMoveToSuperview_view1", "viewWillMoveToWindow_view1", "viewDidMoveToWindow_view1"]; + + [[window contentView] setSubviews:[view1]]; + + [self assert:expectedRestult equals:methodCalled]; +} + +- (void)testWhenAddedTwoSubviewsWithSetSubviewsMethodCalled +{ + var expectedRestult = [@"viewWillMoveToSuperview_view1", @"viewDidMoveToSuperview_view1", "viewWillMoveToWindow_view1", "viewDidMoveToWindow_view1", @"viewWillMoveToSuperview_view2", @"viewDidMoveToSuperview_view2", "viewWillMoveToWindow_view2", "viewDidMoveToWindow_view2"]; + + [[window contentView] setSubviews:[view1, view2]]; + + [self assert:expectedRestult equals:methodCalled]; +} + +- (void)testWhenAddedTwoSubviewsThenSetSubviewsWithOneViewMethodCalled +{ + var expectedRestult = [@"viewWillMoveToSuperview_view2", @"viewDidMoveToSuperview_view2", "viewWillMoveToWindow_view2", "viewDidMoveToWindow_view2"]; + + [[window contentView] setSubviews:[view1, view2]]; + + methodCalled = []; + + [[window contentView] setSubviews:[view1]]; + + [self assert:expectedRestult equals:methodCalled]; +} + +- (void)testWhenAddedTwoSubviewsThenSetSubviewsWithTwoViewsMethodCalled +{ + var expectedRestult = [@"viewWillMoveToSuperview_view2", @"viewDidMoveToSuperview_view2", "viewWillMoveToWindow_view2", "viewDidMoveToWindow_view2", @"viewWillMoveToSuperview_view3", @"viewDidMoveToSuperview_view3", "viewWillMoveToWindow_view3", "viewDidMoveToWindow_view3"]; + + [[window contentView] setSubviews:[view1, view2]]; + + methodCalled = []; + + [[window contentView] setSubviews:[view1, view3]]; + + [self assert:expectedRestult equals:methodCalled]; +} + +- (void)testWhenReplacedViewWithSameViewMethodCalled +{ + var expectedRestult = []; + + [[window contentView] addSubview:view1]; + + methodCalled = []; + + [[window contentView] replaceSubview:view1 with:view1]; + + [self assert:expectedRestult equals:methodCalled]; +} + +- (void)testWhenReplacedViewWithOtherViewMethodCalled +{ + var expectedRestult = [@"viewWillMoveToSuperview_view2", @"viewDidMoveToSuperview_view2", "viewWillMoveToWindow_view2", "viewDidMoveToWindow_view2", @"viewWillMoveToSuperview_view1", @"viewDidMoveToSuperview_view1", "viewWillMoveToWindow_view1", "viewDidMoveToWindow_view1"]; + + [[window contentView] addSubview:view1]; + + methodCalled = []; + + [[window contentView] replaceSubview:view1 with:view2]; + + [self assert:expectedRestult equals:methodCalled]; +} + +- (void)testWhenReplacedViewWithOtherAddedViewMethodCalled +{ + var expectedRestult = [@"viewWillMoveToSuperview_view2", @"viewDidMoveToSuperview_view2", "viewWillMoveToWindow_view2", "viewDidMoveToWindow_view2", @"viewWillMoveToSuperview_view1", @"viewDidMoveToSuperview_view1", "viewWillMoveToWindow_view1", "viewDidMoveToWindow_view1"]; + + [[window contentView] addSubview:view1]; + [[window contentView] addSubview:view2]; + + methodCalled = []; + + [[window contentView] replaceSubview:view1 with:view2]; + + [self assert:expectedRestult equals:methodCalled]; +} + @end + @implementation CPResponderView : CPView +- (void)viewDidMoveToSuperview +{ + var string = @"viewDidMoveToSuperview_" + [self identifier]; + [methodCalled addObject:string]; +} + +- (void)viewDidMoveToWindow +{ + var string = @"viewDidMoveToWindow_" + [self identifier]; + [methodCalled addObject:string]; +} + +- (void)viewWillMoveToSuperview:(CPView)newSuperview +{ + var string = @"viewWillMoveToSuperview_" + [self identifier]; + [methodCalled addObject:string]; +} + +- (void)viewWillMoveToWindow:(CPWindow)newWindow +{ + var string = @"viewWillMoveToWindow_" + [self identifier]; + [methodCalled addObject:string]; +} + - (BOOL)acceptsFirstResponder { return YES; From abdf1156d4d3d33afff55e168881cd7729c28fe0 Mon Sep 17 00:00:00 2001 From: Alexandre Wilhelm Date: Tue, 19 Aug 2014 17:45:01 -0700 Subject: [PATCH 02/11] Fixed: memory leaks with CPNotifications Previously, when removing a view, Cappuccino didn't clean the notification center. The notification center kept in reference old views. This PR fix this issue. When a CPView is added to a view, the methods _removeObservers and _addObservers are called. In these both methods we remove and add the observer to the notification center if needed. _removeObservers and _addObservers are called for the view and its subviews. These both methods are called through the method viewWillMoveToSuperview. When a CPView is removed, we only call the method _removeObservers. When a CPWindow is closed, we call the method _removeObservers on its contentView. When a CPWindow is about to be opened, we call the method _removeObservers and _addObservers on its contentView. Refs #1880 Refs #2024 Test app in Tests/Manual/AttachedSheet2/SheetWindowController.j --- AppKit/CPClipView.j | 45 +++-- AppKit/CPComboBox.j | 202 +++++++++++-------- AppKit/CPDatePicker/_CPDatePickerTextField.j | 40 +++- AppKit/CPSearchField.j | 18 +- AppKit/CPTableView.j | 16 +- AppKit/CPTextField.j | 14 ++ AppKit/CPView.j | 20 ++ AppKit/CPWindow/CPWindow.j | 4 + 8 files changed, 255 insertions(+), 104 deletions(-) diff --git a/AppKit/CPClipView.j b/AppKit/CPClipView.j index 7cc52d2bc..6c941aace 100644 --- a/AppKit/CPClipView.j +++ b/AppKit/CPClipView.j @@ -47,18 +47,7 @@ if (_documentView) { - var defaultCenter = [CPNotificationCenter defaultCenter]; - - [defaultCenter - removeObserver:self - name:CPViewFrameDidChangeNotification - object:_documentView]; - - [defaultCenter - removeObserver:self - name:CPViewBoundsDidChangeNotification - object:_documentView]; - + [self _removeObserverDocumentView:_documentView]; [_documentView removeFromSuperview]; } @@ -91,6 +80,38 @@ object:_documentView]; } +- (void)_removeObserverDocumentView:(CPView)aDocumentView +{ + var defaultCenter = [CPNotificationCenter defaultCenter]; + + [defaultCenter + removeObserver:self + name:CPViewFrameDidChangeNotification + object:_documentView]; + + [defaultCenter + removeObserver:self + name:CPViewBoundsDidChangeNotification + object:_documentView]; +} + +- (void)_addObservers +{ + [super _addObservers]; + + if (_documentView) + [self _observeDocumentView]; +} + +- (void)_removeObservers +{ + [super _removeObservers]; + + if (_documentView) + [self _removeObserverDocumentView:_documentView]; +} + + /*! Returns the document view. */ diff --git a/AppKit/CPComboBox.j b/AppKit/CPComboBox.j index 6b783b426..c75b01886 100644 --- a/AppKit/CPComboBox.j +++ b/AppKit/CPComboBox.j @@ -210,42 +210,11 @@ var CPComboBoxTextSubview = @"text", if (aDelegate === delegate) return; - var defaultCenter = [CPNotificationCenter defaultCenter]; - if (delegate) - { - [defaultCenter removeObserver:delegate name:CPComboBoxSelectionIsChangingNotification object:self]; - [defaultCenter removeObserver:delegate name:CPComboBoxSelectionDidChangeNotification object:self]; - [defaultCenter removeObserver:delegate name:CPComboBoxWillDismissNotification object:self]; - [defaultCenter removeObserver:delegate name:CPComboBoxWillPopUpNotification object:self]; - } + [self _removeObserversDelegate:delegate]; if (aDelegate) - { - if ([aDelegate respondsToSelector:@selector(comboBoxSelectionIsChanging:)]) - [defaultCenter addObserver:delegate - selector:@selector(comboBoxSelectionIsChanging:) - name:CPComboBoxSelectionIsChangingNotification - object:self]; - - if ([aDelegate respondsToSelector:@selector(comboBoxSelectionDidChange:)]) - [defaultCenter addObserver:delegate - selector:@selector(comboBoxSelectionDidChange:) - name:CPComboBoxSelectionDidChangeNotification - object:self]; - - if ([aDelegate respondsToSelector:@selector(comboBoxWillPopUp:)]) - [defaultCenter addObserver:delegate - selector:@selector(comboBoxWillPopUp:) - name:CPComboBoxWillPopUpNotification - object:self]; - - if ([aDelegate respondsToSelector:@selector(comboBoxWillDismiss:)]) - [defaultCenter addObserver:delegate - selector:@selector(comboBoxWillDissmis:) - name:CPComboBoxWillDismissNotification - object:self]; - } + [self _addObserversDelegate:aDelegate]; [super setDelegate:aDelegate]; } @@ -397,56 +366,11 @@ var CPComboBoxTextSubview = @"text", var defaultCenter = [CPNotificationCenter defaultCenter]; if (_listDelegate) - { - [defaultCenter removeObserver:self name:_CPPopUpListWillPopUpNotification object:_listDelegate]; - [defaultCenter removeObserver:self name:_CPPopUpListWillDismissNotification object:_listDelegate]; - [defaultCenter removeObserver:self name:_CPPopUpListDidDismissNotification object:_listDelegate]; - [defaultCenter removeObserver:self name:_CPPopUpListItemWasClickedNotification object:_listDelegate]; - - var oldTableView = [_listDelegate tableView]; - - if (oldTableView) - { - [defaultCenter removeObserver:self name:CPTableViewSelectionIsChangingNotification object:oldTableView]; - [defaultCenter removeObserver:self name:CPTableViewSelectionDidChangeNotification object:oldTableView]; - } - } + [self _removeObserversListDelegate:_listDelegate] _listDelegate = aDelegate; - [defaultCenter addObserver:self - selector:@selector(comboBoxWillPopUp:) - name:_CPPopUpListWillPopUpNotification - object:_listDelegate]; - - [defaultCenter addObserver:self - selector:@selector(comboBoxWillDismiss:) - name:_CPPopUpListWillDismissNotification - object:_listDelegate]; - - [defaultCenter addObserver:self - selector:@selector(listDidDismiss:) - name:_CPPopUpListDidDismissNotification - object:_listDelegate]; - - [defaultCenter addObserver:self - selector:@selector(itemWasClicked:) - name:_CPPopUpListItemWasClickedNotification - object:_listDelegate]; - - [[_listDelegate scrollView] setHasVerticalScroller:_hasVerticalScroller]; - - var tableView = [_listDelegate tableView]; - - [defaultCenter addObserver:self - selector:@selector(comboBoxSelectionIsChanging:) - name:CPTableViewSelectionIsChangingNotification - object:tableView]; - - [defaultCenter addObserver:self - selector:@selector(comboBoxSelectionDidChange:) - name:CPTableViewSelectionDidChangeNotification - object:tableView]; + [self _addObserversListDelegate:_listDelegate] // Apply our text style to the list [_listDelegate setFont:[self font]]; @@ -973,6 +897,124 @@ var CPComboBoxTextSubview = @"text", } } +- (void)_removeObservers +{ + [super _removeObservers]; + + if (_listDelegate) + [self _removeObserversListDelegate:_listDelegate]; + + if ([self delegate]) + [self _removeObserversDelegate:[self delegate]]; +} + +- (void)_removeObserversDelegate:(id)aDelegate +{ + var defaultCenter = [CPNotificationCenter defaultCenter]; + + [defaultCenter removeObserver:aDelegate name:CPComboBoxSelectionIsChangingNotification object:self]; + [defaultCenter removeObserver:aDelegate name:CPComboBoxSelectionDidChangeNotification object:self]; + [defaultCenter removeObserver:aDelegate name:CPComboBoxWillDismissNotification object:self]; + [defaultCenter removeObserver:aDelegate name:CPComboBoxWillPopUpNotification object:self]; +} + +- (void)_removeObserversListDelegate:(id)aDelegate +{ + var defaultCenter = [CPNotificationCenter defaultCenter]; + + [defaultCenter removeObserver:self name:_CPPopUpListWillPopUpNotification object:aDelegate]; + [defaultCenter removeObserver:self name:_CPPopUpListWillDismissNotification object:aDelegate]; + [defaultCenter removeObserver:self name:_CPPopUpListDidDismissNotification object:aDelegate]; + [defaultCenter removeObserver:self name:_CPPopUpListItemWasClickedNotification object:aDelegate]; + + var oldTableView = [aDelegate tableView]; + + if (oldTableView) + { + [defaultCenter removeObserver:self name:CPTableViewSelectionIsChangingNotification object:oldTableView]; + [defaultCenter removeObserver:self name:CPTableViewSelectionDidChangeNotification object:oldTableView]; + } +} + +- (void)_addObservers +{ + [super _addObservers]; + + if (_listDelegate) + [self _addObserversListDelegate:_listDelegate]; + + if ([self delegate]) + [self _addObserversDelegate:[self delegate]]; +} + +- (void)_addObserversDelegate:(id)aDelegate +{ + var defaultCenter = [CPNotificationCenter defaultCenter]; + + if ([aDelegate respondsToSelector:@selector(comboBoxSelectionIsChanging:)]) + [defaultCenter addObserver:aDelegate + selector:@selector(comboBoxSelectionIsChanging:) + name:CPComboBoxSelectionIsChangingNotification + object:self]; + + if ([aDelegate respondsToSelector:@selector(comboBoxSelectionDidChange:)]) + [defaultCenter addObserver:aDelegate + selector:@selector(comboBoxSelectionDidChange:) + name:CPComboBoxSelectionDidChangeNotification + object:self]; + + if ([aDelegate respondsToSelector:@selector(comboBoxWillPopUp:)]) + [defaultCenter addObserver:aDelegate + selector:@selector(comboBoxWillPopUp:) + name:CPComboBoxWillPopUpNotification + object:self]; + + if ([aDelegate respondsToSelector:@selector(comboBoxWillDismiss:)]) + [defaultCenter addObserver:aDelegate + selector:@selector(comboBoxWillDissmis:) + name:CPComboBoxWillDismissNotification + object:self]; +} + +- (void)_addObserversListDelegate:(id)aDelegate +{ + var defaultCenter = [CPNotificationCenter defaultCenter]; + + [defaultCenter addObserver:self + selector:@selector(comboBoxWillPopUp:) + name:_CPPopUpListWillPopUpNotification + object:aDelegate]; + + [defaultCenter addObserver:self + selector:@selector(comboBoxWillDismiss:) + name:_CPPopUpListWillDismissNotification + object:aDelegate]; + + [defaultCenter addObserver:self + selector:@selector(listDidDismiss:) + name:_CPPopUpListDidDismissNotification + object:aDelegate]; + + [defaultCenter addObserver:self + selector:@selector(itemWasClicked:) + name:_CPPopUpListItemWasClickedNotification + object:aDelegate]; + + [[aDelegate scrollView] setHasVerticalScroller:_hasVerticalScroller]; + + var tableView = [aDelegate tableView]; + + [defaultCenter addObserver:self + selector:@selector(comboBoxSelectionIsChanging:) + name:CPTableViewSelectionIsChangingNotification + object:tableView]; + + [defaultCenter addObserver:self + selector:@selector(comboBoxSelectionDidChange:) + name:CPTableViewSelectionDidChangeNotification + object:tableView]; +} + @end @implementation CPComboBox (CPComboBoxDelegate) diff --git a/AppKit/CPDatePicker/_CPDatePickerTextField.j b/AppKit/CPDatePicker/_CPDatePickerTextField.j index 0429c5752..d991f5006 100644 --- a/AppKit/CPDatePicker/_CPDatePickerTextField.j +++ b/AppKit/CPDatePicker/_CPDatePickerTextField.j @@ -92,8 +92,6 @@ var CPZeroKeyCode = 48, [_stepper setAction:@selector(_clickStepper:)]; [self addSubview:_stepper]; - [[CPNotificationCenter defaultCenter] addObserver:self selector:@selector(_datePickerElementTextFieldBecomeFirstResponder:) name:CPDatePickerElementTextFieldBecomeFirstResponder object:self]; - [self setNeedsLayout]; [self setNeedsDisplay:YES]; } @@ -410,6 +408,24 @@ var CPZeroKeyCode = 48, [_datePickerElementView setNeedsLayout]; } + +#pragma mark - +#pragma mark Override observers + +- (void)_removeObservers +{ + [super _removeObservers]; + + [[CPNotificationCenter defaultCenter] removeObserver:self name:CPDatePickerElementTextFieldBecomeFirstResponder object:self]; +} + +- (void)_addObservers +{ + [super _addObservers]; + + [[CPNotificationCenter defaultCenter] addObserver:self selector:@selector(_datePickerElementTextFieldBecomeFirstResponder:) name:CPDatePickerElementTextFieldBecomeFirstResponder object:self]; +} + @end @@ -520,8 +536,6 @@ var CPZeroKeyCode = 48, [self addSubview: _textFieldSeparatorThree]; [self addSubview: _textFieldSeparatorFour]; - [[CPNotificationCenter defaultCenter] addObserver:self selector:@selector(_datePickerElementTextFieldAMPMChangedNotification:) name:CPDatePickerElementTextFieldAMPMChangedNotification object:_textFieldPMAM]; - [self setNeedsLayout]; } @@ -535,6 +549,24 @@ var CPZeroKeyCode = 48, } +#pragma mark - +#pragma mark Override observers + +- (void)_removeObservers +{ + [super _removeObservers]; + + [[CPNotificationCenter defaultCenter] removeObserver:self name:CPDatePickerElementTextFieldAMPMChangedNotification object:_textFieldPMAM]; +} + +- (void)_addObservers +{ + [super _addObservers]; + + [[CPNotificationCenter defaultCenter] addObserver:self selector:@selector(_datePickerElementTextFieldAMPMChangedNotification:) name:CPDatePickerElementTextFieldAMPMChangedNotification object:_textFieldPMAM]; +} + + #pragma mark - #pragma mark Setter Getter methods diff --git a/AppKit/CPSearchField.j b/AppKit/CPSearchField.j index d7346f9ed..0f6d9d9e4 100644 --- a/AppKit/CPSearchField.j +++ b/AppKit/CPSearchField.j @@ -119,16 +119,22 @@ var RECENT_SEARCH_PREFIX = @" "; _canResignFirstResponder = YES; } -- (void)viewWillMoveToSuperview:(CPView)aView + +#pragma mark - +#pragma mark Override observers + +- (void)_removeObservers { - [super viewWillMoveToSuperview:aView]; + [super _removeObservers]; - // First we remove any observer that may have been in place to avoid memory leakage. [[CPNotificationCenter defaultCenter] removeObserver:self name:CPControlTextDidChangeNotification object:self]; +} - // Register the observe here if we need to. - if (aView) - [[CPNotificationCenter defaultCenter] addObserver:self selector:@selector(_searchFieldTextDidChange:) name:CPControlTextDidChangeNotification object:self]; +- (void)_addObservers +{ + [super _addObservers]; + + [[CPNotificationCenter defaultCenter] addObserver:self selector:@selector(_searchFieldTextDidChange:) name:CPControlTextDidChangeNotification object:self]; } // Managing Buttons diff --git a/AppKit/CPTableView.j b/AppKit/CPTableView.j index e597f2646..ebeb66e19 100644 --- a/AppKit/CPTableView.j +++ b/AppKit/CPTableView.j @@ -473,8 +473,6 @@ CPTableViewFirstColumnOnlyAutoresizingStyle = 5; if (!_sortDescriptors) _sortDescriptors = []; - - [self _startObservingFirstResponder]; } /*! @@ -4453,6 +4451,8 @@ Your delegate can implement this method to avoid subclassing the tableview to ad */ - (void)viewWillMoveToSuperview:(CPView)aView { + [super viewWillMoveToSuperview:aView]; + var superview = [self superview], defaultCenter = [CPNotificationCenter defaultCenter]; @@ -5125,6 +5125,18 @@ Your delegate can implement this method to avoid subclassing the tableview to ad return hit; } +- (void)_removeObservers +{ + [super _removeObservers]; + [self _stopObservingFirstResponder]; +} + +- (void)_addObservers +{ + [super _addObservers]; + [self _startObservingFirstResponder]; +} + - (void)_startObservingFirstResponder { [[CPNotificationCenter defaultCenter] addObserver:self selector:@selector(_firstResponderDidChange:) name:_CPWindowDidChangeFirstResponderNotification object:[self window]]; diff --git a/AppKit/CPTextField.j b/AppKit/CPTextField.j index 5ff471141..6b1591222 100644 --- a/AppKit/CPTextField.j +++ b/AppKit/CPTextField.j @@ -820,6 +820,20 @@ CPTextFieldStatePlaceholder = CPThemeState("placeholder"); } } +- (void)_removeObservers +{ + [super _removeObservers]; + [self _setObserveWindowKeyNotifications:NO]; +} + +- (void)_addObservers +{ + [super _addObservers]; + + if ([self window] === self) + [self _setObserveWindowKeyNotifications:YES]; +} + - (void)_windowDidResignKey:(CPNotification)aNotification { if (![[self window] isKeyWindow]) diff --git a/AppKit/CPView.j b/AppKit/CPView.j index d0517df4b..9fb8b6de4 100644 --- a/AppKit/CPView.j +++ b/AppKit/CPView.j @@ -816,6 +816,10 @@ var CPViewFlags = { }, */ - (void)viewWillMoveToSuperview:(CPView)aView { + [self _removeObservers]; + + if (aView) + [self _addObservers]; } /*! @@ -834,6 +838,22 @@ var CPViewFlags = { }, { } +- (void)_removeObservers +{ + var count = [_subviews count]; + + while (count--) + [_subviews[count] _removeObservers]; +} + +- (void)_addObservers +{ + var count = [_subviews count]; + + while (count--) + [_subviews[count] _addObservers]; +} + /*! Returns the menu item containing the receiver or one of its ancestor views. @return a menu item, or \c nil if the view or one of its ancestors wasn't found diff --git a/AppKit/CPWindow/CPWindow.j b/AppKit/CPWindow/CPWindow.j index 0e074ccb1..70fb8034e 100644 --- a/AppKit/CPWindow/CPWindow.j +++ b/AppKit/CPWindow/CPWindow.j @@ -901,6 +901,8 @@ CPTexturedBackgroundWindowMask - (void)_orderFront { + [[self contentView] _addObservers]; + #if PLATFORM(DOM) // -dw- if a sheet is clicked, the parent window should come up too if (_isSheet) @@ -960,6 +962,8 @@ CPTexturedBackgroundWindowMask if (!_isVisible) return; + [[self contentView] _removeObservers]; + if ([self isSheet]) { // -dw- as in Cocoa, orderOut: detaches the sheet and animates out From 0479e9b3f3991f5255f3bb29f99adc70b6d77e52 Mon Sep 17 00:00:00 2001 From: Alexandre Wilhelm Date: Tue, 19 Aug 2014 17:52:24 -0700 Subject: [PATCH 03/11] Fixed: make sure to remove observers when opening a CPWindow --- AppKit/CPWindow/CPWindow.j | 1 + 1 file changed, 1 insertion(+) diff --git a/AppKit/CPWindow/CPWindow.j b/AppKit/CPWindow/CPWindow.j index 70fb8034e..6f7e684c4 100644 --- a/AppKit/CPWindow/CPWindow.j +++ b/AppKit/CPWindow/CPWindow.j @@ -901,6 +901,7 @@ CPTexturedBackgroundWindowMask - (void)_orderFront { + [[self contentView] _removeObservers]; [[self contentView] _addObservers]; #if PLATFORM(DOM) From ce0233d0a24c834da6517a6c35ae68ac3a105e87 Mon Sep 17 00:00:00 2001 From: Alexandre Wilhelm Date: Wed, 20 Aug 2014 15:11:21 -0700 Subject: [PATCH 04/11] Fixed: revert commit abdf1156d4d3d33afff55e168881cd7729c28fe0 for CPComboBox --- AppKit/CPComboBox.j | 202 ++++++++++++++++++-------------------------- 1 file changed, 80 insertions(+), 122 deletions(-) diff --git a/AppKit/CPComboBox.j b/AppKit/CPComboBox.j index c75b01886..6b783b426 100644 --- a/AppKit/CPComboBox.j +++ b/AppKit/CPComboBox.j @@ -210,11 +210,42 @@ var CPComboBoxTextSubview = @"text", if (aDelegate === delegate) return; + var defaultCenter = [CPNotificationCenter defaultCenter]; + if (delegate) - [self _removeObserversDelegate:delegate]; + { + [defaultCenter removeObserver:delegate name:CPComboBoxSelectionIsChangingNotification object:self]; + [defaultCenter removeObserver:delegate name:CPComboBoxSelectionDidChangeNotification object:self]; + [defaultCenter removeObserver:delegate name:CPComboBoxWillDismissNotification object:self]; + [defaultCenter removeObserver:delegate name:CPComboBoxWillPopUpNotification object:self]; + } if (aDelegate) - [self _addObserversDelegate:aDelegate]; + { + if ([aDelegate respondsToSelector:@selector(comboBoxSelectionIsChanging:)]) + [defaultCenter addObserver:delegate + selector:@selector(comboBoxSelectionIsChanging:) + name:CPComboBoxSelectionIsChangingNotification + object:self]; + + if ([aDelegate respondsToSelector:@selector(comboBoxSelectionDidChange:)]) + [defaultCenter addObserver:delegate + selector:@selector(comboBoxSelectionDidChange:) + name:CPComboBoxSelectionDidChangeNotification + object:self]; + + if ([aDelegate respondsToSelector:@selector(comboBoxWillPopUp:)]) + [defaultCenter addObserver:delegate + selector:@selector(comboBoxWillPopUp:) + name:CPComboBoxWillPopUpNotification + object:self]; + + if ([aDelegate respondsToSelector:@selector(comboBoxWillDismiss:)]) + [defaultCenter addObserver:delegate + selector:@selector(comboBoxWillDissmis:) + name:CPComboBoxWillDismissNotification + object:self]; + } [super setDelegate:aDelegate]; } @@ -366,11 +397,56 @@ var CPComboBoxTextSubview = @"text", var defaultCenter = [CPNotificationCenter defaultCenter]; if (_listDelegate) - [self _removeObserversListDelegate:_listDelegate] + { + [defaultCenter removeObserver:self name:_CPPopUpListWillPopUpNotification object:_listDelegate]; + [defaultCenter removeObserver:self name:_CPPopUpListWillDismissNotification object:_listDelegate]; + [defaultCenter removeObserver:self name:_CPPopUpListDidDismissNotification object:_listDelegate]; + [defaultCenter removeObserver:self name:_CPPopUpListItemWasClickedNotification object:_listDelegate]; + + var oldTableView = [_listDelegate tableView]; + + if (oldTableView) + { + [defaultCenter removeObserver:self name:CPTableViewSelectionIsChangingNotification object:oldTableView]; + [defaultCenter removeObserver:self name:CPTableViewSelectionDidChangeNotification object:oldTableView]; + } + } _listDelegate = aDelegate; - [self _addObserversListDelegate:_listDelegate] + [defaultCenter addObserver:self + selector:@selector(comboBoxWillPopUp:) + name:_CPPopUpListWillPopUpNotification + object:_listDelegate]; + + [defaultCenter addObserver:self + selector:@selector(comboBoxWillDismiss:) + name:_CPPopUpListWillDismissNotification + object:_listDelegate]; + + [defaultCenter addObserver:self + selector:@selector(listDidDismiss:) + name:_CPPopUpListDidDismissNotification + object:_listDelegate]; + + [defaultCenter addObserver:self + selector:@selector(itemWasClicked:) + name:_CPPopUpListItemWasClickedNotification + object:_listDelegate]; + + [[_listDelegate scrollView] setHasVerticalScroller:_hasVerticalScroller]; + + var tableView = [_listDelegate tableView]; + + [defaultCenter addObserver:self + selector:@selector(comboBoxSelectionIsChanging:) + name:CPTableViewSelectionIsChangingNotification + object:tableView]; + + [defaultCenter addObserver:self + selector:@selector(comboBoxSelectionDidChange:) + name:CPTableViewSelectionDidChangeNotification + object:tableView]; // Apply our text style to the list [_listDelegate setFont:[self font]]; @@ -897,124 +973,6 @@ var CPComboBoxTextSubview = @"text", } } -- (void)_removeObservers -{ - [super _removeObservers]; - - if (_listDelegate) - [self _removeObserversListDelegate:_listDelegate]; - - if ([self delegate]) - [self _removeObserversDelegate:[self delegate]]; -} - -- (void)_removeObserversDelegate:(id)aDelegate -{ - var defaultCenter = [CPNotificationCenter defaultCenter]; - - [defaultCenter removeObserver:aDelegate name:CPComboBoxSelectionIsChangingNotification object:self]; - [defaultCenter removeObserver:aDelegate name:CPComboBoxSelectionDidChangeNotification object:self]; - [defaultCenter removeObserver:aDelegate name:CPComboBoxWillDismissNotification object:self]; - [defaultCenter removeObserver:aDelegate name:CPComboBoxWillPopUpNotification object:self]; -} - -- (void)_removeObserversListDelegate:(id)aDelegate -{ - var defaultCenter = [CPNotificationCenter defaultCenter]; - - [defaultCenter removeObserver:self name:_CPPopUpListWillPopUpNotification object:aDelegate]; - [defaultCenter removeObserver:self name:_CPPopUpListWillDismissNotification object:aDelegate]; - [defaultCenter removeObserver:self name:_CPPopUpListDidDismissNotification object:aDelegate]; - [defaultCenter removeObserver:self name:_CPPopUpListItemWasClickedNotification object:aDelegate]; - - var oldTableView = [aDelegate tableView]; - - if (oldTableView) - { - [defaultCenter removeObserver:self name:CPTableViewSelectionIsChangingNotification object:oldTableView]; - [defaultCenter removeObserver:self name:CPTableViewSelectionDidChangeNotification object:oldTableView]; - } -} - -- (void)_addObservers -{ - [super _addObservers]; - - if (_listDelegate) - [self _addObserversListDelegate:_listDelegate]; - - if ([self delegate]) - [self _addObserversDelegate:[self delegate]]; -} - -- (void)_addObserversDelegate:(id)aDelegate -{ - var defaultCenter = [CPNotificationCenter defaultCenter]; - - if ([aDelegate respondsToSelector:@selector(comboBoxSelectionIsChanging:)]) - [defaultCenter addObserver:aDelegate - selector:@selector(comboBoxSelectionIsChanging:) - name:CPComboBoxSelectionIsChangingNotification - object:self]; - - if ([aDelegate respondsToSelector:@selector(comboBoxSelectionDidChange:)]) - [defaultCenter addObserver:aDelegate - selector:@selector(comboBoxSelectionDidChange:) - name:CPComboBoxSelectionDidChangeNotification - object:self]; - - if ([aDelegate respondsToSelector:@selector(comboBoxWillPopUp:)]) - [defaultCenter addObserver:aDelegate - selector:@selector(comboBoxWillPopUp:) - name:CPComboBoxWillPopUpNotification - object:self]; - - if ([aDelegate respondsToSelector:@selector(comboBoxWillDismiss:)]) - [defaultCenter addObserver:aDelegate - selector:@selector(comboBoxWillDissmis:) - name:CPComboBoxWillDismissNotification - object:self]; -} - -- (void)_addObserversListDelegate:(id)aDelegate -{ - var defaultCenter = [CPNotificationCenter defaultCenter]; - - [defaultCenter addObserver:self - selector:@selector(comboBoxWillPopUp:) - name:_CPPopUpListWillPopUpNotification - object:aDelegate]; - - [defaultCenter addObserver:self - selector:@selector(comboBoxWillDismiss:) - name:_CPPopUpListWillDismissNotification - object:aDelegate]; - - [defaultCenter addObserver:self - selector:@selector(listDidDismiss:) - name:_CPPopUpListDidDismissNotification - object:aDelegate]; - - [defaultCenter addObserver:self - selector:@selector(itemWasClicked:) - name:_CPPopUpListItemWasClickedNotification - object:aDelegate]; - - [[aDelegate scrollView] setHasVerticalScroller:_hasVerticalScroller]; - - var tableView = [aDelegate tableView]; - - [defaultCenter addObserver:self - selector:@selector(comboBoxSelectionIsChanging:) - name:CPTableViewSelectionIsChangingNotification - object:tableView]; - - [defaultCenter addObserver:self - selector:@selector(comboBoxSelectionDidChange:) - name:CPTableViewSelectionDidChangeNotification - object:tableView]; -} - @end @implementation CPComboBox (CPComboBoxDelegate) From bd20050db8321fc2b25974dc6ed54bd59a4c8622 Mon Sep 17 00:00:00 2001 From: Alexandre Wilhelm Date: Wed, 20 Aug 2014 15:23:10 -0700 Subject: [PATCH 05/11] Fixed: added a boolean to avoid to addObservers seveal times --- AppKit/CPView.j | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/AppKit/CPView.j b/AppKit/CPView.j index 9fb8b6de4..8218bffd7 100644 --- a/AppKit/CPView.j +++ b/AppKit/CPView.j @@ -228,6 +228,8 @@ var CPViewFlags = { }, Function _toolTipFunctionIn; Function _toolTipFunctionOut; BOOL _toolTipInstalled; + + BOOL _isObserving; } /* @@ -844,14 +846,21 @@ var CPViewFlags = { }, while (count--) [_subviews[count] _removeObservers]; + + _isObserving = NO; } - (void)_addObservers { + if (_isObserving) + return; + var count = [_subviews count]; while (count--) [_subviews[count] _addObservers]; + + _isObserving = YES; } /*! From 1fcf13d149756d61dd90d3621623b9d36b459c25 Mon Sep 17 00:00:00 2001 From: Alexandre Wilhelm Date: Wed, 20 Aug 2014 16:13:59 -0700 Subject: [PATCH 06/11] Fixed: added optimization for releasing observers --- AppKit/CPClipView.j | 6 +++++ AppKit/CPDatePicker/_CPDatePickerTextField.j | 12 ++++++++++ .../CPRuleEditor/_CPRuleEditorViewSliceRow.j | 23 ++++++++++++++++--- AppKit/CPSearchField.j | 6 +++++ AppKit/CPTableView.j | 6 +++++ AppKit/CPTextField.j | 6 +++++ AppKit/CPView.j | 3 +++ 7 files changed, 59 insertions(+), 3 deletions(-) diff --git a/AppKit/CPClipView.j b/AppKit/CPClipView.j index 6c941aace..8128cc4bc 100644 --- a/AppKit/CPClipView.j +++ b/AppKit/CPClipView.j @@ -97,6 +97,9 @@ - (void)_addObservers { + if (_isObserving) + return; + [super _addObservers]; if (_documentView) @@ -105,6 +108,9 @@ - (void)_removeObservers { + if (!_isObserving) + return; + [super _removeObservers]; if (_documentView) diff --git a/AppKit/CPDatePicker/_CPDatePickerTextField.j b/AppKit/CPDatePicker/_CPDatePickerTextField.j index d991f5006..882747a6e 100644 --- a/AppKit/CPDatePicker/_CPDatePickerTextField.j +++ b/AppKit/CPDatePicker/_CPDatePickerTextField.j @@ -414,6 +414,9 @@ var CPZeroKeyCode = 48, - (void)_removeObservers { + if (!_isObserving) + return; + [super _removeObservers]; [[CPNotificationCenter defaultCenter] removeObserver:self name:CPDatePickerElementTextFieldBecomeFirstResponder object:self]; @@ -421,6 +424,9 @@ var CPZeroKeyCode = 48, - (void)_addObservers { + if (_isObserving) + return; + [super _addObservers]; [[CPNotificationCenter defaultCenter] addObserver:self selector:@selector(_datePickerElementTextFieldBecomeFirstResponder:) name:CPDatePickerElementTextFieldBecomeFirstResponder object:self]; @@ -554,6 +560,9 @@ var CPZeroKeyCode = 48, - (void)_removeObservers { + if (!_isObserving) + return; + [super _removeObservers]; [[CPNotificationCenter defaultCenter] removeObserver:self name:CPDatePickerElementTextFieldAMPMChangedNotification object:_textFieldPMAM]; @@ -561,6 +570,9 @@ var CPZeroKeyCode = 48, - (void)_addObservers { + if (_isObserving) + return; + [super _addObservers]; [[CPNotificationCenter defaultCenter] addObserver:self selector:@selector(_datePickerElementTextFieldAMPMChangedNotification:) name:CPDatePickerElementTextFieldAMPMChangedNotification object:_textFieldPMAM]; diff --git a/AppKit/CPRuleEditor/_CPRuleEditorViewSliceRow.j b/AppKit/CPRuleEditor/_CPRuleEditorViewSliceRow.j index 20b573790..c06f3021b 100644 --- a/AppKit/CPRuleEditor/_CPRuleEditorViewSliceRow.j +++ b/AppKit/CPRuleEditor/_CPRuleEditorViewSliceRow.j @@ -65,9 +65,6 @@ var CONTROL_HEIGHT = 16., [self addSubview:_subtractButton]; [self setAutoresizingMask:CPViewWidthSizable]; - - var center = [CPNotificationCenter defaultCenter]; - [center addObserver:self selector:@selector(_textDidChange:) name:CPControlTextDidChangeNotification object:nil]; } - (CPButton)_createRowButton @@ -481,6 +478,26 @@ var CONTROL_HEIGHT = 16., [self layoutSubviews]; } +- (void)_addObservers +{ + if (_isObserving) + return; + + [super _addObservers]; + + [[CPNotificationCenter defaultCenter] addObserver:self selector:@selector(_textDidChange:) name:CPControlTextDidChangeNotification object:nil]; +} + +- (void)_removeObservers +{ + if (!_isObserving) + return; + + [super _removeObservers]; + + [[CPNotificationCenter defaultCenter] removeObserver:self name:CPControlTextDidChangeNotification object:nil]; +} + - (void)drawRect:(CGRect)rect { [super drawRect:rect]; diff --git a/AppKit/CPSearchField.j b/AppKit/CPSearchField.j index 0f6d9d9e4..c26e79140 100644 --- a/AppKit/CPSearchField.j +++ b/AppKit/CPSearchField.j @@ -125,6 +125,9 @@ var RECENT_SEARCH_PREFIX = @" "; - (void)_removeObservers { + if (!_isObserving) + return; + [super _removeObservers]; [[CPNotificationCenter defaultCenter] removeObserver:self name:CPControlTextDidChangeNotification object:self]; @@ -132,6 +135,9 @@ var RECENT_SEARCH_PREFIX = @" "; - (void)_addObservers { + if (_isObserving) + return; + [super _addObservers]; [[CPNotificationCenter defaultCenter] addObserver:self selector:@selector(_searchFieldTextDidChange:) name:CPControlTextDidChangeNotification object:self]; diff --git a/AppKit/CPTableView.j b/AppKit/CPTableView.j index ebeb66e19..eed697cf8 100644 --- a/AppKit/CPTableView.j +++ b/AppKit/CPTableView.j @@ -5127,12 +5127,18 @@ Your delegate can implement this method to avoid subclassing the tableview to ad - (void)_removeObservers { + if (!_isObserving) + return; + [super _removeObservers]; [self _stopObservingFirstResponder]; } - (void)_addObservers { + if (_isObserving) + return; + [super _addObservers]; [self _startObservingFirstResponder]; } diff --git a/AppKit/CPTextField.j b/AppKit/CPTextField.j index 6b1591222..523d56bd4 100644 --- a/AppKit/CPTextField.j +++ b/AppKit/CPTextField.j @@ -822,12 +822,18 @@ CPTextFieldStatePlaceholder = CPThemeState("placeholder"); - (void)_removeObservers { + if (!_isObserving) + return; + [super _removeObservers]; [self _setObserveWindowKeyNotifications:NO]; } - (void)_addObservers { + if (_isObserving) + return; + [super _addObservers]; if ([self window] === self) diff --git a/AppKit/CPView.j b/AppKit/CPView.j index 8218bffd7..90f3cd529 100644 --- a/AppKit/CPView.j +++ b/AppKit/CPView.j @@ -842,6 +842,9 @@ var CPViewFlags = { }, - (void)_removeObservers { + if (!_isObserving) + return; + var count = [_subviews count]; while (count--) From aa4a4b07d828fbb0911de87ad3ac4c337193ec0d Mon Sep 17 00:00:00 2001 From: Alexandre Wilhelm Date: Thu, 28 Aug 2014 22:22:50 -0700 Subject: [PATCH 07/11] Fixed: method viewWillMoveToWindow and viewDidMoveToWindow not called when when adding a view to a view without a window where the previous superview had a window --- AppKit/CPView.j | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/AppKit/CPView.j b/AppKit/CPView.j index 90f3cd529..785a420e5 100644 --- a/AppKit/CPView.j +++ b/AppKit/CPView.j @@ -514,7 +514,8 @@ var CPViewFlags = { }, [aSubview viewWillMoveToSuperview:self]; // We will have to adjust the z-index of all views starting at this index. - var count = _subviews.length; + var count = _subviews.length, + lastWindow; // Dirty the key view loop, in case the window wants to auto recalculate it [[self window] _dirtyKeyViewLoop]; @@ -544,12 +545,11 @@ var CPViewFlags = { }, { var superview = aSubview._superview; + lastWindow = [superview window]; + // Remove the view from its previous superview. [aSubview _removeFromSuperview]; - if (superview) - [aSubview _setWindow:nil]; - // Set ourselves as the superview. aSubview._superview = self; } @@ -587,6 +587,9 @@ var CPViewFlags = { }, if (_window) [aSubview _setWindow:_window]; + if (!_window && lastWindow) + [aSubview _setWindow:nil]; + // This method might be called before we are fully unarchived, in which case the theme state isn't set up yet // and none of the below matters anyhow. if (_themeState) From cf0b4c8f0344dd1dac0561e7d1456ecba529d99e Mon Sep 17 00:00:00 2001 From: Alexandre Wilhelm Date: Thu, 28 Aug 2014 22:23:20 -0700 Subject: [PATCH 08/11] Fixed: added CPView test for the method addSubview --- Tests/AppKit/CPViewTest.j | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/Tests/AppKit/CPViewTest.j b/Tests/AppKit/CPViewTest.j index 49857032c..3d1430492 100644 --- a/Tests/AppKit/CPViewTest.j +++ b/Tests/AppKit/CPViewTest.j @@ -22,8 +22,8 @@ var methodCalled; view = [[CPView alloc] initWithFrame:CGRectMakeZero()]; view1 = [[CPResponderView alloc] initWithFrame:CGRectMakeZero()]; - view2 = [[CPResponderView alloc] initWithFrame:CGRectMakeZero()], - view3 = [[CPResponderView alloc] initWithFrame:CGRectMakeZero()], + view2 = [[CPResponderView alloc] initWithFrame:CGRectMakeZero()]; + view3 = [[CPResponderView alloc] initWithFrame:CGRectMakeZero()]; [view1 setIdentifier:@"view1"]; [view2 setIdentifier:@"view2"]; @@ -549,6 +549,16 @@ var methodCalled; [self assert:expectedRestult equals:methodCalled]; } +- (void)testWhenAddedTwoSubviewsThenAddedTheViewToAnotherViewWithoutWindowMethodCalled +{ + var expectedRestult = [@"viewWillMoveToSuperview_view1", @"viewDidMoveToSuperview_view1", @"viewWillMoveToWindow_view1", @"viewDidMoveToWindow_view1", @"viewWillMoveToSuperview_view1", @"viewDidMoveToSuperview_view1", @"viewWillMoveToWindow_view1", @"viewDidMoveToWindow_view1"]; + + [[window contentView] addSubview:view1]; + [view2 addSubview:view1]; + + [self assert:expectedRestult equals:methodCalled]; +} + - (void)testWhenAddedOneSubviewsWithSetSubviewsMethodCalled { var expectedRestult = [@"viewWillMoveToSuperview_view1", @"viewDidMoveToSuperview_view1", "viewWillMoveToWindow_view1", "viewDidMoveToWindow_view1"]; From de495aa1c551a29d9fd35fe19b18c816081d1a17 Mon Sep 17 00:00:00 2001 From: Alexandre Wilhelm Date: Thu, 28 Aug 2014 22:28:41 -0700 Subject: [PATCH 09/11] Fixed: _removeObservers called in orderFront, we don't need this as addObservers and removeObservers work with boolean to check if the observers are set or not --- AppKit/CPWindow/CPWindow.j | 1 - 1 file changed, 1 deletion(-) diff --git a/AppKit/CPWindow/CPWindow.j b/AppKit/CPWindow/CPWindow.j index 6f7e684c4..70fb8034e 100644 --- a/AppKit/CPWindow/CPWindow.j +++ b/AppKit/CPWindow/CPWindow.j @@ -901,7 +901,6 @@ CPTexturedBackgroundWindowMask - (void)_orderFront { - [[self contentView] _removeObservers]; [[self contentView] _addObservers]; #if PLATFORM(DOM) From c4924e4005fc872222216b4e964c006373e8f646 Mon Sep 17 00:00:00 2001 From: Alexandre Wilhelm Date: Thu, 4 Sep 2014 14:30:40 -0700 Subject: [PATCH 10/11] Fixed: typo --- AppKit/CPClipView.j | 1 - 1 file changed, 1 deletion(-) diff --git a/AppKit/CPClipView.j b/AppKit/CPClipView.j index 8128cc4bc..8503c4cc5 100644 --- a/AppKit/CPClipView.j +++ b/AppKit/CPClipView.j @@ -117,7 +117,6 @@ [self _removeObserverDocumentView:_documentView]; } - /*! Returns the document view. */ From be6e222d8d6f0005ea7198d0d3a638eb5f8779ff Mon Sep 17 00:00:00 2001 From: Alexandre Wilhelm Date: Wed, 24 Sep 2014 10:14:38 -0700 Subject: [PATCH 11/11] Fixed: memory leak in CPColorWell --- AppKit/CPColorWell.j | 44 +++++++++++++++++++++++++++++++++++++++----- 1 file changed, 39 insertions(+), 5 deletions(-) diff --git a/AppKit/CPColorWell.j b/AppKit/CPColorWell.j index 8d8132f3a..15fa5343b 100644 --- a/AppKit/CPColorWell.j +++ b/AppKit/CPColorWell.j @@ -86,14 +86,12 @@ var _CPColorWellDidBecomeExclusiveNotification = @"_CPColorWellDidBecomeExclusiv _active = NO; _color = [CPColor whiteColor]; [self setBordered:YES]; - - [self _registerForNotifications]; } return self; } -- (void)_registerForNotifications +- (void)_registerNotifications { var defaultCenter = [CPNotificationCenter defaultCenter]; @@ -110,6 +108,22 @@ var _CPColorWellDidBecomeExclusiveNotification = @"_CPColorWellDidBecomeExclusiv object:[CPColorPanel sharedColorPanel]]; } +- (void)_removeNotifications +{ + var defaultCenter = [CPNotificationCenter defaultCenter]; + + [defaultCenter + removeObserver:self + name:_CPColorWellDidBecomeExclusiveNotification + object:nil]; + + [defaultCenter + removeObserver:self + name:CPWindowWillCloseNotification + object:[CPColorPanel sharedColorPanel]]; + +} + /*! Sets whether the color well is bordered. */ @@ -312,6 +326,28 @@ var _CPColorWellDidBecomeExclusiveNotification = @"_CPColorWellDidBecomeExclusiv [contentBorderView setBackgroundColor:[self currentValueForThemeAttribute:@"content-border-color"]]; } + +#pragma mark - +#pragma mark Observers method + +- (void)_addObservers +{ + if (_isObserving) + return; + + [super _addObservers]; + [self _registerNotifications]; +} + +- (void)_removeObservers +{ + if (!_isObserving) + return; + + [super _removeObservers]; + [self _removeNotifications]; +} + @end @implementation CPColorWellValueBinder : CPBinder @@ -363,8 +399,6 @@ var CPColorWellColorKey = "CPColorWellColorKey", _active = NO; _color = [aCoder decodeObjectForKey:CPColorWellColorKey]; [self setBordered:[aCoder decodeBoolForKey:CPColorWellBorderedKey]]; - - [self _registerForNotifications]; } return self;