From 144526b58bb1d6866f8c427a63036c31f8e1d8a9 Mon Sep 17 00:00:00 2001 From: "gtm.daemon" Date: Mon, 8 Nov 2010 21:39:10 +0000 Subject: [Author: avi] Allow adding a new sheet in a sheet's closing callback in GTMWindowSheetController. GTMWindowSheetController does not completely remove a sheet until after the callback has been invoked. This causes an assert to trigger if another sheet is launched in the callback, which is inconsistent with normal Cocoa sheets. Switch the order of the two operations and add a unit test. This affects http://crbug.com/56948 . Patch by davidben@chromium.org . R=dmaclach DELTA=112 (106 added, 5 deleted, 1 changed) --- AppKit/GTMWindowSheetController.m | 16 +++--- AppKit/GTMWindowSheetControllerTest.m | 97 +++++++++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+), 6 deletions(-) (limited to 'AppKit') diff --git a/AppKit/GTMWindowSheetController.m b/AppKit/GTMWindowSheetController.m index f7c87a3..60cd611 100644 --- a/AppKit/GTMWindowSheetController.m +++ b/AppKit/GTMWindowSheetController.m @@ -523,7 +523,10 @@ willPositionSheet:(NSWindow*)sheet contextInfo:(void*)contextInfo arg1Size:(int)size { NSValue* viewKey = [NSValue valueWithNonretainedObject:(NSView*)contextInfo]; - GTMWSCSheetInfo* sheetInfo = [sheets_ objectForKey:viewKey]; + // Retain a reference to sheetInfo so we can use it after it is + // removed from sheets_. + GTMWSCSheetInfo* sheetInfo = + [[[sheets_ objectForKey:viewKey] retain] autorelease]; _GTMDevAssert(sheetInfo, @"Could not find information about the sheet that " @"just ended"); _GTMDevAssert(size == 8 || size == 32 || size == 64, @@ -537,6 +540,12 @@ willPositionSheet:(NSWindow*)sheet name:NSViewFrameDidChangeNotification object:contextInfo]; + // We clean up the sheet before calling the callback so that the + // callback is free to fire another sheet if it so desires. + [window_ removeChildWindow:sheetInfo->overlayWindow_]; + [sheetInfo->overlayWindow_ release]; + [sheets_ removeObjectForKey:viewKey]; + NSInvocation* invocation = [NSInvocation invocationWithMethodSignature: [sheetInfo->modalDelegate_ @@ -555,11 +564,6 @@ willPositionSheet:(NSWindow*)sheet } [invocation setArgument:&sheetInfo->contextInfo_ atIndex:4]; [invocation invokeWithTarget:sheetInfo->modalDelegate_]; - - [window_ removeChildWindow:sheetInfo->overlayWindow_]; - [sheetInfo->overlayWindow_ release]; - - [sheets_ removeObjectForKey:viewKey]; } - (void)systemRequestsVisibilityForWindow:(NSWindow*)window { diff --git a/AppKit/GTMWindowSheetControllerTest.m b/AppKit/GTMWindowSheetControllerTest.m index b2cb7b2..65ef084 100644 --- a/AppKit/GTMWindowSheetControllerTest.m +++ b/AppKit/GTMWindowSheetControllerTest.m @@ -25,12 +25,16 @@ NSTabViewDelegate> { @private GTMWindowSheetController *sheetController_; + NSWindow* window_; BOOL didAlertClose_; BOOL didSheetClose_; } - (void)alertDidEnd:(NSAlert *)alert returnCode:(NSInteger)returnCode context:(void *)context; +- (void)openSecondSheet:(NSAlert *)alert + returnCode:(NSInteger)returnCode + context:(void *)context; - (void)sheetDidEnd:(NSWindow *)sheet returnCode:(NSInteger)returnCode context:(void *)context; @@ -172,12 +176,105 @@ STAssertTrue(didSheetClose_, @"Sheet should have closed"); } +- (void)testOpenSheetAfterFirst { + // Set up window + window_ = + [[[NSWindow alloc] initWithContentRect:NSMakeRect(100, 100, 600, 600) + styleMask:NSTitledWindowMask + backing:NSBackingStoreBuffered + defer:NO] autorelease]; + STAssertNotNil(window_, @"Could not allocate window"); + + sheetController_ = + [[[GTMWindowSheetController alloc] initWithWindow:window_ + delegate:self] autorelease]; + + STAssertFalse([sheetController_ isSheetAttachedToView: + [window_ contentView]], + @"Sheet should not be attached to current view"); + STAssertEquals([[sheetController_ viewsWithAttachedSheets] count], + (NSUInteger)0, + @"Should have no views with sheets"); + + // Pop alert on first tab + NSAlert* alert = [[NSAlert alloc] init]; + + [alert setMessageText:@"Hell Has Broken Loose."]; + [alert setInformativeText:@"All hell has broken loose. You may want to run " + @"outside screaming and waving your arms around " + @"wildly."]; + + NSButton *alertButton = [alert addButtonWithTitle:@"OK"]; + + // Allocate a second sheet to be launched by the alert + NSPanel *sheet = + [[[NSPanel alloc] initWithContentRect:NSMakeRect(0, 0, 300, 200) + styleMask:NSTitledWindowMask + backing:NSBackingStoreBuffered + defer:NO] autorelease]; + + [sheetController_ beginSystemSheet:alert + modalForView:[window_ contentView] + withParameters:[NSArray arrayWithObjects: + [NSNull null], + self, + [NSValue valueWithPointer: + @selector(openSecondSheet:returnCode:context:)], + [NSValue valueWithPointer:sheet], + nil]]; + didAlertClose_ = NO; + didSheetClose_ = NO; + + STAssertTrue([sheetController_ isSheetAttachedToView: + [window_ contentView]], + @"Sheet should be attached to view"); + STAssertEquals([[sheetController_ viewsWithAttachedSheets] count], + (NSUInteger)1, + @"Should have one view with sheets"); + + // Close alert + [alertButton performClick:self]; + + STAssertTrue([sheetController_ isSheetAttachedToView: + [window_ contentView]], + @"Second sheet should be attached to view"); + STAssertEquals([[sheetController_ viewsWithAttachedSheets] count], + (NSUInteger)1, + @"Should have one view with sheets"); + STAssertTrue(didAlertClose_, @"Alert should have closed"); + + // Close sheet + [[NSApplication sharedApplication] endSheet:sheet returnCode:NSOKButton]; + + STAssertFalse([sheetController_ isSheetAttachedToView: + [window_ contentView]], + @"Sheet should not be attached to current view"); + STAssertEquals([[sheetController_ viewsWithAttachedSheets] count], + (NSUInteger)0, + @"Should have no views with sheets"); + STAssertTrue(didSheetClose_, @"Sheet should have closed"); +} + - (void)alertDidEnd:(NSAlert *)alert returnCode:(NSInteger)returnCode context:(void *)context { didAlertClose_ = YES; } +- (void)openSecondSheet:(NSAlert *)alert + returnCode:(NSInteger)returnCode + context:(void *)context { + didAlertClose_ = YES; + + NSPanel* sheet = context; + // Pop a second sheet + [sheetController_ beginSheet:sheet + modalForView:[window_ contentView] + modalDelegate:self + didEndSelector:@selector(sheetDidEnd:returnCode:context:) + contextInfo:nil]; +} + - (void)sheetDidEnd:(NSWindow *)sheet returnCode:(NSInteger)returnCode context:(void *)context { -- cgit v1.2.3