Re-reland "Add blink effect to the PopupMenu"

This CL adds an effect to make the "New Incognito Tab" row of the tools
menu blink a selected state when the in product help requires it.

TBR=edchin@chromium.org

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

ios/chrome/browser/ui/browser_view_controller.mm

@@ -2300,6 +2300,8 @@ applicationCommandEndpoint:(id<ApplicationCommands>)applicationCommandEndpoint {
     self.popupMenuCoordinator = [[PopupMenuCoordinator alloc]
         initWithBaseViewController:self
                       browserState:self.browserState];
+    self.popupMenuCoordinator.incognitoTabTipPresenter =
+        self.incognitoTabTipBubblePresenter;
     self.popupMenuCoordinator.dispatcher = _dispatcher;
     self.popupMenuCoordinator.webStateList = [_model webStateList];
     self.popupMenuCoordinator.UIUpdater = _toolbarCoordinatorAdaptor;
@@ -2826,6 +2828,7 @@ bubblePresenterForFeature:(const base::Feature&)feature
     return;
 
   self.incognitoTabTipBubblePresenter = presenter;
+  self.popupMenuCoordinator.incognitoTabTipPresenter = presenter;
 
   [self.incognitoTabTipBubblePresenter
       presentInViewController:self

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

@@ -25,6 +25,7 @@ source_set("popup_menu") {
     "//ios/chrome/browser/ui",
     "//ios/chrome/browser/ui/activity_services",
     "//ios/chrome/browser/ui/bookmarks",
+    "//ios/chrome/browser/ui/bubble",
     "//ios/chrome/browser/ui/commands",
     "//ios/chrome/browser/ui/coordinators:chrome_coordinators",
     "//ios/chrome/browser/ui/popup_menu/cells",
@@ -127,7 +128,9 @@ source_set("unit_tests") {
     "//ios/chrome/browser/web_state_list",
     "//ios/chrome/browser/web_state_list:test_support",
     "//ios/chrome/test:test_support",
+    "//ios/public/provider/chrome/browser",
     "//ios/public/provider/chrome/browser:test_support",
+    "//ios/public/provider/chrome/browser/user_feedback",
     "//ios/web",
     "//ios/web/public/test",
     "//ios/web/public/test/fakes",

ios/chrome/browser/ui/popup_menu/popup_menu_constants.h

@@ -13,5 +13,7 @@ extern NSString* const kPopupMenuNavigationTableViewId;
 
 // Alpha for the background color of the highlighted items.
 extern const CGFloat kSelectedItemBackgroundAlpha;
+// Duration of the highlight animation of the popup menu.
+extern const CGFloat kHighlightAnimationDuration;
 
 #endif  // IOS_CHROME_BROWSER_UI_POPUP_MENU_POPUP_MENU_CONSTANTS_H_

ios/chrome/browser/ui/popup_menu/popup_menu_constants.mm

@@ -14,3 +14,4 @@ NSString* const kPopupMenuNavigationTableViewId =
     @"kPopupMenuNavigationTableViewId";
 
 const CGFloat kSelectedItemBackgroundAlpha = 0.05;
+const CGFloat kHighlightAnimationDuration = 0.5;

ios/chrome/browser/ui/popup_menu/popup_menu_coordinator.h

@@ -9,6 +9,7 @@
 
 #import "ios/chrome/browser/ui/coordinators/chrome_coordinator.h"
 
+@class BubbleViewControllerPresenter;
 @class CommandDispatcher;
 @protocol PopupMenuUIUpdating;
 class WebStateList;
@@ -22,6 +23,9 @@ class WebStateList;
 @property(nonatomic, assign) WebStateList* webStateList;
 // UI updater.
 @property(nonatomic, weak) id<PopupMenuUIUpdating> UIUpdater;
+// Bubble view presenter for the incognito tip.
+@property(nonatomic, weak)
+    BubbleViewControllerPresenter* incognitoTabTipPresenter;
 
 // Returns whether this coordinator is showing a popup menu.
 - (BOOL)isShowingPopupMenu;

ios/chrome/browser/ui/popup_menu/popup_menu_coordinator.mm

@@ -12,6 +12,7 @@
 #include "ios/chrome/browser/browser_state/chrome_browser_state.h"
 #include "ios/chrome/browser/feature_engagement/tracker_factory.h"
 #include "ios/chrome/browser/reading_list/reading_list_model_factory.h"
+#import "ios/chrome/browser/ui/bubble/bubble_view_controller_presenter.h"
 #import "ios/chrome/browser/ui/commands/browser_commands.h"
 #import "ios/chrome/browser/ui/commands/command_dispatcher.h"
 #import "ios/chrome/browser/ui/commands/popup_menu_commands.h"
@@ -54,6 +55,7 @@ PopupMenuCommandType CommandTypeFromPopupType(PopupMenuType type) {
 @synthesize requestStartTime = _requestStartTime;
 @synthesize UIUpdater = _UIUpdater;
 @synthesize webStateList = _webStateList;
+@synthesize incognitoTabTipPresenter = _incognitoTabTipPresenter;
 
 #pragma mark - ChromeCoordinator
 
@@ -166,11 +168,19 @@ PopupMenuCommandType CommandTypeFromPopupType(PopupMenuType type) {
       static_cast<id<ApplicationCommands, BrowserCommands>>(self.dispatcher);
   tableViewController.baseViewController = self.baseViewController;
 
+  BOOL triggerNewIncognitoTabTip = NO;
+  if (type == PopupMenuTypeToolsMenu) {
+    triggerNewIncognitoTabTip =
+        self.incognitoTabTipPresenter.triggerFollowUpAction;
+    self.incognitoTabTipPresenter.triggerFollowUpAction = NO;
+  }
+
   self.mediator = [[PopupMenuMediator alloc]
-          initWithType:type
-           isIncognito:self.browserState->IsOffTheRecord()
-      readingListModel:ReadingListModelFactory::GetForBrowserState(
-                           self.browserState)];
+                   initWithType:type
+                    isIncognito:self.browserState->IsOffTheRecord()
+               readingListModel:ReadingListModelFactory::GetForBrowserState(
+                                    self.browserState)
+      triggerNewIncognitoTabTip:triggerNewIncognitoTabTip];
   self.mediator.engagementTracker =
       feature_engagement::TrackerFactory::GetForBrowserState(self.browserState);
   self.mediator.webStateList = self.webStateList;

ios/chrome/browser/ui/popup_menu/popup_menu_mediator.h

@@ -24,9 +24,13 @@ class WebStateList;
 // updating the items of the popup menu.
 @interface PopupMenuMediator : NSObject
 
+// Initializes the mediator with a |type| of popup menu, whether it
+// |isIncognito|, a |readingListModel| used to display the badge for the reading
+// list entry, and whether the mediator should |triggerNewIncognitoTabTip|.
 - (instancetype)initWithType:(PopupMenuType)type
-                 isIncognito:(BOOL)isIncognito
-            readingListModel:(ReadingListModel*)readingListModel
+                  isIncognito:(BOOL)isIncognito
+             readingListModel:(ReadingListModel*)readingListModel
+    triggerNewIncognitoTabTip:(BOOL)triggerNewIncognitoTabTip
     NS_DESIGNATED_INITIALIZER;
 - (instancetype)init NS_UNAVAILABLE;
 

ios/chrome/browser/ui/popup_menu/popup_menu_mediator.mm

@@ -89,8 +89,12 @@ PopupMenuToolsItem* CreateTableViewItem(int titleID,
 // Items notifying this items of changes happening to the ReadingList model.
 @property(nonatomic, strong) ReadingListMenuNotifier* readingListMenuNotifier;
 
+// Whether the hint for the "New Incognito Tab" item should be triggered.
+@property(nonatomic, assign) BOOL triggerNewIncognitoTabTip;
+
 #pragma mark*** Specific Items ***
 
+@property(nonatomic, strong) PopupMenuToolsItem* openNewIncognitoTabItem;
 @property(nonatomic, strong) PopupMenuToolsItem* reloadStopItem;
 @property(nonatomic, strong) PopupMenuToolsItem* readLaterItem;
 @property(nonatomic, strong) PopupMenuToolsItem* bookmarkItem;
@@ -113,9 +117,11 @@ PopupMenuToolsItem* CreateTableViewItem(int titleID,
 @synthesize dispatcher = _dispatcher;
 @synthesize engagementTracker = _engagementTracker;
 @synthesize readingListMenuNotifier = _readingListMenuNotifier;
+@synthesize triggerNewIncognitoTabTip = _triggerNewIncognitoTabTip;
 @synthesize type = _type;
 @synthesize webState = _webState;
 @synthesize webStateList = _webStateList;
+@synthesize openNewIncognitoTabItem = _openNewIncognitoTabItem;
 @synthesize reloadStopItem = _reloadStopItem;
 @synthesize readLaterItem = _readLaterItem;
 @synthesize bookmarkItem = _bookmarkItem;
@@ -129,8 +135,9 @@ PopupMenuToolsItem* CreateTableViewItem(int titleID,
 #pragma mark - Public
 
 - (instancetype)initWithType:(PopupMenuType)type
-                 isIncognito:(BOOL)isIncognito
-            readingListModel:(ReadingListModel*)readingListModel {
+                  isIncognito:(BOOL)isIncognito
+             readingListModel:(ReadingListModel*)readingListModel
+    triggerNewIncognitoTabTip:(BOOL)triggerNewIncognitoTabTip {
   self = [super init];
   if (self) {
     _isIncognito = isIncognito;
@@ -139,6 +146,7 @@ PopupMenuToolsItem* CreateTableViewItem(int titleID,
         [[ReadingListMenuNotifier alloc] initWithReadingList:readingListModel];
     _webStateObserver = std::make_unique<web::WebStateObserverBridge>(self);
     _webStateListObserver = std::make_unique<WebStateListObserverBridge>(self);
+    _triggerNewIncognitoTabTip = triggerNewIncognitoTabTip;
   }
   return self;
 }
@@ -308,6 +316,10 @@ PopupMenuToolsItem* CreateTableViewItem(int titleID,
   }
 
   [_popupMenu setPopupMenuItems:self.items];
+  if (self.triggerNewIncognitoTabTip) {
+    _popupMenu.itemToHighlight = self.openNewIncognitoTabItem;
+    self.triggerNewIncognitoTabTip = NO;
+  }
   _popupMenu.commandHandler = self;
   if (self.webState) {
     [self updatePopupMenu];
@@ -591,11 +603,11 @@ PopupMenuToolsItem* CreateTableViewItem(int titleID,
                           @"popup_menu_new_tab", kToolsMenuNewTabId);
 
   // Open New Incognito Tab.
-  TableViewItem* openNewIncognitoTabItem = CreateTableViewItem(
+  self.openNewIncognitoTabItem = CreateTableViewItem(
       IDS_IOS_TOOLS_MENU_NEW_INCOGNITO_TAB, PopupMenuActionOpenNewIncognitoTab,
       @"popup_menu_new_incognito_tab", kToolsMenuNewIncognitoTabId);
 
-  return @[ openNewTabItem, openNewIncognitoTabItem ];
+  return @[ openNewTabItem, self.openNewIncognitoTabItem ];
 }
 
 - (NSArray<TableViewItem*>*)actionItems {

ios/chrome/browser/ui/popup_menu/popup_menu_mediator_unittest.mm

@@ -13,6 +13,8 @@
 #include "ios/chrome/browser/web_state_list/web_state_list.h"
 #import "ios/chrome/browser/web_state_list/web_state_list_observer_bridge.h"
 #import "ios/chrome/browser/web_state_list/web_state_opener.h"
+#include "ios/public/provider/chrome/browser/chrome_browser_provider.h"
+#import "ios/public/provider/chrome/browser/user_feedback/user_feedback_provider.h"
 #import "ios/web/public/test/fakes/fake_navigation_context.h"
 #import "ios/web/public/test/fakes/test_navigation_manager.h"
 #import "ios/web/public/test/fakes/test_web_state.h"
@@ -43,14 +45,6 @@ class PopupMenuMediatorTest : public PlatformTest {
   PopupMenuMediatorTest() {
     reading_list_model_.reset(new ReadingListModelImpl(
         nullptr, nullptr, base::DefaultClock::GetInstance()));
-    mediator_incognito_ =
-        [[PopupMenuMediator alloc] initWithType:PopupMenuTypeToolsMenu
-                                    isIncognito:YES
-                               readingListModel:reading_list_model_.get()];
-    mediator_ =
-        [[PopupMenuMediator alloc] initWithType:PopupMenuTypeToolsMenu
-                                    isIncognito:NO
-                               readingListModel:reading_list_model_.get()];
     popup_menu_ = OCMClassMock([PopupMenuTableViewController class]);
     popup_menu_strict_ =
         OCMStrictClassMock([PopupMenuTableViewController class]);
@@ -62,11 +56,21 @@ class PopupMenuMediatorTest : public PlatformTest {
   // Explicitly disconnect the mediator so there won't be any WebStateList
   // observers when web_state_list_ gets dealloc.
   ~PopupMenuMediatorTest() override {
-    [mediator_incognito_ disconnect];
     [mediator_ disconnect];
   }
 
  protected:
+  PopupMenuMediator* CreateMediator(PopupMenuType type,
+                                    BOOL is_incognito,
+                                    BOOL trigger_incognito_hint) {
+    mediator_ =
+        [[PopupMenuMediator alloc] initWithType:type
+                                    isIncognito:is_incognito
+                               readingListModel:reading_list_model_.get()
+                      triggerNewIncognitoTabTip:trigger_incognito_hint];
+    return mediator_;
+  }
+
   void SetUpWebStateList() {
     auto navigation_manager = std::make_unique<ToolbarTestNavigationManager>();
     navigation_manager_ = navigation_manager.get();
@@ -97,7 +101,29 @@ class PopupMenuMediatorTest : public PlatformTest {
 
   void SetUpActiveWebState() { web_state_list_->ActivateWebStateAt(0); }
 
-  PopupMenuMediator* mediator_incognito_;
+  // Checks that the popup_menu_ is receiving a number of items corresponding to
+  // |number_items|.
+  void CheckMediatorSetItems(NSArray<NSNumber*>* number_items) {
+    mediator_.webStateList = web_state_list_.get();
+    SetUpActiveWebState();
+    auto same_number_items = ^BOOL(id items) {
+      if (![items isKindOfClass:[NSArray class]])
+        return NO;
+      if ([items count] != number_items.count)
+        return NO;
+      for (NSUInteger index = 0; index < number_items.count; index++) {
+        NSArray* section = [items objectAtIndex:index];
+        if (section.count != number_items[index].unsignedIntegerValue)
+          return NO;
+      }
+      return YES;
+    };
+    OCMExpect([popup_menu_
+        setPopupMenuItems:[OCMArg checkWithBlock:same_number_items]]);
+    mediator_.popupMenu = popup_menu_;
+    EXPECT_OCMOCK_VERIFY(popup_menu_);
+  }
+
   PopupMenuMediator* mediator_;
   std::unique_ptr<ReadingListModelImpl> reading_list_model_;
   ToolbarTestWebState* web_state_;
@@ -112,6 +138,7 @@ class PopupMenuMediatorTest : public PlatformTest {
 // Tests that the feature engagement tracker get notified when the mediator is
 // disconnected and the tracker wants the notification badge displayed.
 TEST_F(PopupMenuMediatorTest, TestFeatureEngagementDisconnect) {
+  CreateMediator(PopupMenuTypeToolsMenu, NO, NO);
   feature_engagement::test::MockTracker tracker;
   EXPECT_CALL(tracker, ShouldTriggerHelpUI(testing::_))
       .WillRepeatedly(testing::Return(true));
@@ -121,3 +148,59 @@ TEST_F(PopupMenuMediatorTest, TestFeatureEngagementDisconnect) {
   EXPECT_CALL(tracker, Dismissed(testing::_));
   [mediator_ disconnect];
 }
+
+// Tests that the mediator is returning the right number of items and sections
+// for the Tools Menu type.
+TEST_F(PopupMenuMediatorTest, TestElementsToolsMenu) {
+  CreateMediator(PopupMenuTypeToolsMenu, NO, NO);
+  NSUInteger number_of_action_items = 6;
+  if (ios::GetChromeBrowserProvider()
+          ->GetUserFeedbackProvider()
+          ->IsUserFeedbackEnabled()) {
+    number_of_action_items++;
+  }
+  CheckMediatorSetItems(@[ @(3), @(number_of_action_items), @(5) ]);
+}
+
+// Tests that the mediator is returning the right number of items and sections
+// for the Tab Grid type, in non-incognito.
+TEST_F(PopupMenuMediatorTest, TestElementsTabGridNonIncognito) {
+  CreateMediator(PopupMenuTypeTabGrid, NO, NO);
+  CheckMediatorSetItems(@[ @(2), @(1) ]);
+}
+
+// Tests that the mediator is returning the right number of items and sections
+// for the Tab Grid type, in incognito.
+TEST_F(PopupMenuMediatorTest, TestElementsTabGridIncognito) {
+  CreateMediator(PopupMenuTypeTabGrid, YES, NO);
+  CheckMediatorSetItems(@[ @(2), @(2) ]);
+}
+
+// Tests that the mediator is asking for an item to be highlighted when asked.
+TEST_F(PopupMenuMediatorTest, TestNewIncognitoHint) {
+  CreateMediator(PopupMenuTypeToolsMenu, NO, YES);
+  mediator_.webStateList = web_state_list_.get();
+  SetUpActiveWebState();
+  OCMExpect([popup_menu_ setItemToHighlight:[OCMArg isNotNil]]);
+  mediator_.popupMenu = popup_menu_;
+  EXPECT_OCMOCK_VERIFY(popup_menu_);
+}
+
+// Test that the mediator isn't asking for an highlighted item.
+TEST_F(PopupMenuMediatorTest, TestNewIncognitoNoHint) {
+  CreateMediator(PopupMenuTypeToolsMenu, NO, NO);
+  [[popup_menu_ reject] setItemToHighlight:[OCMArg any]];
+  mediator_.webStateList = web_state_list_.get();
+  SetUpActiveWebState();
+  mediator_.popupMenu = popup_menu_;
+}
+
+// Tests that the mediator is asking for an item to be highlighted when asked.
+TEST_F(PopupMenuMediatorTest, TestNewIncognitoHintTabGrid) {
+  CreateMediator(PopupMenuTypeTabGrid, NO, YES);
+  OCMExpect([popup_menu_ setItemToHighlight:[OCMArg isNotNil]]);
+  mediator_.webStateList = web_state_list_.get();
+  SetUpActiveWebState();
+  mediator_.popupMenu = popup_menu_;
+  EXPECT_OCMOCK_VERIFY(popup_menu_);
+}

ios/chrome/browser/ui/popup_menu/popup_menu_table_view_controller.h

@@ -26,6 +26,9 @@
 // Presenting ViewController for the ViewController needing to be presented as
 // result of an interaction with the popup.
 @property(nonatomic, weak) UIViewController* baseViewController;
+// Item to be highlighted. Nil if no item should be highlighted. Must be set
+// after the popup menu items.
+@property(nonatomic, weak) TableViewItem<PopupMenuItem>* itemToHighlight;
 
 // Initializers.
 - (instancetype)init NS_DESIGNATED_INITIALIZER;

ios/chrome/browser/ui/popup_menu/popup_menu_table_view_controller.mm

@@ -11,6 +11,7 @@
 #import "ios/chrome/browser/ui/commands/open_new_tab_command.h"
 #import "ios/chrome/browser/ui/popup_menu/cells/popup_menu_footer_item.h"
 #import "ios/chrome/browser/ui/popup_menu/cells/popup_menu_item.h"
+#import "ios/chrome/browser/ui/popup_menu/popup_menu_constants.h"
 #import "ios/chrome/browser/ui/popup_menu/popup_menu_table_view_controller_commands.h"
 #import "ios/chrome/browser/ui/table_view/chrome_table_view_styler.h"
 
@@ -26,18 +27,35 @@ const CGFloat kPopupMenuVerticalInsets = 7;
 const CGFloat kScrollIndicatorVerticalInsets = 11;
 }  // namespace
 
+@interface PopupMenuTableViewController ()
+// Whether the -viewDidAppear: callback has been called.
+@property(nonatomic, assign) BOOL viewDidAppear;
+@end
+
 @implementation PopupMenuTableViewController
 
 @dynamic tableViewModel;
 @synthesize baseViewController = _baseViewController;
 @synthesize commandHandler = _commandHandler;
 @synthesize dispatcher = _dispatcher;
+@synthesize itemToHighlight = _itemToHighlight;
+@synthesize viewDidAppear = _viewDidAppear;
 
 - (instancetype)init {
   return [super initWithTableViewStyle:UITableViewStyleGrouped
                            appBarStyle:ChromeTableViewControllerStyleNoAppBar];
 }
 
+#pragma mark - Properties
+
+- (void)setItemToHighlight:(TableViewItem<PopupMenuItem>*)itemToHighlight {
+  DCHECK_GT(self.tableViewModel.numberOfSections, 0L);
+  _itemToHighlight = itemToHighlight;
+  if (itemToHighlight && self.viewDidAppear) {
+    [self highlightItem:itemToHighlight repeat:YES];
+  }
+}
+
 #pragma mark - UIViewController
 
 - (void)viewDidLoad {
@@ -57,6 +75,14 @@ const CGFloat kScrollIndicatorVerticalInsets = 11;
                                0.01f)];
 }
 
+- (void)viewDidAppear:(BOOL)animated {
+  [super viewDidAppear:animated];
+  self.viewDidAppear = YES;
+  if (self.itemToHighlight) {
+    [self highlightItem:self.itemToHighlight repeat:YES];
+  }
+}
+
 - (void)setPopupMenuItems:
     (NSArray<NSArray<TableViewItem<PopupMenuItem>*>*>*)items {
   [super loadModel];
@@ -126,6 +152,36 @@ const CGFloat kScrollIndicatorVerticalInsets = 11;
 
 #pragma mark - Private
 
+// Highlights the |item| and |repeat| the highlighting once.
+- (void)highlightItem:(TableViewItem<PopupMenuItem>*)item repeat:(BOOL)repeat {
+  NSIndexPath* indexPath = [self.tableViewModel indexPathForItem:item];
+  [self.tableView selectRowAtIndexPath:indexPath
+                              animated:YES
+                        scrollPosition:UITableViewScrollPositionNone];
+  dispatch_after(
+      dispatch_time(DISPATCH_TIME_NOW,
+                    (int64_t)(kHighlightAnimationDuration * NSEC_PER_SEC)),
+      dispatch_get_main_queue(), ^{
+        [self unhighlightItem:item repeat:repeat];
+      });
+}
+
+// Removes the highlight from |item| and |repeat| the highlighting once.
+- (void)unhighlightItem:(TableViewItem<PopupMenuItem>*)item
+                 repeat:(BOOL)repeat {
+  NSIndexPath* indexPath = [self.tableViewModel indexPathForItem:item];
+  [self.tableView deselectRowAtIndexPath:indexPath animated:YES];
+  if (!repeat)
+    return;
+
+  dispatch_after(
+      dispatch_time(DISPATCH_TIME_NOW,
+                    (int64_t)(kHighlightAnimationDuration * NSEC_PER_SEC)),
+      dispatch_get_main_queue(), ^{
+        [self highlightItem:item repeat:NO];
+      });
+}
+
 // Executes the action associated with |identifier|, using |origin| as the point
 // of origin of the action if one is needed.
 - (void)executeActionForItem:(TableViewItem<PopupMenuItem>*)item