[Mac] Flip toolbar in RTL

chrome/browser/ui/cocoa/extensions/browser_actions_container_view.mm

Index: chrome/browser/ui/cocoa/extensions/browser_actions_container_view.mm
diff --git a/chrome/browser/ui/cocoa/extensions/browser_actions_container_view.mm b/chrome/browser/ui/cocoa/extensions/browser_actions_container_view.mm
index 97ed1a6ea1085fd73592af01ce133792e3657b81..d28f03a8e23c45b6a6a5a9fbcf22f8a646bd8c34 100644
--- a/chrome/browser/ui/cocoa/extensions/browser_actions_container_view.mm
+++ b/chrome/browser/ui/cocoa/extensions/browser_actions_container_view.mm
@@ -7,6 +7,7 @@
 #include <algorithm>
 #include <utility>
 
+#import "chrome/browser/ui/cocoa/l10n_util.h"
 #import "chrome/browser/ui/cocoa/view_id_util.h"
 #include "ui/base/cocoa/appkit_utils.h"
 #include "ui/events/keycodes/keyboard_code_conversion_mac.h"
@@ -58,6 +59,9 @@ const CGFloat kMinimumContainerWidth = 3.0;
 - (id)initWithFrame:(NSRect)frameRect {
   if ((self = [super initWithFrame:frameRect])) {
     grippyRect_ = NSMakeRect(0.0, 0.0, kGrippyWidth, NSHeight([self bounds]));
+    if (cocoa_l10n_util::ShouldDoExperimentalRTLLayout())
+      grippyRect_.origin.x = NSWidth(frameRect) - NSWidth(grippyRect_);
+
     canDragLeft_ = YES;
     canDragRight_ = YES;
     resizable_ = YES;
@@ -162,6 +166,8 @@ const CGFloat kMinimumContainerWidth = 3.0;
 }
 
 - (void)resetCursorRects {
+  if (cocoa_l10n_util::ShouldDoExperimentalRTLLayout())
+    grippyRect_.origin.x = NSWidth([self frame]) - NSWidth(grippyRect_);
   [self addCursorRect:grippyRect_ cursor:[self appropriateCursorForGrippy]];
 }
 
@@ -214,23 +220,31 @@ const CGFloat kMinimumContainerWidth = 3.0;
   NSRect containerFrame = [self frame];
   CGFloat dX = [theEvent deltaX];
   CGFloat withDelta = location.x - dX;
-  canDragRight_ = (withDelta >= initialDragPoint_.x) &&
-      (NSWidth(containerFrame) > kMinimumContainerWidth);
+  BOOL isRTL = cocoa_l10n_util::ShouldDoExperimentalRTLLayout();
+
   CGFloat maxAllowedWidth = [self maxAllowedWidth];
-  containerFrame.size.width =
-      std::max(NSWidth(containerFrame) - dX, kMinimumContainerWidth);
-  canDragLeft_ = withDelta <= initialDragPoint_.x &&
-      NSWidth(containerFrame) < maxDesiredWidth_ &&
-      NSWidth(containerFrame) < maxAllowedWidth;
 
-  if ((dX < 0.0 && !canDragLeft_) || (dX > 0.0 && !canDragRight_))
+  const CGFloat maxWidth = std::min(maxAllowedWidth, maxDesiredWidth_);
+  CGFloat newWidth = NSWidth(containerFrame) + (isRTL ? dX : -dX);
+  newWidth = std::min(std::max(newWidth, kMinimumContainerWidth), maxWidth);
+  if (fabs(newWidth - NSWidth(containerFrame)) < FLT_EPSILON)
     return;

[tapted, 2017/01/05 23:09:36]

hehe yeah - I was wondering about this. I thought about suggesting an early
return on the first patchset, but saw some other data members changing.

Of course the lines

  if (NSWidth(containerFrame) <= kMinimumContainerWidth)
    return;

are now gone. I may have missed something, but I'm pretty sure that's redundant
with the other checks. So this all looks fine.

[lgrey, 2017/01/06 11:52:19]

What I ended up finding with this one is things would get screwed up for some
reason that wasn't easily apparent to me. The one thing I can think of is that
FLT_EPSILON is a valid delta at the rate this gets called!? Doesn't seem right
to me, but since I added this in the first place, I figured it was fine to take
back out.

 if (NSWidth(containerFrame) <= kMinimumContainerWidth)
    return;

is basically redundant with clipping to kMinimumContainerWidth. I'm not sure why
we would bail if it's equal though, so I guess this can be 1 pixel smaller than
the previous version -- IMO, we should increase the minimum by 1 if that's a
problem.

 
-  if (NSWidth(containerFrame) <= kMinimumContainerWidth)
+  BOOL canGrow = NSWidth(containerFrame) < maxWidth;
+  BOOL canShrink = NSWidth(containerFrame) > kMinimumContainerWidth;
+
+  canDragLeft_ =
+      withDelta <= initialDragPoint_.x && (isRTL ? canShrink : canGrow);
+  canDragRight_ =
+      (withDelta >= initialDragPoint_.x) && (isRTL ? canGrow : canShrink);
+  if ((dX < 0.0 && !canDragLeft_) || (dX > 0.0 && !canDragRight_) ||
+      fabs(dX) < FLT_EPSILON)
     return;
 
-  grippyPinned_ = NSWidth(containerFrame) >= maxAllowedWidth;
-  containerFrame.origin.x += dX;
+  grippyPinned_ = newWidth >= maxAllowedWidth;
+  if (!isRTL)
+    containerFrame.origin.x += dX;
+  containerFrame.size.width = newWidth;
 
   [self setFrame:containerFrame];
   [self setNeedsDisplay:YES];
@@ -276,9 +290,15 @@ const CGFloat kMinimumContainerWidth = 3.0;
   CGFloat maxAllowedWidth = [self maxAllowedWidth];
   width = std::min(maxAllowedWidth, width);
 
-  CGFloat dX = frame.size.width - width;
-  frame.size.width = width;
-  NSRect newFrame = NSOffsetRect(frame, dX, 0);
+  NSRect newFrame;
+  if (cocoa_l10n_util::ShouldDoExperimentalRTLLayout()) {
+    newFrame = frame;
+    newFrame.size.width = width;
+  } else {
+    CGFloat dX = frame.size.width - width;
+    frame.size.width = width;
+    newFrame = NSOffsetRect(frame, dX, 0);
+  }
 
   grippyPinned_ = width == maxAllowedWidth;