Add a switch for different toolbar layouts

This CL adds a switch used to change the positions of the toolbars
buttons, allowing to test different layouts.

The 3 layouts possible are:
- Navigation controls in the bottom and nothing in top toolbar (default)
- Navigation controls in the bottom and share button in the top toolbar
- Navigation controls in the top toolbar and bookmark/share in the
   bottom toolbar

Those layout will mainly change the visibility of the different buttons.
Also the second forward button is added to the toolbar views.

Bug: 824175
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I8e76b081038fb444bdac8f9e823dc023b4b9e883
Reviewed-on: https://chromium-review.googlesource.com/973609
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Reviewed-by: Eric Noyau <noyau@chromium.org>
Cr-Commit-Position: refs/heads/master@{#545015}

ios/chrome/browser/about_flags.mm

@@ -124,6 +124,13 @@ const FeatureEntry::Choice kAutofillIOSDelayBetweenFieldsChoices[] = {
     {"1000", autofill::switches::kAutofillIOSDelayBetweenFields, "1000"},
 };
 
+const FeatureEntry::Choice kToolbarButtonPositionsChoices[] = {
+    {flags_ui::kGenericExperimentChoiceDefault, "", ""},
+    {"Bottom navigation, no top", kToolbarButtonPositionsSwitch, "0"},
+    {"Bottom navigation, share on top", kToolbarButtonPositionsSwitch, "1"},
+    {"Top navigation", kToolbarButtonPositionsSwitch, "2"},
+};
+
 // To add a new entry, add to the end of kFeatureEntries. There are four
 // distinct types of entries:
 // . ENABLE_DISABLE_VALUE: entry is either enabled, disabled, or uses the
@@ -247,6 +254,9 @@ const flags_ui::FeatureEntry kFeatureEntries[] = {
     {"new-tools_menu", flag_descriptions::kNewToolsMenuName,
      flag_descriptions::kNewToolsMenuDescription, flags_ui::kOsIos,
      FEATURE_VALUE_TYPE(kNewToolsMenu)},
+    {"toolbar-button-positions", flag_descriptions::kToolbarButtonPositionsName,
+     flag_descriptions::kToolbarButtonPositionsDescription, flags_ui::kOsIos,
+     MULTI_VALUE_TYPE(kToolbarButtonPositionsChoices)},
 };
 
 // Add all switches from experimental flags to |command_line|.

ios/chrome/browser/ios_chrome_flag_descriptions.cc

@@ -126,6 +126,10 @@ const char kTabSwitcherPresentsBVCDescription[] =
     "BVC is visible, the tab switcher will remain in the VC hierarchy "
     "underneath it.";
 
+const char kToolbarButtonPositionsName[] = "Toolbar's buttons positions";
+const char kToolbarButtonPositionsDescription[] =
+    "Position of the toolbars buttons.";
+
 const char kUIRefreshPhase1Name[] = "UI Refresh Phase 1";
 const char kUIRefreshPhase1Description[] =
     "When enabled, the first phase of the iOS UI refresh will be displayed.";

ios/chrome/browser/ios_chrome_flag_descriptions.h

@@ -114,6 +114,11 @@ extern const char kShowAutofillTypePredictionsDescription[];
 extern const char kTabSwitcherPresentsBVCName[];
 extern const char kTabSwitcherPresentsBVCDescription[];
 
+// Title and description for the flag to choose the position of the toolbars'
+// buttons.
+extern const char kToolbarButtonPositionsName[];
+extern const char kToolbarButtonPositionsDescription[];
+
 // Title and description for the flag to enable the phase 1 UI Refresh.
 extern const char kUIRefreshPhase1Name[];
 extern const char kUIRefreshPhase1Description[];

ios/chrome/browser/ui/toolbar/adaptive/BUILD.gn

@@ -78,6 +78,7 @@ source_set("adaptive_ui") {
     "//ios/chrome/browser/ui/toolbar/buttons",
     "//ios/chrome/browser/ui/toolbar/clean:toolbar_ui",
     "//ios/chrome/browser/ui/toolbar/public",
+    "//ios/chrome/browser/ui/toolbar/public:toolbar_base_feature",
     "//ios/chrome/browser/ui/util",
     "//ios/chrome/browser/ui/util:constraints_ui",
     "//ios/third_party/material_components_ios",

ios/chrome/browser/ui/toolbar/adaptive/adaptive_toolbar_view.h

@@ -29,6 +29,10 @@
 @property(nonatomic, strong, readonly) ToolbarButton* backButton;
 // Buttons to navigate forward.
 @property(nonatomic, strong, readonly) ToolbarButton* forwardButton;
+// Button to navigate forward, positioned on the trailing side of the toolbar
+// relatively to the omnibox.
+@property(nonatomic, strong, readonly)
+    ToolbarButton* forwardButtonTrailingPosition;
 // Button to display the TabGrid.
 @property(nonatomic, strong, readonly) ToolbarTabGridButton* tabGridButton;
 // Button to stop the loading of the page.

ios/chrome/browser/ui/toolbar/adaptive/adaptive_toolbar_view_controller.mm

@@ -70,11 +70,13 @@
   self.view.toolsMenuButton.guideName = kToolsMenuGuide;
   self.view.tabGridButton.guideName = kTabSwitcherGuide;
   self.view.forwardButton.guideName = kForwardButtonGuide;
+  self.view.forwardButtonTrailingPosition.guideName = kForwardButtonGuide;
   self.view.backButton.guideName = kBackButtonGuide;
 
   // Add navigation popup menu triggers.
   [self addLongPressGestureToView:self.view.backButton];
   [self addLongPressGestureToView:self.view.forwardButton];
+  [self addLongPressGestureToView:self.view.forwardButtonTrailingPosition];
 }
 
 - (void)traitCollectionDidChange:(UITraitCollection*)previousTraitCollection {
@@ -92,6 +94,7 @@
 
 - (void)setCanGoForward:(BOOL)canGoForward {
   self.view.forwardButton.enabled = canGoForward;
+  self.view.forwardButtonTrailingPosition.enabled = canGoForward;
 }
 
 - (void)setCanGoBack:(BOOL)canGoBack {
@@ -176,12 +179,14 @@
     self.view.backButton.selected = YES;
   } else {
     self.view.forwardButton.selected = YES;
+    self.view.forwardButtonTrailingPosition.selected = YES;
   }
 }
 
 - (void)updateUIForTabHistoryWasDismissed {
   self.view.backButton.selected = NO;
   self.view.forwardButton.selected = NO;
+  self.view.forwardButtonTrailingPosition.selected = NO;
 }
 
 #pragma mark - Private
@@ -216,7 +221,8 @@
 
   if (sender == self.view.backButton) {
     base::RecordAction(base::UserMetricsAction("MobileToolbarBack"));
-  } else if (sender == self.view.forwardButton) {
+  } else if (sender == self.view.forwardButton ||
+             sender == self.view.forwardButtonTrailingPosition) {
     base::RecordAction(base::UserMetricsAction("MobileToolbarForward"));
   } else if (sender == self.view.reloadButton) {
     base::RecordAction(base::UserMetricsAction("MobileToolbarReload"));
@@ -257,7 +263,8 @@
     } else {
       [self.dispatcher showTabHistoryPopupForBackwardHistory];
     }
-  } else if (gesture.view == self.view.forwardButton) {
+  } else if (gesture.view == self.view.forwardButton ||
+             gesture.view == self.view.forwardButtonTrailingPosition) {
     if (base::FeatureList::IsEnabled(kNewToolsMenu)) {
       [self.dispatcher showNavigationHistoryForwardPopupMenu];
     } else {

ios/chrome/browser/ui/toolbar/adaptive/primary_toolbar_view.mm

@@ -11,6 +11,7 @@
 #import "ios/chrome/browser/ui/toolbar/buttons/toolbar_constants.h"
 #import "ios/chrome/browser/ui/toolbar/buttons/toolbar_tab_grid_button.h"
 #import "ios/chrome/browser/ui/toolbar/buttons/toolbar_tools_menu_button.h"
+#import "ios/chrome/browser/ui/toolbar/public/toolbar_controller_base_feature.h"
 #import "ios/chrome/browser/ui/util/constraints_ui_util.h"
 #import "ios/third_party/material_components_ios/src/components/ProgressView/src/MaterialProgressView.h"
 
@@ -52,6 +53,10 @@
 @property(nonatomic, strong, readwrite) ToolbarButton* backButton;
 // Button to navigate forward, redefined as readwrite.
 @property(nonatomic, strong, readwrite) ToolbarButton* forwardButton;
+// Button to navigate forward, positioned on the trailing side of the toolbar
+// relatively to the omnibox, redefined as readwrite.
+@property(nonatomic, strong, readwrite)
+    ToolbarButton* forwardButtonTrailingPosition;
 // Button to display the TabGrid, redefined as readwrite.
 @property(nonatomic, strong, readwrite) ToolbarTabGridButton* tabGridButton;
 // Button to stop the loading of the page, redefined as readwrite.
@@ -92,6 +97,7 @@
 @synthesize leadingStackViewButtons = _leadingStackViewButtons;
 @synthesize backButton = _backButton;
 @synthesize forwardButton = _forwardButton;
+@synthesize forwardButtonTrailingPosition = _forwardButtonTrailingPosition;
 @synthesize tabGridButton = _tabGridButton;
 @synthesize stopButton = _stopButton;
 @synthesize reloadButton = _reloadButton;
@@ -222,12 +228,16 @@
 
 // Sets the trailing stack view.
 - (void)setUpTrailingStackView {
+  self.forwardButtonTrailingPosition =
+      [self.buttonFactory forwardButtonTrailingPosition];
   self.shareButton = [self.buttonFactory shareButton];
   self.bookmarkButton = [self.buttonFactory bookmarkButton];
   self.toolsMenuButton = [self.buttonFactory toolsMenuButton];
 
-  self.trailingStackViewButtons =
-      @[ self.shareButton, self.bookmarkButton, self.toolsMenuButton ];
+  self.trailingStackViewButtons = @[
+    self.forwardButtonTrailingPosition, self.shareButton, self.bookmarkButton,
+    self.toolsMenuButton
+  ];
   self.trailingStackView = [[UIStackView alloc]
       initWithArrangedSubviews:self.trailingStackViewButtons];
   self.trailingStackView.translatesAutoresizingMaskIntoConstraints = NO;
@@ -285,14 +295,43 @@
         constraintEqualToAnchor:self.leadingStackView.trailingAnchor
                        constant:kContractedLocationBarHorizontalMargin],
   ]];
-  [self.contractedNoMarginConstraints addObjectsFromArray:@[
-    [self.locationBarContainer.leadingAnchor
-        constraintEqualToAnchor:safeArea.leadingAnchor
-                       constant:kExpandedLocationBarHorizontalMargin],
-    [self.locationBarContainer.trailingAnchor
-        constraintEqualToAnchor:safeArea.trailingAnchor
-                       constant:-kExpandedLocationBarHorizontalMargin]
-  ]];
+
+  // Constraints for contractedNoMarginConstraints, depending on the flag.
+  switch (PositionForCurrentProcess()) {
+    case ToolbarButtonPositionNavigationBottomNoTop:
+      [self.contractedNoMarginConstraints addObjectsFromArray:@[
+        [self.locationBarContainer.leadingAnchor
+            constraintEqualToAnchor:safeArea.leadingAnchor
+                           constant:kExpandedLocationBarHorizontalMargin],
+        [self.locationBarContainer.trailingAnchor
+            constraintEqualToAnchor:safeArea.trailingAnchor
+                           constant:-kExpandedLocationBarHorizontalMargin]
+      ]];
+
+      break;
+
+    case ToolbarButtonPositionNavigationBottomShareTop:
+      [self.contractedNoMarginConstraints addObjectsFromArray:@[
+        [self.locationBarContainer.leadingAnchor
+            constraintEqualToAnchor:safeArea.leadingAnchor
+                           constant:kExpandedLocationBarHorizontalMargin],
+        [self.locationBarContainer.trailingAnchor
+            constraintEqualToAnchor:self.trailingStackView.leadingAnchor
+                           constant:-kExpandedLocationBarHorizontalMargin],
+      ]];
+      break;
+    case ToolbarButtonPositionNavigationTop:
+      [self.contractedNoMarginConstraints addObjectsFromArray:@[
+        [self.locationBarContainer.leadingAnchor
+            constraintEqualToAnchor:self.leadingStackView.trailingAnchor
+                           constant:kContractedLocationBarHorizontalMargin],
+        [self.locationBarContainer.trailingAnchor
+            constraintEqualToAnchor:self.trailingStackView.leadingAnchor
+                           constant:-kContractedLocationBarHorizontalMargin],
+      ]];
+      break;
+  }
+
   [self.expandedConstraints addObjectsFromArray:@[
     [self.locationBarContainer.trailingAnchor
         constraintEqualToAnchor:self.cancelButton.leadingAnchor],

ios/chrome/browser/ui/toolbar/adaptive/secondary_toolbar_view.mm

@@ -39,6 +39,10 @@
 @property(nonatomic, strong, readwrite) ToolbarTabGridButton* tabGridButton;
 // Button to focus the omnibox, redefined as readwrite.
 @property(nonatomic, strong, readwrite) ToolbarButton* omniboxButton;
+// Button to display the share menu, redefined as readwrite.
+@property(nonatomic, strong, readwrite) ToolbarButton* shareButton;
+// Button to manage the bookmarks of this page, redefined as readwrite.
+@property(nonatomic, strong, readwrite) ToolbarButton* bookmarkButton;
 
 @end
 
@@ -52,6 +56,8 @@
 @synthesize toolsMenuButton = _toolsMenuButton;
 @synthesize omniboxButton = _omniboxButton;
 @synthesize tabGridButton = _tabGridButton;
+@synthesize shareButton = _shareButton;
+@synthesize bookmarkButton = _bookmarkButton;
 @synthesize blur = _blur;
 
 #pragma mark - Public
@@ -106,10 +112,12 @@
   self.omniboxButton = [self.buttonFactory omniboxButton];
   self.tabGridButton = [self.buttonFactory tabGridButton];
   self.toolsMenuButton = [self.buttonFactory toolsMenuButton];
+  self.shareButton = [self.buttonFactory shareButton];
+  self.bookmarkButton = [self.buttonFactory bookmarkButton];
 
   self.allButtons = @[
-    self.backButton, self.forwardButton, self.omniboxButton, self.tabGridButton,
-    self.toolsMenuButton
+    self.backButton, self.forwardButton, self.bookmarkButton, self.shareButton,
+    self.omniboxButton, self.tabGridButton, self.toolsMenuButton
   ];
 
   self.stackView =
@@ -135,23 +143,19 @@
 
 #pragma mark - AdaptiveToolbarView
 
-- (ToolbarButton*)stopButton {
-  return nil;
-}
-
-- (ToolbarButton*)reloadButton {
+- (ToolbarButton*)forwardButtonTrailingPosition {
   return nil;
 }
 
-- (MDCProgressView*)progressBar {
+- (ToolbarButton*)stopButton {
   return nil;
 }
 
-- (ToolbarButton*)bookmarkButton {
+- (ToolbarButton*)reloadButton {
   return nil;
 }
 
-- (ToolbarButton*)shareButton {
+- (MDCProgressView*)progressBar {
   return nil;
 }
 

ios/chrome/browser/ui/toolbar/buttons/toolbar_button_factory.h

@@ -43,6 +43,8 @@
 - (ToolbarButton*)backButton;
 // Forward ToolbarButton.
 - (ToolbarButton*)forwardButton;
+// Forward ToolbarButton, to be positioned on the trailing side of the toolbar.
+- (ToolbarButton*)forwardButtonTrailingPosition;
 // Tab Grid ToolbarButton.
 - (ToolbarTabGridButton*)tabGridButton;
 // StackView ToolbarButton.

ios/chrome/browser/ui/toolbar/buttons/toolbar_button_factory.mm

@@ -140,6 +140,22 @@ const int styleCount = 2;
   return forwardButton;
 }
 
+- (ToolbarButton*)forwardButtonTrailingPosition {
+  DCHECK(IsUIRefreshPhase1Enabled());
+  ToolbarButton* forwardButton = [ToolbarButton
+      toolbarButtonWithImage:[[UIImage imageNamed:@"toolbar_forward"]
+                                 imageFlippedForRightToLeftLayoutDirection]];
+  [self configureButton:forwardButton width:kAdaptiveToolbarButtonWidth];
+  forwardButton.visibilityMask =
+      self.visibilityConfiguration.forwardButtonTrailingPositionVisibility;
+  forwardButton.accessibilityLabel =
+      l10n_util::GetNSString(IDS_ACCNAME_FORWARD);
+  [forwardButton addTarget:self.dispatcher
+                    action:@selector(goForward)
+          forControlEvents:UIControlEventTouchUpInside];
+  return forwardButton;
+}
+
 - (ToolbarTabGridButton*)tabGridButton {
   DCHECK(IsUIRefreshPhase1Enabled());
   ToolbarTabGridButton* tabGridButton = [ToolbarTabGridButton

ios/chrome/browser/ui/toolbar/buttons/toolbar_button_visibility_configuration.h

@@ -27,6 +27,8 @@
 // toolbar.
 @property(nonatomic, readonly)
     ToolbarComponentVisibility forwardButtonVisibility;
+@property(nonatomic, readonly)
+    ToolbarComponentVisibility forwardButtonTrailingPositionVisibility;
 @property(nonatomic, readonly)
     ToolbarComponentVisibility tabGridButtonVisibility;
 @property(nonatomic, readonly)

ios/chrome/browser/ui/toolbar/buttons/toolbar_button_visibility_configuration.mm

@@ -4,6 +4,8 @@
 
 #import "ios/chrome/browser/ui/toolbar/buttons/toolbar_button_visibility_configuration.h"
 
+#import "ios/chrome/browser/ui/toolbar/public/toolbar_controller_base_feature.h"
+
 #if !defined(__has_feature) || !__has_feature(objc_arc)
 #error "This file requires ARC support."
 #endif
@@ -23,10 +25,18 @@
 - (ToolbarComponentVisibility)backButtonVisibility {
   switch (self.type) {
     case PRIMARY:
-      return ToolbarComponentVisibilityAlways &
-             ~ToolbarComponentVisibilitySplit;
+      if (PositionForCurrentProcess() == ToolbarButtonPositionNavigationTop) {
+        return ToolbarComponentVisibilityAlways;
+      } else {
+        return ToolbarComponentVisibilityAlways &
+               ~ToolbarComponentVisibilitySplit;
+      }
     case SECONDARY:
-      return ToolbarComponentVisibilitySplit;
+      if (PositionForCurrentProcess() == ToolbarButtonPositionNavigationTop) {
+        return ToolbarComponentVisibilityNone;
+      } else {
+        return ToolbarComponentVisibilitySplit;
+      }
     case LEGACY:
       return ToolbarComponentVisibilityAlways;
   }
@@ -38,7 +48,27 @@
       return ToolbarComponentVisibilityAlways &
              ~ToolbarComponentVisibilitySplit;
     case SECONDARY:
-      return ToolbarComponentVisibilitySplit;
+      if (PositionForCurrentProcess() == ToolbarButtonPositionNavigationTop) {
+        return ToolbarComponentVisibilityNone;
+      } else {
+        return ToolbarComponentVisibilitySplit;
+      }
+    case LEGACY:
+      return ToolbarComponentVisibilityOnlyWhenEnabled |
+             ToolbarComponentVisibilityRegularWidthRegularHeight;
+  }
+}
+
+- (ToolbarComponentVisibility)forwardButtonTrailingPositionVisibility {
+  switch (self.type) {
+    case PRIMARY:
+      if (PositionForCurrentProcess() == ToolbarButtonPositionNavigationTop) {
+        return ToolbarComponentVisibilitySplit;
+      } else {
+        return ToolbarComponentVisibilityNone;
+      }
+    case SECONDARY:
+      return ToolbarComponentVisibilityNone;
     case LEGACY:
       return ToolbarComponentVisibilityOnlyWhenEnabled |
              ToolbarComponentVisibilityRegularWidthRegularHeight;
@@ -72,10 +102,19 @@
 - (ToolbarComponentVisibility)shareButtonVisibility {
   switch (self.type) {
     case PRIMARY:
-      return ToolbarComponentVisibilityAlways &
-             ~ToolbarComponentVisibilityCompactWidthRegularHeight;
+      if (PositionForCurrentProcess() ==
+          ToolbarButtonPositionNavigationBottomShareTop) {
+        return ToolbarComponentVisibilityAlways;
+      } else {
+        return ToolbarComponentVisibilityAlways &
+               ~ToolbarComponentVisibilitySplit;
+      }
     case SECONDARY:
-      return ToolbarComponentVisibilityNone;
+      if (PositionForCurrentProcess() == ToolbarButtonPositionNavigationTop) {
+        return ToolbarComponentVisibilitySplit;
+      } else {
+        return ToolbarComponentVisibilityNone;
+      }
     case LEGACY:
       return ToolbarComponentVisibilityRegularWidthRegularHeight;
   }
@@ -109,7 +148,11 @@
       return ToolbarComponentVisibilityAlways &
              ~ToolbarComponentVisibilityCompactWidthRegularHeight;
     case SECONDARY:
-      return ToolbarComponentVisibilityNone;
+      if (PositionForCurrentProcess() == ToolbarButtonPositionNavigationTop) {
+        return ToolbarComponentVisibilitySplit;
+      } else {
+        return ToolbarComponentVisibilityNone;
+      }
     case LEGACY:
       return ToolbarComponentVisibilityRegularWidthCompactHeight |
              ToolbarComponentVisibilityRegularWidthRegularHeight;

ios/chrome/browser/ui/toolbar/public/toolbar_controller_base_feature.h

@@ -5,10 +5,24 @@
 #ifndef IOS_CHROME_BROWSER_UI_TOOLBAR_TOOLBAR_CONTROLLER_BASE_FEATURE_H_
 #define IOS_CHROME_BROWSER_UI_TOOLBAR_TOOLBAR_CONTROLLER_BASE_FEATURE_H_
 
+#import <Foundation/Foundation.h>
+
 #include "base/feature_list.h"
 
+// Enum for the different positions of the toolbars' buttons.
+typedef NS_ENUM(NSUInteger, ToolbarButtonPosition) {
+  ToolbarButtonPositionNavigationBottomNoTop,
+  ToolbarButtonPositionNavigationBottomShareTop,
+  ToolbarButtonPositionNavigationTop,
+};
+
 // Feature to choose whether to use the memex prototype tab switcher or the
 // regular native tab switcher.
 extern const base::Feature kMemexTabSwitcher;
 
+// Switch with different values for the layout of the toolbar buttons.
+extern const char kToolbarButtonPositionsSwitch[];
+
+ToolbarButtonPosition PositionForCurrentProcess();
+
 #endif  // IOS_CHROME_BROWSER_UI_TOOLBAR_TOOLBAR_CONTROLLER_BASE_FEATURE_H_

ios/chrome/browser/ui/toolbar/public/toolbar_controller_base_feature.mm

@@ -4,9 +4,30 @@
 
 #import "ios/chrome/browser/ui/toolbar/public/toolbar_controller_base_feature.h"
 
+#include "base/command_line.h"
+
 #if !defined(__has_feature) || !__has_feature(objc_arc)
 #error "This file requires ARC support."
 #endif
 
 const base::Feature kMemexTabSwitcher{"MemexTabSwitcher",
                                       base::FEATURE_DISABLED_BY_DEFAULT};
+
+const char kToolbarButtonPositionsSwitch[] = "toolbar-buttons-positions-switch";
+
+ToolbarButtonPosition PositionForCurrentProcess() {
+  const base::CommandLine* command_line =
+      base::CommandLine::ForCurrentProcess();
+  if (command_line->HasSwitch(kToolbarButtonPositionsSwitch)) {
+    if (command_line->GetSwitchValueASCII(kToolbarButtonPositionsSwitch) ==
+        "1") {
+      return ToolbarButtonPositionNavigationBottomShareTop;
+    } else if (command_line->GetSwitchValueASCII(
+                   kToolbarButtonPositionsSwitch) == "2") {
+      return ToolbarButtonPositionNavigationTop;
+    } else {
+      return ToolbarButtonPositionNavigationBottomNoTop;
+    }
+  }
+  return ToolbarButtonPositionNavigationBottomNoTop;
+}