[Mac] Fix favicon disappearing in pinned tabs in RTL.

In RTL mode a tab transitioning from non-pinned to pinned ends up with
no visible favicon. This cl fixes this problem, and also uses the
correct tab width to calculate the iconview's position in non-pinned
RTL mode.

Bug: 789347
Change-Id: I972ef71c47811f9b2a5cf6ca2ac5f2c654d0c2bc
Reviewed-on: https://chromium-review.googlesource.com/795151Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Jayson Adams <shrike@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521835}

chrome/browser/ui/cocoa/tabs/tab_controller.mm

@@ -19,6 +19,7 @@
 #import "chrome/browser/ui/cocoa/sprite_view.h"
 #import "chrome/browser/ui/cocoa/tabs/alert_indicator_button_cocoa.h"
 #import "chrome/browser/ui/cocoa/tabs/tab_controller_target.h"
+#import "chrome/browser/ui/cocoa/tabs/tab_strip_controller.h"
 #import "chrome/browser/ui/cocoa/tabs/tab_view.h"
 #import "chrome/browser/ui/cocoa/themed_window.h"
 #import "extensions/common/extension.h"
@@ -317,20 +318,31 @@ static const CGFloat kPinnedTabWidth = kDefaultTabHeight * 2;
 }
 
 - (void)updateIconViewFrameWithAnimation:(BOOL)shouldAnimate {
-  NSRect iconViewFrame = [iconView_ frame];
-
-  if ([self pinned]) {
-    // Center the icon.
-    iconViewFrame.origin.x =
-        std::floor(([TabController pinnedTabWidth] - gfx::kFaviconSize) / 2.0);
-  } else {
-    BOOL isRTL = cocoa_l10n_util::ShouldDoExperimentalRTLLayout();
-    iconViewFrame.origin.x =
-        isRTL ? kInitialTabWidth - kTabLeadingPadding - gfx::kFaviconSize
-              : kTabLeadingPadding;
-  }
+  static const CGFloat kPinnedTabLeadingPadding =
+      std::floor((kPinnedTabWidth - gfx::kFaviconSize) / 2.0);
+  BOOL isRTL = cocoa_l10n_util::ShouldDoExperimentalRTLLayout();
 
-  if (shouldAnimate) {
+  // Determine the padding between the iconView and the tab edge.
+  CGFloat leadingPadding =
+      [self pinned] ? kPinnedTabLeadingPadding : kTabLeadingPadding;
+
+  NSRect iconViewFrame;
+  iconViewFrame.origin.x = isRTL ? NSWidth([[self tabView] frame]) -
+                                       leadingPadding - gfx::kFaviconSize
+                                 : leadingPadding;
+
+  // As long as the iconView gets repeatedly created and destroyed we have to
+  // initialize the other struct values. Once the iconView gets created a single
+  // time per tab we can rely on the values that get set in -init (and remove
+  // these lines).
+  iconViewFrame.origin.y = kTabElementYOrigin;
+  iconViewFrame.size = NSMakeSize(gfx::kFaviconSize, gfx::kFaviconSize);
+
+  // The iconView animation looks funky in RTL so don't allow it.
+  if (shouldAnimate && !isRTL) {
+    // Animate at the same rate as the tab changes shape.
+    [[NSAnimationContext currentContext]
+        setDuration:[TabStripController tabAnimationDuration]];
     [[iconView_ animator] setFrame:iconViewFrame];
   } else {
     [iconView_ setFrame:iconViewFrame];
@@ -465,24 +477,7 @@ static const CGFloat kPinnedTabWidth = kDefaultTabHeight * 2;
 
     [iconView_ setImage:image withToastAnimation:animate];
 
-    if ([self pinned]) {
-      NSRect appIconFrame = [iconView_ frame];
-
-      const CGFloat tabWidth = [TabController pinnedTabWidth];
-
-      // Center the icon.
-      appIconFrame.origin = NSMakePoint(
-          std::floor((tabWidth - gfx::kFaviconSize) / 2.0), kTabElementYOrigin);
-      [iconView_ setFrame:appIconFrame];
-    } else {
-      const CGFloat tabWidth = NSWidth([[self tabView] frame]);
-      const CGFloat iconOrigin =
-          isRTL ? tabWidth - gfx::kFaviconSize - kTabLeadingPadding
-                : kTabLeadingPadding;
-      NSRect iconFrame = NSMakeRect(iconOrigin, kTabElementYOrigin,
-                                    gfx::kFaviconSize, gfx::kFaviconSize);
-      [iconView_ setFrame:iconFrame];
-    }
+    [self updateIconViewFrameWithAnimation:NO];
   }
 
   [self updateAttentionIndicator];

chrome/browser/ui/cocoa/tabs/tab_strip_controller.h

@@ -165,6 +165,9 @@ class WebContents;
 
 @property(assign, nonatomic) TabView* hoveredTab;
 
+// Time (in seconds) in which tabs animate to their final position.
++ (CGFloat)tabAnimationDuration;
+
 // Initialize the controller with a view and browser that contains
 // everything else we'll need. |switchView| is the view whose contents get
 // "switched" every time the user switches tabs. The children of this view

chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm

@@ -93,7 +93,6 @@ const CGFloat kLastPinnedTabSpacing = 2.0;
 // The amount by which the new tab button is offset (from the tabs).
 const CGFloat kNewTabButtonOffset = 10.0;
 
-// Time (in seconds) in which tabs animate to their final position.
 const NSTimeInterval kAnimationDuration = 0.125;
 
 // Helper class for doing NSAnimationContext calls that takes a bool to disable
@@ -411,6 +410,10 @@ NSRect FlipRectInView(NSView* view, NSRect rect) {
 @synthesize leadingIndentForControls = leadingIndentForControls_;
 @synthesize trailingIndentForControls = trailingIndentForControls_;
 
++ (CGFloat)tabAnimationDuration {
+  return kAnimationDuration;
+}
+
 - (id)initWithView:(TabStripView*)view
         switchView:(NSView*)switchView
            browser:(Browser*)browser