iPhone toolbar behave the same in portrait and landscape

This CL changes the toolbar so when the location bar is focused in
iPhone landscape, the cancel button is shown, as it is done in
portrait.

Bug: 847847
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I351d166e52d80975579bf6c74fd44230543b4ce1
Reviewed-on: https://chromium-review.googlesource.com/1107634
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Reviewed-by: Olivier Robin <olivierrobin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568852}

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

@@ -37,6 +37,9 @@
 
 namespace {
 
+using chrome_test_util::BackButton;
+using chrome_test_util::ForwardButton;
+
 const char kPageURL[] = "/test-page.html";
 const char kPageURL2[] = "/test-page-2.html";
 const char kPageURL3[] = "/test-page-3.html";
@@ -44,6 +47,13 @@ const char kLinkID[] = "linkID";
 const char kTextID[] = "textID";
 const char kPageLoadedString[] = "Page loaded!";
 
+// Defines the visibility of an element, in relation to the toolbar.
+typedef NS_ENUM(NSInteger, ButtonVisibility) {
+  ButtonVisibilityNone,
+  ButtonVisibilityPrimary,
+  ButtonVisibilitySecondary
+};
+
 // Provides responses for redirect and changed window location URLs.
 std::unique_ptr<net::test_server::HttpResponse> StandardResponse(
     const net::test_server::HttpRequest& request) {
@@ -79,6 +89,34 @@ id<GREYMatcher> ShareButton() {
                     grey_sufficientlyVisible(), nil);
 }
 
+// Returns a matcher for the reload button.
+id<GREYMatcher> ReloadButton() {
+  return chrome_test_util::ButtonWithAccessibilityLabelId(
+      IDS_IOS_ACCNAME_RELOAD);
+}
+
+// Returns a matcher for the tools menu button.
+id<GREYMatcher> ToolsMenuButton() {
+  return chrome_test_util::ButtonWithAccessibilityLabelId(
+      IDS_IOS_TOOLBAR_SETTINGS);
+}
+
+// Returns a matcher for the cancel button.
+id<GREYMatcher> CancelButton() {
+  return chrome_test_util::ButtonWithAccessibilityLabelId(IDS_CANCEL);
+}
+
+// Returns a matcher for the search button.
+id<GREYMatcher> SearchButton() {
+  return grey_accessibilityID(kToolbarOmniboxButtonIdentifier);
+}
+
+// Returns a matcher for the tab grid button.
+id<GREYMatcher> TabGridButton() {
+  return chrome_test_util::ButtonWithAccessibilityLabelId(
+      IDS_IOS_TOOLBAR_SHOW_TABS);
+}
+
 // Returns a matcher for a UIResponder object being first responder.
 id<GREYMatcher> firstResponder() {
   MatchesBlock matches = ^BOOL(UIResponder* responder) {
@@ -109,6 +147,26 @@ id<GREYMatcher> VisibleInSecondaryToolbar() {
       grey_sufficientlyVisible(), nil);
 }
 
+// Checks that the element designated by |matcher| is |visible| in the primary
+// toolbar.
+void CheckVisibleInPrimaryToolbar(id<GREYMatcher> matcher, BOOL visible) {
+  id<GREYMatcher> assertionMatcher = visible ? grey_notNil() : grey_nil();
+  [[EarlGrey
+      selectElementWithMatcher:grey_allOf(matcher, VisibleInPrimaryToolbar(),
+                                          nil)]
+      assertWithMatcher:assertionMatcher];
+}
+
+// Checks that the element designed by |matcher| is |visible| in the secondary
+// toolbar.
+void CheckVisibleInSecondaryToolbar(id<GREYMatcher> matcher, BOOL visible) {
+  id<GREYMatcher> assertionMatcher = visible ? grey_notNil() : grey_nil();
+  [[EarlGrey
+      selectElementWithMatcher:grey_allOf(matcher, VisibleInSecondaryToolbar(),
+                                          nil)]
+      assertWithMatcher:assertionMatcher];
+}
+
 // Returns a matcher for a UIControl object being spotlighted.
 id<GREYMatcher> Spotlighted() {
   MatchesBlock matches = ^BOOL(UIControl* control) {
@@ -160,117 +218,135 @@ UITraitCollection* RotateOrChangeTraitCollection(
   return secondTraitCollection;
 }
 
-// Check that the button displayed are the ones which should be displayed in the
-// environment described by |traitCollection| and with |omniboxFocused|.
-void CheckToolbarButtonVisibility(UITraitCollection* traitCollection,
-                                  BOOL omniboxFocused) {
-  if (traitCollection.horizontalSizeClass == UIUserInterfaceSizeClassCompact &&
-      traitCollection.verticalSizeClass != UIUserInterfaceSizeClassCompact) {
-    // Split toolbar.
-    if (omniboxFocused) {
-      // Check that the omnibox and the cancel button are shown.
-      [[EarlGrey
-          selectElementWithMatcher:
-              grey_allOf(
-                  chrome_test_util::ButtonWithAccessibilityLabelId(IDS_CANCEL),
-                  VisibleInPrimaryToolbar(), nil)]
-          assertWithMatcher:grey_notNil()];
-      [[EarlGrey selectElementWithMatcher:chrome_test_util::Omnibox()]
-          assertWithMatcher:VisibleInPrimaryToolbar()];
+// Checks that the element associated with |matcher| is visible in the toolbar
+// defined by |visibility|.
+void CheckVisibilityInToolbar(id<GREYMatcher> matcher,
+                              ButtonVisibility visibility) {
+  CheckVisibleInPrimaryToolbar(matcher, visibility == ButtonVisibilityPrimary);
+  CheckVisibleInSecondaryToolbar(matcher,
+                                 visibility == ButtonVisibilitySecondary);
+}
 
+// Checks the visibility of the different part of the omnibox, depending on it
+// being focused or not.
+void CheckOmniboxVisibility(BOOL omniboxFocused) {
+  // Check omnibox/steady view visibility.
+  if (omniboxFocused) {
+    // Check that the omnibox is visible.
+    CheckVisibleInPrimaryToolbar(chrome_test_util::Omnibox(), YES);
   } else {
-      // Test the visibility of the primary toolbar buttons.
+    // Check that location view is visible.
     if (IsRefreshLocationBarEnabled()) {
-        [[EarlGrey
-            selectElementWithMatcher:chrome_test_util::DefocusedLocationView()]
-            assertWithMatcher:VisibleInPrimaryToolbar()];
+      CheckVisibleInPrimaryToolbar(chrome_test_util::DefocusedLocationView(),
+                                   YES);
     } else {
-        [[EarlGrey selectElementWithMatcher:chrome_test_util::Omnibox()]
-            assertWithMatcher:VisibleInPrimaryToolbar()];
+      CheckVisibleInPrimaryToolbar(chrome_test_util::Omnibox(), YES);
     }
-
-      // Test the visibility of the secondary toolbar buttons.
-      [[EarlGrey selectElementWithMatcher:chrome_test_util::BackButton()]
-          assertWithMatcher:VisibleInSecondaryToolbar()];
-      [[EarlGrey selectElementWithMatcher:chrome_test_util::ForwardButton()]
-          assertWithMatcher:VisibleInSecondaryToolbar()];
-      [[EarlGrey selectElementWithMatcher:grey_accessibilityID(
-                                              kToolbarOmniboxButtonIdentifier)]
-          assertWithMatcher:VisibleInSecondaryToolbar()];
-      [[EarlGrey selectElementWithMatcher:chrome_test_util::
-                                              ButtonWithAccessibilityLabelId(
-                                                  IDS_IOS_TOOLBAR_SHOW_TABS)]
-          assertWithMatcher:VisibleInSecondaryToolbar()];
-      [[EarlGrey selectElementWithMatcher:chrome_test_util::
-                                              ButtonWithAccessibilityLabelId(
-                                                  IDS_IOS_TOOLBAR_SETTINGS)]
-          assertWithMatcher:VisibleInSecondaryToolbar()];
   }
+}
 
-  } else {
-    // Unsplit toolbar.
+// Check the visibility of the buttons if the device is an iPhone in portrait or
+// an iPad in multitasking.
+void CheckButtonsVisibilityIPhonePortrait(BOOL omniboxFocused) {
+  // Check that the cancel button visibility.
   if (omniboxFocused) {
-      // Check that the omnibox is visible.
-      [[EarlGrey selectElementWithMatcher:chrome_test_util::Omnibox()]
-          assertWithMatcher:VisibleInPrimaryToolbar()];
-
-    } else {
-      // Check that location view is visible.
-      if (IsRefreshLocationBarEnabled()) {
-        [[EarlGrey
-            selectElementWithMatcher:chrome_test_util::DefocusedLocationView()]
-            assertWithMatcher:VisibleInPrimaryToolbar()];
+    CheckVisibilityInToolbar(CancelButton(), ButtonVisibilityPrimary);
+
+    CheckVisibilityInToolbar(ShareButton(), ButtonVisibilityNone);
+    CheckVisibilityInToolbar(ReloadButton(), ButtonVisibilityNone);
+    CheckVisibilityInToolbar(BookmarkButton(), ButtonVisibilityNone);
+
+    // Those buttons are hidden by the keyboard.
+    CheckVisibilityInToolbar(BackButton(), ButtonVisibilityNone);
+    CheckVisibilityInToolbar(ForwardButton(), ButtonVisibilityNone);
+    CheckVisibilityInToolbar(SearchButton(), ButtonVisibilityNone);
+    CheckVisibilityInToolbar(TabGridButton(), ButtonVisibilityNone);
+    CheckVisibilityInToolbar(ToolsMenuButton(), ButtonVisibilityNone);
   } else {
-        [[EarlGrey selectElementWithMatcher:chrome_test_util::Omnibox()]
-            assertWithMatcher:VisibleInPrimaryToolbar()];
+    CheckVisibilityInToolbar(CancelButton(), ButtonVisibilityNone);
+
+    CheckVisibilityInToolbar(ShareButton(), ButtonVisibilityNone);
+    CheckVisibilityInToolbar(ReloadButton(), ButtonVisibilityNone);
+    CheckVisibilityInToolbar(BookmarkButton(), ButtonVisibilityNone);
+
+    CheckVisibilityInToolbar(BackButton(), ButtonVisibilitySecondary);
+    CheckVisibilityInToolbar(ForwardButton(), ButtonVisibilitySecondary);
+    CheckVisibilityInToolbar(SearchButton(), ButtonVisibilitySecondary);
+    CheckVisibilityInToolbar(TabGridButton(), ButtonVisibilitySecondary);
+    CheckVisibilityInToolbar(ToolsMenuButton(), ButtonVisibilitySecondary);
   }
+}
+
+// Check the visibility of the buttons if the device is an iPhone in landscape.
+void CheckButtonsVisibilityIPhoneLandscape(BOOL omniboxFocused) {
+  if (omniboxFocused) {
+    // Omnibox focused in iPhone landscape.
+    CheckVisibilityInToolbar(CancelButton(), ButtonVisibilityPrimary);
+
+    CheckVisibilityInToolbar(ShareButton(), ButtonVisibilityNone);
+    CheckVisibilityInToolbar(ReloadButton(), ButtonVisibilityNone);
+    CheckVisibilityInToolbar(BookmarkButton(), ButtonVisibilityNone);
+
+    CheckVisibilityInToolbar(BackButton(), ButtonVisibilityNone);
+    CheckVisibilityInToolbar(ForwardButton(), ButtonVisibilityNone);
+    CheckVisibilityInToolbar(SearchButton(), ButtonVisibilityNone);
+    CheckVisibilityInToolbar(TabGridButton(), ButtonVisibilityNone);
+    CheckVisibilityInToolbar(ToolsMenuButton(), ButtonVisibilityNone);
+  } else {
+    CheckVisibilityInToolbar(CancelButton(), ButtonVisibilityNone);
+
+    CheckVisibilityInToolbar(ShareButton(), ButtonVisibilityPrimary);
+    CheckVisibilityInToolbar(ReloadButton(), ButtonVisibilityPrimary);
+    CheckVisibilityInToolbar(BookmarkButton(), ButtonVisibilityNone);
+
+    CheckVisibilityInToolbar(BackButton(), ButtonVisibilityPrimary);
+    CheckVisibilityInToolbar(ForwardButton(), ButtonVisibilityPrimary);
+    CheckVisibilityInToolbar(SearchButton(), ButtonVisibilityNone);
+    CheckVisibilityInToolbar(TabGridButton(), ButtonVisibilityPrimary);
+    CheckVisibilityInToolbar(ToolsMenuButton(), ButtonVisibilityPrimary);
   }
-    // Check that the cancel button is hidden.
+  // The secondary toolbar is not visible.
   [[EarlGrey
-        selectElementWithMatcher:
-            grey_allOf(
-                chrome_test_util::ButtonWithAccessibilityLabelId(IDS_CANCEL),
-                VisibleInPrimaryToolbar(), nil)] assertWithMatcher:grey_nil()];
+      selectElementWithMatcher:grey_kindOfClass([SecondaryToolbarView class])]
+      assertWithMatcher:grey_not(grey_sufficientlyVisible())];
+}
 
-    // Test the visibility of the primary toolbar buttons.
-    [[EarlGrey selectElementWithMatcher:chrome_test_util::BackButton()]
-        assertWithMatcher:VisibleInPrimaryToolbar()];
-    [[EarlGrey selectElementWithMatcher:chrome_test_util::ForwardButton()]
-        assertWithMatcher:VisibleInPrimaryToolbar()];
-    [[EarlGrey selectElementWithMatcher:ShareButton()]
-        assertWithMatcher:VisibleInPrimaryToolbar()];
-    [[EarlGrey selectElementWithMatcher:chrome_test_util::
-                                            ButtonWithAccessibilityLabelId(
-                                                IDS_IOS_ACCNAME_RELOAD)]
-        assertWithMatcher:VisibleInPrimaryToolbar()];
-    [[EarlGrey selectElementWithMatcher:chrome_test_util::
-                                            ButtonWithAccessibilityLabelId(
-                                                IDS_IOS_TOOLBAR_SETTINGS)]
-        assertWithMatcher:VisibleInPrimaryToolbar()];
+// Check the visibility of the buttons if the device is an iPad not in
+// multitasking.
+void CheckButtonsVisibilityIPad() {
+  CheckVisibilityInToolbar(CancelButton(), ButtonVisibilityNone);
+
+  CheckVisibilityInToolbar(ShareButton(), ButtonVisibilityPrimary);
+  CheckVisibilityInToolbar(ReloadButton(), ButtonVisibilityPrimary);
+  CheckVisibilityInToolbar(BookmarkButton(), ButtonVisibilityPrimary);
+
+  CheckVisibilityInToolbar(BackButton(), ButtonVisibilityPrimary);
+  CheckVisibilityInToolbar(ForwardButton(), ButtonVisibilityPrimary);
+  CheckVisibilityInToolbar(SearchButton(), ButtonVisibilityNone);
+  CheckVisibilityInToolbar(TabGridButton(), ButtonVisibilityNone);
+  CheckVisibilityInToolbar(ToolsMenuButton(), ButtonVisibilityPrimary);
 
   // The secondary toolbar is not visible.
   [[EarlGrey
       selectElementWithMatcher:grey_kindOfClass([SecondaryToolbarView class])]
       assertWithMatcher:grey_not(grey_sufficientlyVisible())];
+}
+
+// Check that the button displayed are the ones which should be displayed in the
+// environment described by |traitCollection| and with |omniboxFocused|.
+void CheckToolbarButtonVisibility(UITraitCollection* traitCollection,
+                                  BOOL omniboxFocused) {
+  CheckOmniboxVisibility(omniboxFocused);
 
-    if (traitCollection.verticalSizeClass == UIUserInterfaceSizeClassCompact) {
-      // Unsplit in compact height, the stack view button is visible.
-      [[EarlGrey selectElementWithMatcher:chrome_test_util::
-                                              ButtonWithAccessibilityLabelId(
-                                                  IDS_IOS_TOOLBAR_SHOW_TABS)]
-          assertWithMatcher:VisibleInPrimaryToolbar()];
+  // Button checks.
+  if (traitCollection.horizontalSizeClass == UIUserInterfaceSizeClassCompact &&
+      traitCollection.verticalSizeClass != UIUserInterfaceSizeClassCompact) {
+    CheckButtonsVisibilityIPhonePortrait(omniboxFocused);
+  } else if (traitCollection.verticalSizeClass ==
+             UIUserInterfaceSizeClassCompact) {
+    CheckButtonsVisibilityIPhoneLandscape(omniboxFocused);
   } else {
-      // Unsplit in Regular x Regular, the bookmark button is visible, the stack
-      // view button is hidden.
-      [[EarlGrey
-          selectElementWithMatcher:
-              grey_allOf(chrome_test_util::ButtonWithAccessibilityLabelId(
-                             IDS_IOS_TOOLBAR_SHOW_TABS),
-                         VisibleInPrimaryToolbar(), nil)]
-          assertWithMatcher:grey_nil()];
-      [[EarlGrey selectElementWithMatcher:BookmarkButton()]
-          assertWithMatcher:VisibleInPrimaryToolbar()];
-    }
+    CheckButtonsVisibilityIPad();
   }
 }
 

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

@@ -119,8 +119,8 @@
 - (void)transitionToLocationBarFocusedState:(BOOL)focused {
   [self.orchestrator
       transitionToStateOmniboxFocused:focused
-                      toolbarExpanded:focused &&
-                                      IsSplitToolbarMode(self.viewController)
+                      toolbarExpanded:focused && !IsRegularXRegularSizeClass(
+                                                     self.viewController)
                              animated:YES];
 }
 
@@ -133,7 +133,8 @@
   [self.orchestrator
       transitionToStateOmniboxFocused:omniboxFocused
                       toolbarExpanded:omniboxFocused &&
-                                      IsSplitToolbarMode(self.viewController)
+                                      !IsRegularXRegularSizeClass(
+                                          self.viewController)
                              animated:NO];
 }