Add feature tracker to the popup menu

This CL adds the feature tracker to display the "new" badge for the
reading list in the tools menu.

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

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

@@ -13,8 +13,10 @@ source_set("popup_menu") {
   deps = [
     ":popup_menu_ui",
     "//base",
+    "//components/feature_engagement/public",
     "//ios/chrome/app/strings",
     "//ios/chrome/browser/browser_state",
+    "//ios/chrome/browser/feature_engagement",
     "//ios/chrome/browser/find_in_page",
     "//ios/chrome/browser/reading_list",
     "//ios/chrome/browser/ui",
@@ -98,6 +100,7 @@ source_set("unit_tests") {
     ":popup_menu_ui",
     "//base",
     "//base/test:test_support",
+    "//components/feature_engagement/test:test_support",
     "//components/reading_list/core",
     "//ios/chrome/browser/browser_state:test_support",
     "//ios/chrome/browser/ui/toolbar/test",
@@ -108,6 +111,7 @@ source_set("unit_tests") {
     "//ios/web",
     "//ios/web/public/test",
     "//ios/web/public/test/fakes",
+    "//testing/gmock",
     "//testing/gtest",
     "//third_party/ocmock",
   ]

ios/chrome/browser/ui/popup_menu/cells/popup_menu_tools_item.h

@@ -17,9 +17,10 @@
 @property(nonatomic, strong) UIImage* image;
 // Whether the cell associated with this item should be enabled.
 @property(nonatomic, assign) BOOL enabled;
-// Number to be displayed in the badge.
+// Number to be displayed in the badge. If 0, the badge is hidden.
 @property(nonatomic, assign) NSInteger badgeNumber;
-// Text to be displayed in the badge. Set to nil to hide the badge.
+// Text to be displayed in the badge. Set to nil to hide the badge. The text
+// badge is only displayed if the numbered badge is hidden.
 @property(nonatomic, copy) NSString* badgeText;
 
 @end

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

@@ -8,6 +8,7 @@
 #include "base/metrics/user_metrics.h"
 #include "base/metrics/user_metrics_action.h"
 #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/commands/browser_commands.h"
 #import "ios/chrome/browser/ui/commands/command_dispatcher.h"
@@ -140,6 +141,8 @@ PopupMenuCommandType CommandTypeFromPopupType(PopupMenuType type) {
            isIncognito:self.browserState->IsOffTheRecord()
       readingListModel:ReadingListModelFactory::GetForBrowserState(
                            self.browserState)];
+  self.mediator.engagementTracker =
+      feature_engagement::TrackerFactory::GetForBrowserState(self.browserState);
   self.mediator.webStateList = self.webStateList;
   self.mediator.popupMenu = tableViewController;
   self.mediator.dispatcher = static_cast<id<BrowserCommands>>(self.dispatcher);

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

@@ -7,6 +7,9 @@
 
 #import <UIKit/UIKit.h>
 
+namespace feature_engagement {
+class Tracker;
+}
 @protocol BrowserCommands;
 @class PopupMenuTableViewController;
 class ReadingListModel;
@@ -38,6 +41,10 @@ typedef NS_ENUM(NSInteger, PopupMenuType) {
 @property(nonatomic, strong) PopupMenuTableViewController* popupMenu;
 // Dispatcher.
 @property(nonatomic, weak) id<BrowserCommands> dispatcher;
+// Records events for the use of in-product help. The mediator does not take
+// ownership of tracker. Tracker must not be destroyed during lifetime of the
+// object.
+@property(nonatomic, assign) feature_engagement::Tracker* engagementTracker;
 
 // Disconnect the mediator.
 - (void)disconnect;

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

@@ -5,6 +5,8 @@
 #import "ios/chrome/browser/ui/popup_menu/popup_menu_mediator.h"
 
 #include "base/strings/sys_string_conversions.h"
+#include "components/feature_engagement/public/feature_constants.h"
+#include "components/feature_engagement/public/tracker.h"
 #import "ios/chrome/browser/find_in_page/find_tab_helper.h"
 #import "ios/chrome/browser/ui/activity_services/canonical_url_retriever.h"
 #import "ios/chrome/browser/ui/commands/browser_commands.h"
@@ -97,6 +99,7 @@ PopupMenuToolsItem* CreateTableViewItem(int titleID,
 @synthesize isIncognito = _isIncognito;
 @synthesize popupMenu = _popupMenu;
 @synthesize dispatcher = _dispatcher;
+@synthesize engagementTracker = _engagementTracker;
 @synthesize readingListMenuNotifier = _readingListMenuNotifier;
 @synthesize type = _type;
 @synthesize webState = _webState;
@@ -137,6 +140,16 @@ PopupMenuToolsItem* CreateTableViewItem(int titleID,
     _webStateObserver.reset();
     _webState = nullptr;
   }
+
+  if (_engagementTracker &&
+      _engagementTracker->ShouldTriggerHelpUI(
+          feature_engagement::kIPHBadgedReadingListFeature)) {
+    _engagementTracker->Dismissed(
+        feature_engagement::kIPHBadgedReadingListFeature);
+    _engagementTracker = nullptr;
+  }
+
+  _readingListMenuNotifier = nil;
 }
 
 #pragma mark - CRWWebStateObserver
@@ -247,6 +260,18 @@ PopupMenuToolsItem* CreateTableViewItem(int titleID,
   }
 }
 
+- (void)setEngagementTracker:(feature_engagement::Tracker*)engagementTracker {
+  _engagementTracker = engagementTracker;
+
+  if (self.popupMenu && self.readingList && engagementTracker &&
+      self.engagementTracker->ShouldTriggerHelpUI(
+          feature_engagement::kIPHBadgedReadingListFeature)) {
+    self.readingList.badgeText = l10n_util::GetNSStringWithFixup(
+        IDS_IOS_READING_LIST_CELL_NEW_FEATURE_BADGE);
+    [self.popupMenu reconfigureCellsForItems:@[ self.readingList ]];
+  }
+}
+
 - (NSArray<NSArray<TableViewItem<PopupMenuItem>*>*>*)items {
   if (!_items) {
     switch (self.type) {
@@ -515,6 +540,13 @@ PopupMenuToolsItem* CreateTableViewItem(int titleID,
       @"popup_menu_reading_list", kToolsMenuReadingListId);
   self.readingList.badgeNumber =
       [self.readingListMenuNotifier readingListUnreadCount];
+  if (self.engagementTracker &&
+      self.engagementTracker->ShouldTriggerHelpUI(
+          feature_engagement::kIPHBadgedReadingListFeature)) {
+    self.readingList.badgeText = l10n_util::GetNSStringWithFixup(
+        IDS_IOS_READING_LIST_CELL_NEW_FEATURE_BADGE);
+  }
+
   // TODO(crbug.com/828367): Once the "unseen items effect" is defined,
   // implement it.
 

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

@@ -4,6 +4,7 @@
 
 #import "ios/chrome/browser/ui/popup_menu/popup_menu_mediator.h"
 #include "base/time/default_clock.h"
+#include "components/feature_engagement/test/mock_tracker.h"
 #include "components/reading_list/core/reading_list_model_impl.h"
 #import "ios/chrome/browser/ui/popup_menu/popup_menu_table_view_controller.h"
 #import "ios/chrome/browser/ui/toolbar/test/toolbar_test_navigation_manager.h"
@@ -17,6 +18,7 @@
 #import "ios/web/public/test/fakes/test_web_state.h"
 #include "ios/web/public/test/test_web_thread_bundle.h"
 #import "ios/web/public/web_state/web_state_observer_bridge.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/platform_test.h"
 #import "third_party/ocmock/OCMock/OCMock.h"
 #include "third_party/ocmock/gtest_support.h"
@@ -45,7 +47,7 @@ class PopupMenuMediatorTest : public PlatformTest {
         [[PopupMenuMediator alloc] initWithType:PopupMenuTypeToolsMenu
                                     isIncognito:YES
                                readingListModel:reading_list_model_.get()];
-    mediator_non_incognito_ =
+    mediator_ =
         [[PopupMenuMediator alloc] initWithType:PopupMenuTypeToolsMenu
                                     isIncognito:NO
                                readingListModel:reading_list_model_.get()];
@@ -61,7 +63,7 @@ class PopupMenuMediatorTest : public PlatformTest {
   // observers when web_state_list_ gets dealloc.
   ~PopupMenuMediatorTest() override {
     [mediator_incognito_ disconnect];
-    [mediator_non_incognito_ disconnect];
+    [mediator_ disconnect];
   }
 
  protected:
@@ -96,7 +98,7 @@ class PopupMenuMediatorTest : public PlatformTest {
   void SetUpActiveWebState() { web_state_list_->ActivateWebStateAt(0); }
 
   PopupMenuMediator* mediator_incognito_;
-  PopupMenuMediator* mediator_non_incognito_;
+  PopupMenuMediator* mediator_;
   std::unique_ptr<ReadingListModelImpl> reading_list_model_;
   ToolbarTestWebState* web_state_;
   ToolbarTestNavigationManager* navigation_manager_;
@@ -106,3 +108,16 @@ class PopupMenuMediatorTest : public PlatformTest {
   // Mock refusing all calls except -setPopupMenuItems:.
   id popup_menu_strict_;
 };
+
+// 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) {
+  feature_engagement::test::MockTracker tracker;
+  EXPECT_CALL(tracker, ShouldTriggerHelpUI(testing::_))
+      .WillRepeatedly(testing::Return(true));
+  mediator_.popupMenu = popup_menu_;
+  mediator_.engagementTracker = &tracker;
+
+  EXPECT_CALL(tracker, Dismissed(testing::_));
+  [mediator_ disconnect];
+}