Total guess fixes for the bug.

Add DCHECKs for main thread in all the places we use animation contexts.
Added a stack based class that takes an animate flag to enable/disable animation operations (grouping, durations) to clean up the code that does optional animation.
This might help the crashes in the bug because before we created animation context groups when we didn't need them and those empty groups could be tripping up something in the system frameworks (I some something similar in nib loading on 10.6).

BUG=26979
TEST=like we have a repeat fail case...
Review URL: http://codereview.chromium.org/390014

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@31781 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 58 insertions, 20 deletions

diff --git a/chrome/browser/autocomplete/autocomplete_popup_view_mac.mm b/chrome/browser/autocomplete/autocomplete_popup_view_mac.mm
index beaa239..38b7547 100644
--- a/chrome/browser/autocomplete/autocomplete_popup_view_mac.mm
+++ b/chrome/browser/autocomplete/autocomplete_popup_view_mac.mm
@@ -44,7 +44,7 @@ const CGFloat kLeftRightMargin = 8.0;
 const CGFloat kTextXOffset = 33.0;
 
 // Animation duration when animating the popup window smaller.
-const float kShrinkAnimationDuration = 0.1;
+const CGFloat kShrinkAnimationDuration = 0.1;
 
 // Maximum fraction of the popup width that can be used to display match
 // contents.
@@ -365,6 +365,7 @@ void AutocompletePopupViewMac::CreatePopupIfNeeded() {
 }
 
 void AutocompletePopupViewMac::UpdatePopupAppearance() {
+  DCHECK([NSThread isMainThread]);
   const AutocompleteResult& result = model_->result();
   if (result.empty()) {
     [[popup_ parentWindow] removeChildWindow:popup_];
diff --git a/chrome/browser/cocoa/download_shelf_controller.mm b/chrome/browser/cocoa/download_shelf_controller.mm
index e1faf3e..7c46e85 100644
--- a/chrome/browser/cocoa/download_shelf_controller.mm
+++ b/chrome/browser/cocoa/download_shelf_controller.mm
@@ -232,6 +232,7 @@ const NSTimeInterval kDownloadShelfCloseDuration = 0.12;
 }
 
 - (void)addDownloadItem:(BaseDownloadItemModel*)model {
+  DCHECK([NSThread isMainThread]);
   // Insert new item at the left.
   scoped_nsobject<DownloadItemController> controller(
       [[DownloadItemController alloc] initWithModel:model shelf:self]);
diff --git a/chrome/browser/cocoa/tab_strip_controller.mm b/chrome/browser/cocoa/tab_strip_controller.mm
index d6a03a0c..dfc70a7 100644
--- a/chrome/browser/cocoa/tab_strip_controller.mm
+++ b/chrome/browser/cocoa/tab_strip_controller.mm
@@ -51,6 +51,50 @@ static const float kIndentLeavingSpaceForControls = 64.0;
 // Time (in seconds) in which tabs animate to their final position.
 static const NSTimeInterval kAnimationDuration = 0.2;
 
+namespace {
+
+// Helper class for doing NSAnimationContext calls that takes a bool to disable
+// all the work.  Useful for code that wants to conditionally animate.
+class ScopedNSAnimationContextGroup {
+ public:
+  explicit ScopedNSAnimationContextGroup(bool animate)
+      : animate_(animate) {
+    if (animate_) {
+      [NSAnimationContext beginGrouping];
+    }
+  }
+
+  ~ScopedNSAnimationContextGroup() {
+    if (animate_) {
+      [NSAnimationContext endGrouping];
+    }
+  }
+
+  void SetCurrentContextDuration(NSTimeInterval duration) {
+    if (animate_) {
+      [[NSAnimationContext currentContext] gtm_setDuration:duration];
+    }
+  }
+
+  void SetCurrentContextShortestDuration() {
+    if (animate_) {
+      // The minimum representable time interval.  This used to stop an
+      // in-progress animation as quickly as possible.
+      const NSTimeInterval kMinimumTimeInterval =
+          std::numeric_limits<NSTimeInterval>::min();
+      // Directly set the duration to be short, avoiding the Steve slowmotion
+      // ettect the gtm_setDuration: provides.
+      [[NSAnimationContext currentContext] setDuration:kMinimumTimeInterval];
+    }
+  }
+
+private:
+  bool animate_;
+  DISALLOW_COPY_AND_ASSIGN(ScopedNSAnimationContextGroup);
+};
+
+}  // namespace
+
 @interface TabStripController(Private)
 - (void)installTrackingArea;
 - (void)addSubviewToPermanentList:(NSView*)aView;
@@ -531,14 +575,10 @@ static const NSTimeInterval kAnimationDuration = 0.2;
 // tabs would cause an overflow.
 - (void)layoutTabsWithAnimation:(BOOL)animate
              regenerateSubviews:(BOOL)doUpdate {
+  DCHECK([NSThread isMainThread]);
   if (![tabArray_ count])
     return;
 
-  // The minimum representable time interval.  This can be used as the value
-  // passed to +[NSAnimationContext setDuration:] to stop an in-progress
-  // animation as quickly as possible.
-  const NSTimeInterval kMinimumTimeInterval =
-      std::numeric_limits<NSTimeInterval>::min();
   const float kTabOverlap = 20.0;
   const float kNewTabButtonOffset = 8.0;
   const float kMaxTabWidth = [TabController maxTabWidth];
@@ -546,12 +586,12 @@ static const NSTimeInterval kAnimationDuration = 0.2;
   const float kMinSelectedTabWidth = [TabController minSelectedTabWidth];
 
   NSRect enclosingRect = NSZeroRect;
-  [NSAnimationContext beginGrouping];
-  [[NSAnimationContext currentContext] gtm_setDuration:kAnimationDuration];
+  ScopedNSAnimationContextGroup mainAnimationGroup(animate);
+  mainAnimationGroup.SetCurrentContextDuration(kAnimationDuration);
 
   // Update the current subviews and their z-order if requested.
   if (doUpdate)
-      [self regenerateSubviewList];
+    [self regenerateSubviewList];
 
   // Compute the base width of tabs given how much room we're allowed. We
   // may not be able to use the entire width if the user is quickly closing
@@ -603,14 +643,13 @@ static const NSTimeInterval kAnimationDuration = 0.2;
     if (isPlaceholder) {
       // Move the current tab to the correct location instantly.
       // We need a duration or else it doesn't cancel an inflight animation.
-      [NSAnimationContext beginGrouping];
-      [[NSAnimationContext currentContext] setDuration:kMinimumTimeInterval];
+      ScopedNSAnimationContextGroup localAnimationGroup(animate);
+      localAnimationGroup.SetCurrentContextShortestDuration();
       tabFrame.origin.x = placeholderFrame_.origin.x;
       // TODO(alcor): reenable this
       //tabFrame.size.height += 10.0 * placeholderStretchiness_;
       id target = animate ? [[tab view] animator] : [tab view];
       [target setFrame:tabFrame];
-      [NSAnimationContext endGrouping];
 
       // Store the frame by identifier to aviod redundant calls to animator.
       NSValue* identifier = [NSValue valueWithPointer:[tab view]];
@@ -699,15 +738,13 @@ static const NSTimeInterval kAnimationDuration = 0.2;
       // to use a very small duration to make sure we cancel any in-flight
       // animation to the left.
       if (visible && animate) {
-        [NSAnimationContext beginGrouping];
+        ScopedNSAnimationContextGroup localAnimationGroup(true);
         BOOL movingLeft = NSMinX(newTabNewFrame) < NSMinX(newTabTargetFrame_);
         if (!movingLeft) {
-          [[NSAnimationContext currentContext]
-              setDuration:kMinimumTimeInterval];
+          localAnimationGroup.SetCurrentContextShortestDuration();
         }
         [[newTabButton_ animator] setFrame:newTabNewFrame];
         newTabTargetFrame_ = newTabNewFrame;
-        [NSAnimationContext endGrouping];
       } else {
         [newTabButton_ setFrame:newTabNewFrame];
         newTabTargetFrame_ = newTabNewFrame;
@@ -715,7 +752,6 @@ static const NSTimeInterval kAnimationDuration = 0.2;
     }
   }
 
-  [NSAnimationContext endGrouping];
   [dragBlockingView_ setFrame:enclosingRect];
 
   // Mark that we've successfully completed layout of at least one tab.
@@ -887,6 +923,7 @@ static const NSTimeInterval kAnimationDuration = 0.2;
 // where to move the tab to. Registers a delegate to call back when the
 // animation is complete in order to remove the tab from the model.
 - (void)startClosingTabWithAnimation:(TabController*)closingTab {
+  DCHECK([NSThread isMainThread]);
   // Save off the controller into the set of animating tabs. This alerts
   // the layout method to not do anything with it and allows us to correctly
   // calculate offsets when working with indices into the model.
@@ -912,10 +949,9 @@ static const NSTimeInterval kAnimationDuration = 0.2;
   // Periscope down! Animate the tab.
   NSRect newFrame = [tabView frame];
   newFrame = NSOffsetRect(newFrame, 0, -newFrame.size.height);
-  [NSAnimationContext beginGrouping];
-  [[NSAnimationContext currentContext] gtm_setDuration:kAnimationDuration];
+  ScopedNSAnimationContextGroup animationGroup(true);
+  animationGroup.SetCurrentContextDuration(kAnimationDuration);
   [[tabView animator] setFrame:newFrame];
-  [NSAnimationContext endGrouping];
 }
 
 // Called when a notification is received from the model that the given tab