[iOS] Update IsReadingListUIRebootEnabled() to use kUIRefreshPhase1.

This enables the table-based Reading List UI to be released with the
rest of the Phase 1 UI Refresh features.

This CL also updates Reading List EGTests to use a11y identifiers for
toolbar buttons rather than labels.  In the UI Refresh, the toolbar
buttons are based on UIBarButtonItems, which are rendered using
multiple views with the same a11y label.  Since EGTests couldn't find
matchers for the label, IDs are used instead.

Bug: 864378
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I223469771314907b614120a6b98f9bb92ce40b25
Reviewed-on: https://chromium-review.googlesource.com/1139901
Commit-Queue: Kurt Horimoto <kkhorimoto@chromium.org>
Reviewed-by: Rohit Rao <rohitrao@chromium.org>
Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576156}

ios/chrome/browser/experimental_flags.mm

@@ -135,7 +135,7 @@ bool IsBookmarksUIRebootEnabled() {
 }
 
 bool IsReadingListUIRebootEnabled() {
-  return base::FeatureList::IsEnabled(kCollectionsUIReboot);
+  return base::FeatureList::IsEnabled(kUIRefreshPhase1);
 }
 
 bool IsCollectionsUIRebootEnabled() {

ios/chrome/browser/feature_engagement/BUILD.gn

@@ -39,12 +39,14 @@ source_set("eg_tests") {
     "//components/feature_engagement/public",
     "//components/feature_engagement/test:test_support",
     "//ios/chrome/app/strings",
+    "//ios/chrome/browser",
     "//ios/chrome/browser/browser_state",
     "//ios/chrome/browser/ui:ui_util",
     "//ios/chrome/browser/ui/popup_menu:constants",
     "//ios/chrome/browser/ui/tab_grid:egtest_support",
     "//ios/chrome/browser/ui/tab_switcher:egtest_support",
     "//ios/chrome/browser/ui/tab_switcher:modes",
+    "//ios/chrome/browser/ui/table_view",
     "//ios/chrome/browser/ui/tools_menu/public",
     "//ios/chrome/test/app:test_support",
     "//ios/chrome/test/earl_grey:test_support",

ios/chrome/browser/feature_engagement/feature_engagement_egtest.mm

@@ -12,11 +12,13 @@
 #include "components/feature_engagement/public/tracker.h"
 #include "components/feature_engagement/test/test_tracker.h"
 #include "ios/chrome/browser/browser_state/chrome_browser_state.h"
+#include "ios/chrome/browser/experimental_flags.h"
 #include "ios/chrome/browser/feature_engagement/tracker_factory.h"
 #import "ios/chrome/browser/ui/popup_menu/popup_menu_constants.h"
 #import "ios/chrome/browser/ui/tab_grid/tab_grid_egtest_util.h"
 #import "ios/chrome/browser/ui/tab_switcher/tab_switcher_egtest_util.h"
 #import "ios/chrome/browser/ui/tab_switcher/tab_switcher_mode.h"
+#import "ios/chrome/browser/ui/table_view/table_view_navigation_controller_constants.h"
 #include "ios/chrome/browser/ui/tools_menu/public/tools_menu_constants.h"
 #include "ios/chrome/browser/ui/ui_util.h"
 #include "ios/chrome/grit/ios_strings.h"
@@ -248,8 +250,15 @@ void EnableNewTabTipTriggering(base::test::ScopedFeatureList& feature_list) {
   }
 
   [chrome_test_util::BrowserCommandDispatcherForMainBVC() showReadingList];
-  [[EarlGrey selectElementWithMatcher:grey_accessibilityLabel(@"Done")]
-      performAction:grey_tap()];
+  if (experimental_flags::IsReadingListUIRebootEnabled()) {
+    [[EarlGrey
+        selectElementWithMatcher:grey_accessibilityID(
+                                     kTableViewNavigationDismissButtonId)]
+        performAction:grey_tap()];
+  } else {
+    [[EarlGrey selectElementWithMatcher:grey_accessibilityLabel(@"Done")]
+        performAction:grey_tap()];
+  }
 
   [ChromeEarlGreyUI openToolsMenu];
 

ios/chrome/browser/ui/reading_list/BUILD.gn

@@ -100,6 +100,8 @@ source_set("reading_list_ui") {
     "reading_list_table_view_controller.h",
     "reading_list_table_view_controller.mm",
     "reading_list_toolbar_button_commands.h",
+    "reading_list_toolbar_button_identifiers.h",
+    "reading_list_toolbar_button_identifiers.mm",
     "reading_list_toolbar_button_manager.h",
     "reading_list_toolbar_button_manager.mm",
     "reading_list_ui_distillation_status.h",
@@ -205,6 +207,7 @@ source_set("eg_tests") {
     "//ios/chrome/browser/ui/popup_menu:constants",
     "//ios/chrome/browser/ui/table_view",
     "//ios/chrome/browser/ui/table_view:views",
+    "//ios/chrome/browser/ui/table_view/cells",
     "//ios/chrome/browser/ui/tools_menu/public",
     "//ios/chrome/test/app:test_support",
     "//ios/chrome/test/earl_grey:test_support",

ios/chrome/browser/ui/reading_list/legacy_reading_list_toolbar.mm

@@ -6,6 +6,7 @@
 
 #import "ios/chrome/browser/ui/alert_coordinator/action_sheet_coordinator.h"
 #import "ios/chrome/browser/ui/reading_list/legacy_reading_list_toolbar_button.h"
+#import "ios/chrome/browser/ui/reading_list/reading_list_toolbar_button_identifiers.h"
 #import "ios/chrome/common/ui_util/constraints_ui_util.h"
 #include "ios/chrome/grit/ios_strings.h"
 #import "ios/third_party/material_components_ios/src/components/Typography/src/MaterialTypography.h"
@@ -85,28 +86,34 @@ const CGFloat kHorizontalSpacing = 8.0f;
         initWithText:l10n_util::GetNSString(IDS_IOS_READING_LIST_DELETE_BUTTON)
          destructive:YES
             position:Leading];
+    _deleteButton.accessibilityIdentifier = kReadingListToolbarDeleteButtonID;
 
     _deleteAllButton = [[LegacyReadingListToolbarButton alloc]
         initWithText:l10n_util::GetNSString(
                          IDS_IOS_READING_LIST_DELETE_ALL_READ_BUTTON)
          destructive:YES
             position:Leading];
+    _deleteAllButton.accessibilityIdentifier =
+        kReadingListToolbarDeleteAllReadButtonID;
 
     _markButton = [[LegacyReadingListToolbarButton alloc]
         initWithText:l10n_util::GetNSString(
                          IDS_IOS_READING_LIST_MARK_ALL_BUTTON)
          destructive:NO
             position:Centered];
+    _markButton.accessibilityIdentifier = kReadingListToolbarMarkButtonID;
 
     _cancelButton = [[LegacyReadingListToolbarButton alloc]
         initWithText:l10n_util::GetNSString(IDS_IOS_READING_LIST_CANCEL_BUTTON)
          destructive:NO
             position:Trailing];
+    _cancelButton.accessibilityIdentifier = kReadingListToolbarCancelButtonID;
 
     _editButton = [[LegacyReadingListToolbarButton alloc]
         initWithText:l10n_util::GetNSString(IDS_IOS_READING_LIST_EDIT_BUTTON)
          destructive:NO
             position:Trailing];
+    _editButton.accessibilityIdentifier = kReadingListToolbarEditButtonID;
 
     [_editButton addTarget:nil
                     action:@selector(enterEditingModePressed)

ios/chrome/browser/ui/reading_list/reading_list_coordinator.mm

@@ -8,9 +8,12 @@
 #include "base/metrics/user_metrics.h"
 #include "base/metrics/user_metrics_action.h"
 #include "base/strings/sys_string_conversions.h"
+#include "components/feature_engagement/public/event_constants.h"
+#include "components/feature_engagement/public/tracker.h"
 #include "components/reading_list/core/reading_list_entry.h"
 #include "ios/chrome/browser/chrome_url_constants.h"
 #include "ios/chrome/browser/favicon/ios_chrome_large_icon_service_factory.h"
+#include "ios/chrome/browser/feature_engagement/tracker_factory.h"
 #import "ios/chrome/browser/metrics/new_tab_page_uma.h"
 #include "ios/chrome/browser/reading_list/offline_url_utils.h"
 #include "ios/chrome/browser/reading_list/reading_list_model_factory.h"
@@ -135,6 +138,11 @@
                                         animated:YES
                                       completion:nil];
 
+  // Send the "Viewed Reading List" event to the feature_engagement::Tracker
+  // when the user opens their reading list.
+  feature_engagement::TrackerFactory::GetForBrowserState(self.browserState)
+      ->NotifyEvent(feature_engagement::events::kViewedReadingList);
+
   [super start];
   self.started = YES;
 }

ios/chrome/browser/ui/reading_list/reading_list_table_view_controller.mm

@@ -649,6 +649,7 @@ ReadingListSelectionState GetSelectionStateForSelectedCounts(
     [self batchEditDidFinish];
   };
   [self performBatchTableViewUpdates:updates completion:completion];
+  [self removeEmptySections];
 }
 
 // Moves the ListItem within self.tableViewModel at |modelIndex| and the

ios/chrome/browser/ui/reading_list/reading_list_table_view_item.mm

@@ -11,6 +11,7 @@
 #include "base/time/time.h"
 #import "ios/chrome/browser/ui/reading_list/reading_list_list_item_custom_action_factory.h"
 #import "ios/chrome/browser/ui/reading_list/reading_list_list_item_util.h"
+#import "ios/chrome/browser/ui/table_view/cells/table_view_url_cell_favicon_badge_view.h"
 #import "ios/chrome/browser/ui/table_view/cells/table_view_url_item.h"
 #import "ios/chrome/browser/ui/table_view/chrome_table_view_styler.h"
 #include "ios/chrome/browser/ui/ui_util.h"

ios/chrome/browser/ui/reading_list/reading_list_toolbar_button_identifiers.h

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef IOS_CHROME_BROWSER_UI_READING_LIST_READING_LIST_TOOLBAR_BUTTON_IDENTIFIERS_H_
+#define IOS_CHROME_BROWSER_UI_READING_LIST_READING_LIST_TOOLBAR_BUTTON_IDENTIFIERS_H_
+
+#import <Foundation/Foundation.h>
+
+// Accessibility identifiers for reading list toolbar buttons.
+extern NSString* const kReadingListToolbarEditButtonID;
+extern NSString* const kReadingListToolbarDeleteButtonID;
+extern NSString* const kReadingListToolbarDeleteAllReadButtonID;
+extern NSString* const kReadingListToolbarCancelButtonID;
+extern NSString* const kReadingListToolbarMarkButtonID;
+
+#endif  // IOS_CHROME_BROWSER_UI_READING_LIST_READING_LIST_TOOLBAR_BUTTON_IDENTIFIERS_H_

ios/chrome/browser/ui/reading_list/reading_list_toolbar_button_identifiers.mm

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#import "ios/chrome/browser/ui/reading_list/reading_list_toolbar_button_identifiers.h"
+
+#if !defined(__has_feature) || !__has_feature(objc_arc)
+#error "This file requires ARC support."
+#endif
+
+NSString* const kReadingListToolbarEditButtonID =
+    @"ReadingListToolbarEditButtonID";
+NSString* const kReadingListToolbarDeleteButtonID =
+    @"ReadingListToolbarDeleteButtonID";
+NSString* const kReadingListToolbarDeleteAllReadButtonID =
+    @"ReadingListToolbarDeleteAllReadButton";
+NSString* const kReadingListToolbarCancelButtonID =
+    @"ReadingListToolbarCancelButton";
+NSString* const kReadingListToolbarMarkButtonID =
+    @"ReadingListToolbarMarkButton";
+;

ios/chrome/browser/ui/reading_list/reading_list_toolbar_button_manager.mm

@@ -7,6 +7,7 @@
 #include "base/logging.h"
 #import "ios/chrome/browser/ui/alert_coordinator/action_sheet_coordinator.h"
 #import "ios/chrome/browser/ui/reading_list/reading_list_toolbar_button_commands.h"
+#import "ios/chrome/browser/ui/reading_list/reading_list_toolbar_button_identifiers.h"
 #include "ios/chrome/grit/ios_strings.h"
 #include "ui/base/l10n/l10n_util_mac.h"
 
@@ -71,12 +72,14 @@ NSString* GetMarkButtonTitleForSelectionState(ReadingListSelectionState state) {
                 style:UIBarButtonItemStylePlain
                target:nil
                action:@selector(enterReadingListEditMode)];
+    _editButton.accessibilityIdentifier = kReadingListToolbarEditButtonID;
 
     _deleteButton = [[UIBarButtonItem alloc]
         initWithTitle:l10n_util::GetNSString(IDS_IOS_READING_LIST_DELETE_BUTTON)
                 style:UIBarButtonItemStylePlain
                target:nil
                action:@selector(deleteSelectedReadingListItems)];
+    _deleteButton.accessibilityIdentifier = kReadingListToolbarDeleteButtonID;
     _deleteButton.tintColor = [UIColor redColor];
 
     _deleteAllReadButton = [[UIBarButtonItem alloc]
@@ -85,6 +88,8 @@ NSString* GetMarkButtonTitleForSelectionState(ReadingListSelectionState state) {
                 style:UIBarButtonItemStylePlain
                target:nil
                action:@selector(deleteAllReadReadingListItems)];
+    _deleteAllReadButton.accessibilityIdentifier =
+        kReadingListToolbarDeleteAllReadButtonID;
     _deleteAllReadButton.tintColor = [UIColor redColor];
 
     _cancelButton = [[UIBarButtonItem alloc]
@@ -92,12 +97,14 @@ NSString* GetMarkButtonTitleForSelectionState(ReadingListSelectionState state) {
                 style:UIBarButtonItemStyleDone
                target:nil
                action:@selector(exitReadingListEditMode)];
+    _cancelButton.accessibilityIdentifier = kReadingListToolbarCancelButtonID;
 
     _markButton = [[UIBarButtonItem alloc]
         initWithTitle:GetMarkButtonTitleForSelectionState(self.selectionState)
                 style:UIBarButtonItemStylePlain
                target:self
                action:@selector(markButtonWasTapped)];
+    _markButton.accessibilityIdentifier = kReadingListToolbarMarkButtonID;
   }
   return self;
 }

ios/chrome/browser/ui/table_view/cells/BUILD.gn

@@ -26,6 +26,8 @@ source_set("cells") {
     "table_view_text_item.mm",
     "table_view_text_link_item.h",
     "table_view_text_link_item.mm",
+    "table_view_url_cell_favicon_badge_view.h",
+    "table_view_url_cell_favicon_badge_view.mm",
     "table_view_url_item.h",
     "table_view_url_item.mm",
   ]

ios/chrome/browser/ui/table_view/cells/table_view_url_cell_favicon_badge_view.h

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef IOS_CHROME_BROWSER_UI_TABLE_VIEW_CELLS_TABLE_VIEW_URL_CELL_FAVICON_BADGE_VIEW_H_
+#define IOS_CHROME_BROWSER_UI_TABLE_VIEW_CELLS_TABLE_VIEW_URL_CELL_FAVICON_BADGE_VIEW_H_
+
+#import <UIKit/UIKit.h>
+
+// View used to display the favicon badge image.  This class automatically
+// updates |hidden| to YES when its |image| is set to nil, rather than the
+// default UIImageView behavior which applies a default highlight to the view
+// for nil images.
+@interface TableViewURLCellFaviconBadgeView : UIImageView
+
+// The accessibility identifier of the badge view.
++ (NSString*)accessibilityIdentifier;
+
+@end
+
+#endif  // IOS_CHROME_BROWSER_UI_TABLE_VIEW_CELLS_TABLE_VIEW_URL_CELL_FAVICON_BADGE_VIEW_H_

ios/chrome/browser/ui/table_view/cells/table_view_url_cell_favicon_badge_view.mm

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#import "ios/chrome/browser/ui/table_view/cells/table_view_url_cell_favicon_badge_view.h"
+
+#if !defined(__has_feature) || !__has_feature(objc_arc)
+#error "This file requires ARC support."
+#endif
+
+@implementation TableViewURLCellFaviconBadgeView
+
+- (instancetype)init {
+  if (self = [super init]) {
+    self.hidden = YES;
+    self.accessibilityIdentifier = [[self class] accessibilityIdentifier];
+  }
+  return self;
+}
+
+#pragma mark - Public
+
++ (NSString*)accessibilityIdentifier {
+  return @"TableViewURLCellFaviconBadgeView";
+}
+
+#pragma mark - UIImageView
+
+- (void)setImage:(UIImage*)image {
+  [super setImage:image];
+  self.hidden = !image;
+}
+
+@end

ios/chrome/browser/ui/table_view/cells/table_view_url_item.h

@@ -11,6 +11,7 @@
 
 class GURL;
 @class FaviconViewNew;
+@class TableViewURLCellFaviconBadgeView;
 
 // TableViewURLItem contains the model data for a TableViewURLCell.
 @interface TableViewURLItem : TableViewItem
@@ -45,7 +46,8 @@ class GURL;
 @property(nonatomic, readonly, strong) UIImageView* faviconContainerView;
 
 // The image view used to display the favicon badge.
-@property(nonatomic, readonly, strong) UIImageView* faviconBadgeView;
+@property(nonatomic, readonly, strong)
+    TableViewURLCellFaviconBadgeView* faviconBadgeView;
 
 // The cell title.
 @property(nonatomic, readonly, strong) UILabel* titleLabel;

ios/chrome/browser/ui/table_view/cells/table_view_url_item.mm

@@ -7,6 +7,7 @@
 #include "base/mac/foundation_util.h"
 #include "base/strings/sys_string_conversions.h"
 #import "ios/chrome/browser/ui/table_view/cells/table_view_cells_constants.h"
+#import "ios/chrome/browser/ui/table_view/cells/table_view_url_cell_favicon_badge_view.h"
 #import "ios/chrome/browser/ui/table_view/chrome_table_view_styler.h"
 #include "ios/chrome/browser/ui/ui_util.h"
 #import "ios/chrome/browser/ui/uikit_ui_util.h"
@@ -32,27 +33,6 @@ const char kDefaultSupplementalURLTextDelimiter[] = "•";
 
 #pragma mark - TableViewURLCellFaviconBadgeView
 
-// View used to display the favicon badge image.  This class automatically
-// updates |hidden| to YES when its |image| is set to nil, rather than the
-// default UIImageView behavior which applies a default highlight to the view
-// for nil images.
-@interface TableViewURLCellFaviconBadgeView : UIImageView
-@end
-
-@implementation TableViewURLCellFaviconBadgeView
-
-- (instancetype)init {
-  if (self = [super init])
-    self.hidden = YES;
-  return self;
-}
-
-- (void)setImage:(UIImage*)image {
-  [super setImage:image];
-  self.hidden = !image;
-}
-
-@end
 
 #pragma mark - TableViewURLItem
 

ios/chrome/browser/ui/table_view/table_view_empty_view.mm

@@ -47,6 +47,7 @@ NSAttributedString* GetAttributedMessage(NSString* message) {
   if (self = [super initWithFrame:frame]) {
     _message = GetAttributedMessage(message);
     _image = image;
+    self.accessibilityIdentifier = [[self class] accessibilityIdentifier];
   }
   return self;
 }
@@ -57,6 +58,7 @@ NSAttributedString* GetAttributedMessage(NSString* message) {
   if (self = [super initWithFrame:frame]) {
     _message = message;
     _image = image;
+    self.accessibilityIdentifier = [[self class] accessibilityIdentifier];
   }
   return self;
 }