[Mac] Flip toolbar in RTL (reland)

This was reverted due to crbug.com/679174 which is fixed by using the
browser action container view's |animationEndFrame| instead of its
frame when calculating the size of the location bar.

Browser actions will be reordered in a future change.

BUG=648558, 648563, 673362

Review-Url: https://codereview.chromium.org/2607533004
Cr-Commit-Position: refs/heads/master@{#441925}
Committed: https://chromium.googlesource.com/chromium/src/+/8b0fd02afc2107ba123774e0a464a43407a108cf

patch from issue 2607533004 at patchset 80001 (http://crrev.com/2607533004#ps80001)

Review-Url: https://codereview.chromium.org/2631463004
Cr-Commit-Position: refs/heads/master@{#443345}

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,29 @@ 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_))
-    return;
+  const CGFloat maxWidth = std::min(maxAllowedWidth, maxDesiredWidth_);
+  CGFloat newWidth = NSWidth(containerFrame) + (isRTL ? dX : -dX);
+  newWidth = std::min(std::max(newWidth, kMinimumContainerWidth), maxWidth);
 
-  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];
@@ -271,14 +283,18 @@ const CGFloat kMinimumContainerWidth = 3.0;
 
 - (void)resizeToWidth:(CGFloat)width animate:(BOOL)animate {
   width = std::max(width, kMinimumContainerWidth);
-  NSRect frame = [self frame];
+  NSRect newFrame = [self frame];
 
   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);
+  if (cocoa_l10n_util::ShouldDoExperimentalRTLLayout()) {
+    newFrame.size.width = width;
+  } else {
+    CGFloat dX = NSWidth(newFrame) - width;
+    newFrame.size.width = width;
+    newFrame.origin.x += dX;
+  }
 
   grippyPinned_ = width == maxAllowedWidth;
 

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

@@ -20,6 +20,7 @@
 #import "chrome/browser/ui/cocoa/extensions/extension_popup_controller.h"
 #import "chrome/browser/ui/cocoa/extensions/toolbar_actions_bar_bubble_mac.h"
 #import "chrome/browser/ui/cocoa/image_button_cell.h"
+#import "chrome/browser/ui/cocoa/l10n_util.h"
 #import "chrome/browser/ui/cocoa/menu_button.h"
 #import "chrome/browser/ui/cocoa/toolbar/toolbar_controller.h"
 #include "chrome/browser/ui/extensions/extension_message_bubble_bridge.h"
@@ -581,11 +582,14 @@ void ToolbarActionsBarBridge::ShowToolbarActionBubble(
     if (NSMinX([button frameAfterAnimation]) == NSMinX(buttonFrame))
       continue;
 
-    // We set the x-origin by calculating the proper distance from the right
-    // edge in the container so that, if the container is animating, the
+    // In LTR, We set the x-origin by calculating the proper distance from the
+    // right edge in the container so that, if the container is animating, the
     // button appears stationary.
-    buttonFrame.origin.x = NSWidth([containerView_ frame]) -
-        (toolbarActionsBar_->GetPreferredSize().width() - NSMinX(buttonFrame));
+    if (!cocoa_l10n_util::ShouldDoExperimentalRTLLayout()) {
+      buttonFrame.origin.x = NSWidth([containerView_ frame]) -
+                             (toolbarActionsBar_->GetPreferredSize().width() -
+                              NSMinX(buttonFrame));
+    }
     [button setFrame:buttonFrame animate:NO];
   }
 }
@@ -783,9 +787,16 @@ void ToolbarActionsBarBridge::ShowToolbarActionBubble(
 }
 
 - (void)updateGrippyCursors {
+  BOOL canClose = [self visibleButtonCount] > 0;
+  BOOL canOpen = toolbarActionsBar_->GetIconCount() != [buttons_ count];
+  [containerView_
+      setCanDragLeft:cocoa_l10n_util::ShouldDoExperimentalRTLLayout()
+                         ? canClose
+                         : canOpen];
   [containerView_
-      setCanDragLeft:toolbarActionsBar_->GetIconCount() != [buttons_ count]];
-  [containerView_ setCanDragRight:[self visibleButtonCount] > 0];
+      setCanDragRight:cocoa_l10n_util::ShouldDoExperimentalRTLLayout()
+                          ? canOpen
+                          : canClose];
   [[containerView_ window] invalidateCursorRectsForView:containerView_];
 }
 

chrome/browser/ui/cocoa/toolbar/toolbar_controller.h

@@ -203,6 +203,10 @@ class NotificationBridge;
 - (void)installAppMenu;
 // Return a hover button for the current event.
 - (NSButton*)hoverButtonForEvent:(NSEvent*)theEvent;
+// Adjusts browser actions container view in response to toolbar frame changes.
+// Outside of tests, called in response to frame changed/new window
+// notifications.
+- (void)toolbarFrameChanged;
 @end
 
 #endif  // CHROME_BROWSER_UI_COCOA_TOOLBAR_TOOLBAR_CONTROLLER_H_

chrome/browser/ui/cocoa/toolbar/toolbar_controller_unittest.mm

@@ -4,25 +4,34 @@
 
 #import <Cocoa/Cocoa.h>
 
+#include "base/command_line.h"
 #import "base/mac/scoped_nsobject.h"
 #include "base/macros.h"
 #include "base/run_loop.h"
 #include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/command_updater.h"
+#include "chrome/browser/extensions/extension_action_test_util.h"
+#include "chrome/browser/extensions/extension_service.h"
+#include "chrome/browser/extensions/test_extension_system.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_command_controller.h"
 #include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/browser_list_observer.h"
 #import "chrome/browser/ui/cocoa/image_button_cell.h"
+#import "chrome/browser/ui/cocoa/extensions/browser_actions_controller.h"
 #import "chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.h"
 #import "chrome/browser/ui/cocoa/location_bar/translate_decoration.h"
 #include "chrome/browser/ui/cocoa/test/cocoa_profile_test.h"
+#include "chrome/browser/ui/cocoa/test/scoped_force_rtl_mac.h"
 #import "chrome/browser/ui/cocoa/toolbar/toolbar_controller.h"
+#include "chrome/browser/ui/toolbar/toolbar_actions_bar.h"
+#include "chrome/browser/ui/toolbar/toolbar_actions_model.h"
 #import "chrome/browser/ui/cocoa/view_resizer_pong.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/testing_profile.h"
 #include "components/prefs/pref_service.h"
+#include "extensions/browser/extension_system.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #import "testing/gtest_mac.h"
 #include "testing/platform_test.h"
@@ -74,14 +83,37 @@ class ToolbarControllerTest : public CocoaProfileTest {
   // Indexes that match the ordering returned by the private ToolbarController
   // |-toolbarViews| method.
   enum SubviewIndex {
-    kBackIndex, kForwardIndex, kReloadIndex, kHomeIndex,
-    kAppMenuIndex, kLocationIndex, kBrowserActionContainerViewIndex
+    kBackIndex,
+    kForwardIndex,
+    kReloadIndex,
+    kHomeIndex,
+    kLocationIndex,
+    kBrowserActionContainerViewIndex,
+    kAppMenuIndex
   };
 
   void SetUp() override {
     CocoaProfileTest::SetUp();
     ASSERT_TRUE(browser());
 
+    // Add an extension so the browser action container view
+    // is visible and has a real size/position.
+    extensions::TestExtensionSystem* extension_system =
+        static_cast<extensions::TestExtensionSystem*>(
+            extensions::ExtensionSystem::Get(profile()));
+    extension_system->CreateExtensionService(
+        base::CommandLine::ForCurrentProcess(), base::FilePath(), false);
+    scoped_refptr<const extensions::Extension> extension =
+        extensions::extension_action_test_util::CreateActionExtension(
+            "ABC", extensions::extension_action_test_util::BROWSER_ACTION);
+    extensions::ExtensionSystem::Get(profile())
+        ->extension_service()
+        ->AddExtension(extension.get());
+    ToolbarActionsModel* model =
+        extensions::extension_action_test_util::CreateToolbarModelForProfile(
+            profile());
+    model->SetVisibleIconCount(1);
+
     resizeDelegate_.reset([[ViewResizerPong alloc] init]);
 
     CommandUpdater* updater =
@@ -98,6 +130,12 @@ class ToolbarControllerTest : public CocoaProfileTest {
     EXPECT_TRUE([bar_ view]);
     NSView* parent = [test_window() contentView];
     [parent addSubview:[bar_ view]];
+
+    // Nudge a few things to ensure the browser actions container gets
+    // laid out.
+    [bar_ createBrowserActionButtons];
+    [[bar_ browserActionsController] update];
+    [bar_ toolbarFrameChanged];
   }
 
   void TearDown() override {
@@ -177,7 +215,7 @@ TEST_F(ToolbarControllerTest, UpdateVisibility) {
   EXPECT_FALSE([GetSubviewAt(kReloadIndex) isHidden]);
   EXPECT_FALSE([GetSubviewAt(kAppMenuIndex) isHidden]);
   EXPECT_TRUE([GetSubviewAt(kHomeIndex) isHidden]);
-  EXPECT_TRUE([GetSubviewAt(kBrowserActionContainerViewIndex) isHidden]);
+  EXPECT_FALSE([GetSubviewAt(kBrowserActionContainerViewIndex) isHidden]);
 
   // For NO/NO, only the top level toolbar view is hidden.
   [bar_ setHasToolbar:NO hasLocationBar:NO];
@@ -188,7 +226,7 @@ TEST_F(ToolbarControllerTest, UpdateVisibility) {
   EXPECT_FALSE([GetSubviewAt(kReloadIndex) isHidden]);
   EXPECT_FALSE([GetSubviewAt(kAppMenuIndex) isHidden]);
   EXPECT_TRUE([GetSubviewAt(kHomeIndex) isHidden]);
-  EXPECT_TRUE([GetSubviewAt(kBrowserActionContainerViewIndex) isHidden]);
+  EXPECT_FALSE([GetSubviewAt(kBrowserActionContainerViewIndex) isHidden]);
 
   // Now test the inescapable state.
   [bar_ setHasToolbar:NO hasLocationBar:YES];
@@ -391,6 +429,36 @@ TEST_F(ToolbarControllerTest, HoverButtonForEvent) {
   [bar_ setView:toolbarView];
 }
 
+// Test that subviews are ordered left to right
+TEST_F(ToolbarControllerTest, ElementOrder) {
+  NSArray* views = [bar_ toolbarViews];
+  for (size_t i = 1; i < [views count]; i++) {
+    NSView* previousSubview = views[i - 1];
+    NSView* subview = views[i];
+    EXPECT_LE(NSMinX([previousSubview frame]), NSMinX([subview frame]));
+  }
+}
+
+class ToolbarControllerRTLTest : public ToolbarControllerTest {
+ public:
+  ToolbarControllerRTLTest() {}
+
+ private:
+  cocoa_l10n_util::ScopedForceRTLMac rtl_;
+
+  DISALLOW_COPY_AND_ASSIGN(ToolbarControllerRTLTest);
+};
+
+// Test that subviews are ordered right to left
+TEST_F(ToolbarControllerRTLTest, ElementOrder) {
+  NSArray* views = [[[bar_ toolbarViews] reverseObjectEnumerator] allObjects];
+  for (size_t i = 1; i < [views count]; i++) {
+    NSView* previousSubview = views[i - 1];
+    NSView* subview = views[i];
+    EXPECT_LE(NSMinX([previousSubview frame]), NSMinX([subview frame]));
+  }
+}
+
 class BrowserRemovedObserver : public chrome::BrowserListObserver {
  public:
   BrowserRemovedObserver() { BrowserList::AddObserver(this); }