Fix Omnibox update problem on El Capitan.

chrome/browser/ui/cocoa/omnibox/omnibox_popup_view_mac.mm

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/cocoa/omnibox/omnibox_popup_view_mac.h"
 
 #include <cmath>
 
 #include "base/mac/mac_util.h"
+#import "base/mac/sdk_forward_declarations.h"
 #include "base/stl_util.h"
 #include "base/strings/sys_string_conversions.h"
 #include "chrome/browser/search/search.h"
 #include "chrome/browser/ui/cocoa/browser_window_controller.h"
 #import "chrome/browser/ui/cocoa/omnibox/omnibox_popup_cell.h"
 #import "chrome/browser/ui/cocoa/omnibox/omnibox_popup_separator_view.h"
 #include "chrome/browser/ui/cocoa/omnibox/omnibox_view_mac.h"
 #include "components/omnibox/browser/autocomplete_match.h"
 #include "components/omnibox/browser/autocomplete_match_type.h"
 #include "components/omnibox/browser/omnibox_edit_model.h"
@@ skipping 247 matching lines @@
     // running animations and set the new frame value immediately.  In practice,
     // zero-length animations are ignored entirely.  Work around this AppKit bug
     // by explicitly setting an NSNull animation for the "frame" key and then
     // running the animation with a non-zero(!!) duration.  This somehow
     // convinces AppKit to do the right thing.  Save off the current animations
     // dictionary so it can be restored later.
     savedAnimations.reset([[popup_ animations] copy]);
     [popup_ setAnimations:@{@"frame" : [NSNull null]}];
   }
 
-  if (!animate && base::mac::IsOSElCapitanOrLater()) {
-    // When using the animator to make |popup_| larger on El Capitan, for some
-    // reason the window does not get redrawn. There's no animation in this case
-    // anyway, so just force the frame change. See http://crbug.com/538590 .
-    [popup_ setFrame:popup_frame display:YES];
+  [NSAnimationContext beginGrouping];
+  // Don't use the GTM addition for the "Steve" slowdown because this can
+  // happen async from user actions and the effects could be a surprise.
+  [[NSAnimationContext currentContext] setDuration:kShrinkAnimationDuration];
+  // When using the animator to update |popup_| on El Capitan, for some reason
+  // the window does not get redrawn. Use a completion handler to make sure
+  // |popup_| gets redrawn once the animation completes. See
+  // http://crbug.com/538590 and http://crbug.com/551007 .
+  if (base::mac::IsOSElCapitanOrLater()) {
+    [[NSAnimationContext currentContext] setCompletionHandler:^{
+      [popup_ display];
+    }];
+    [[popup_ animator] setFrame:popup_frame display:NO];

[rohitrao (ping after 24h), 2015/11/11 20:51:27]

Why do we need display:NO here?

[shrike, 2015/11/11 21:40:18]

I was thinking that would avoid a redundant redraw, but after instrumenting the
code to see what gets drawn when it appears that you get the same number of
redraws regardless. I have updated the code to remove this extra line.

   } else {
-    [NSAnimationContext beginGrouping];
-    // Don't use the GTM addition for the "Steve" slowdown because this can
-    // happen async from user actions and the effects could be a surprise.
-    [[NSAnimationContext currentContext] setDuration:kShrinkAnimationDuration];
     [[popup_ animator] setFrame:popup_frame display:YES];
-    [NSAnimationContext endGrouping];
   }
+  [NSAnimationContext endGrouping];
 
   if (!animate) {
     // Restore the original animations dictionary.  This does not reinstate any
     // previously running animations.
     [popup_ setAnimations:savedAnimations];
   }
 }
 
 NSImage* OmniboxPopupViewMac::ImageForMatch(
     const AutocompleteMatch& match) const {
   gfx::Image image = model_->GetIconIfExtensionMatch(match);
   if (!image.IsEmpty())
     return image.AsNSImage();
 
   const int resource_id = model_->IsStarredMatch(match) ?
       IDR_OMNIBOX_STAR : AutocompleteMatch::TypeToIcon(match.type);
   return OmniboxViewMac::ImageForResource(resource_id);
 }
 
 void OmniboxPopupViewMac::OpenURLForRow(size_t row,
                                         WindowOpenDisposition disposition) {
   DCHECK_LT(row, GetResult().size());
   omnibox_view_->OpenMatch(GetResult().match_at(row), disposition, GURL(),
                            base::string16(), row);
 }