Skip to content

Commit

Permalink
Give up on threaded animations for the Mac App Launcher NSWindow
Browse files Browse the repository at this point in the history
AppKit ignores -[NSView canDrawConcurrently:] and invokes redraws on the
animation thread.

The last attempt to make one part safe was r296591. However,
http://crbug.com/421952 links to undiagnosable crashes within AppKit
from a build at r298667. These are likely race conditions due to this
threading.

The downside is just that, when Chrome is not running, the initial
appearance of the app launcher window will be a bit delayed, and will
just "pop" in, rather than animate.

BUG=421952

Review URL: https://codereview.chromium.org/646353002

Cr-Commit-Position: refs/heads/master@{#299268}
  • Loading branch information
tapted authored and Commit bot committed Oct 13, 2014
1 parent 167ce43 commit 7e27128
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 17 deletions.
14 changes: 6 additions & 8 deletions chrome/browser/ui/app_list/app_list_service_mac.mm
Original file line number Diff line number Diff line change
Expand Up @@ -579,14 +579,12 @@ - (void)animateWindow:(NSWindow*)window
[animation_ setAnimationCurve:NSAnimationEaseOut];
window_.reset();
}
// Threaded animations are buggy on Snow Leopard. See http://crbug.com/335550.
// Note that in the non-threaded case, the animation won't start unless the
// UI runloop has spun up, so on <= Lion the animation will only animate if
// Chrome is already running.
if (base::mac::IsOSMountainLionOrLater())
[animation_ setAnimationBlockingMode:NSAnimationNonblockingThreaded];
else
[animation_ setAnimationBlockingMode:NSAnimationNonblocking];
// This once used a threaded animation, but AppKit would too often ignore
// -[NSView canDrawConcurrently:] and just redraw whole view hierarchies on
// the animation thread anyway, creating a minefield of race conditions.
// Non-threaded means the animation isn't as smooth and doesn't begin unless
// the UI runloop has spun up (after profile loading).
[animation_ setAnimationBlockingMode:NSAnimationNonblocking];

[animation_ startAnimation];
}
Expand Down
9 changes: 0 additions & 9 deletions ui/app_list/cocoa/apps_search_results_controller.mm
Original file line number Diff line number Diff line change
Expand Up @@ -402,15 +402,6 @@ - (AppsSearchResultsController*)controller {
[self delegate]);
}

- (BOOL)canDraw {
// AppKit doesn't call -[NSView canDrawConcurrently] which would have told it
// that this is unsafe. Returning true from canDraw only if there is a message
// loop ensures that no drawing occurs on a background thread. Without this,
// ImageSkia can assert when trying to get bitmaps. http://crbug.com/417148.
// This means unit tests will always return 'NO', but that's OK.
return !!base::MessageLoop::current() && [super canDraw];
}

- (void)mouseDown:(NSEvent*)theEvent {
[[self controller] mouseDown:theEvent];
[super mouseDown:theEvent];
Expand Down

0 comments on commit 7e27128

Please sign in to comment.