[iOS] Refactor CBDCollectionView initialization

-Add another initializer to ClearBrowsingDataCollectionViewController,
so to allow dependency injection in tests.
-Make corrections on ClearBrowsingDataManager, to avoid a strong
reference cycle.
-Change tests to utilise more appropriate methods.

Bug: 935350
Change-Id: I2d4a1b3a740a75785499f3487ee94b842949ce0e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1579719Reviewed-by: Mark Cogan <marq@chromium.org>
Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Commit-Queue: Huanzhong Huang <huanzhong@chromium.org>
Cr-Commit-Position: refs/heads/master@{#653549}

ios/chrome/browser/ui/collection_view/collection_view_controller_test.h

@@ -71,6 +71,14 @@ class CollectionViewControllerTest : public BlockCleanupTest {
   // items in a dedicated section.
   void CheckSectionFooterWithId(int expected_text_id, int section);
 
+  // Verifies that the text cell at |item| in |section| has a text property
+  // which matches |expected_title|.
+  void CheckTextCellText(NSString* expected_text, int section, int item);
+
+  // Verifies that the text cell at |item| in |section| has a text property
+  // which matches the l10n string for |expected_title_id|.
+  void CheckTextCellTextWithId(int expected_text_id, int section, int item);
+
   // Verifies that the text cell at |item| in |section| has a title which
   // matches |expected_title|.
   void CheckTextCellTitle(NSString* expected_title, int section, int item);

ios/chrome/browser/ui/collection_view/collection_view_controller_test.mm

@@ -114,6 +114,20 @@ void CollectionViewControllerTest::CheckSectionFooterWithId(
   return CheckSectionFooter(l10n_util::GetNSString(expected_text_id), section);
 }
 
+void CollectionViewControllerTest::CheckTextCellText(NSString* expected_text,
+                                                     int section,
+                                                     int item) {
+  id cell = GetCollectionViewItem(section, item);
+  ASSERT_TRUE([cell respondsToSelector:@selector(text)]);
+  EXPECT_NSEQ(expected_text, [cell text]);
+}
+
+void CollectionViewControllerTest::CheckTextCellTextWithId(int expected_text_id,
+                                                           int section,
+                                                           int item) {
+  CheckTextCellText(l10n_util::GetNSString(expected_text_id), section, item);
+}
+
 void CollectionViewControllerTest::CheckTextCellTitle(NSString* expected_title,
                                                       int section,
                                                       int item) {

ios/chrome/browser/ui/settings/clear_browsing_data/clear_browsing_data_collection_view_controller.h

@@ -13,12 +13,19 @@ namespace ios {
 class ChromeBrowserState;
 }  // namespace ios
 
+@class ClearBrowsingDataManager;
+
 // CollectionView for clearing browsing data (including history,
 // cookies, caches, passwords, and autofill).
 @interface ClearBrowsingDataCollectionViewController
     : SettingsRootCollectionViewController <ClearBrowsingDataConsumer>
 
+// "Default" convenience initializer that Users should in general make use of.
+- (instancetype)initWithBrowserState:(ios::ChromeBrowserState*)browserState;
+
+// Designated initializer to allow dependency injection (in tests).
 - (instancetype)initWithBrowserState:(ios::ChromeBrowserState*)browserState
+                             manager:(ClearBrowsingDataManager*)manager
     NS_DESIGNATED_INITIALIZER;
 
 - (instancetype)initWithLayout:(UICollectionViewLayout*)layout

ios/chrome/browser/ui/settings/clear_browsing_data/clear_browsing_data_collection_view_controller.mm

@@ -72,6 +72,14 @@
 #pragma mark Initialization
 
 - (instancetype)initWithBrowserState:(ios::ChromeBrowserState*)browserState {
+  ClearBrowsingDataManager* manager = [[ClearBrowsingDataManager alloc]
+      initWithBrowserState:browserState
+                  listType:ClearBrowsingDataListType::kListTypeCollectionView];
+  return [self initWithBrowserState:browserState manager:manager];
+}
+
+- (instancetype)initWithBrowserState:(ios::ChromeBrowserState*)browserState
+                             manager:(ClearBrowsingDataManager*)manager {
   DCHECK(browserState);
   UICollectionViewLayout* layout = [[MDCCollectionViewFlowLayout alloc] init];
   self = [super initWithLayout:layout
@@ -80,10 +88,7 @@
     self.accessibilityTraits |= UIAccessibilityTraitButton;
 
     _browserState = browserState;
-    _dataManager = [[ClearBrowsingDataManager alloc]
-        initWithBrowserState:browserState
-                    listType:ClearBrowsingDataListType::
-                                 kListTypeCollectionView];
+    _dataManager = manager;
     _dataManager.linkDelegate = self;
     _dataManager.consumer = self;
 

ios/chrome/browser/ui/settings/clear_browsing_data/clear_browsing_data_collection_view_controller_unittest.mm

@@ -20,12 +20,15 @@
 #include "ios/chrome/browser/browser_state/test_chrome_browser_state.h"
 #include "ios/chrome/browser/browsing_data/browsing_data_features.h"
 #include "ios/chrome/browser/browsing_data/cache_counter.h"
+#include "ios/chrome/browser/browsing_data/fake_browsing_data_remover.h"
 #include "ios/chrome/browser/pref_names.h"
 #include "ios/chrome/browser/prefs/browser_prefs.h"
 #include "ios/chrome/browser/signin/identity_test_environment_chrome_browser_state_adaptor.h"
 #include "ios/chrome/browser/sync/profile_sync_service_factory.h"
 #import "ios/chrome/browser/ui/collection_view/collection_view_controller_test.h"
 #import "ios/chrome/browser/ui/settings/cells/settings_text_item.h"
+#import "ios/chrome/browser/ui/settings/clear_browsing_data/clear_browsing_data_manager.h"
+#import "ios/chrome/browser/ui/settings/clear_browsing_data/fake_browsing_data_counter_wrapper_producer.h"
 #import "ios/chrome/common/string_util.h"
 #include "ios/chrome/grit/ios_strings.h"
 #include "ios/web/public/test/test_web_thread_bundle.h"
@@ -78,6 +81,8 @@ class ClearBrowsingDataCollectionViewControllerTest
 
     test_sync_service_ = static_cast<syncer::TestSyncService*>(
         ProfileSyncServiceFactory::GetForBrowserState(browser_state_.get()));
+
+    remover_ = std::make_unique<FakeBrowsingDataRemover>();
   }
 
   std::unique_ptr<sync_preferences::PrefServiceSyncable> CreatePrefService() {
@@ -91,8 +96,16 @@ class ClearBrowsingDataCollectionViewControllerTest
   }
 
   CollectionViewController* InstantiateController() override {
+    ClearBrowsingDataManager* manager = [[ClearBrowsingDataManager alloc]
+                      initWithBrowserState:browser_state_.get()
+                                  listType:ClearBrowsingDataListType::
+                                               kListTypeCollectionView
+                       browsingDataRemover:remover_.get()
+        browsingDataCounterWrapperProducer:
+            [[FakeBrowsingDataCounterWrapperProducer alloc] init]];
     return [[ClearBrowsingDataCollectionViewController alloc]
-        initWithBrowserState:browser_state_.get()];
+        initWithBrowserState:browser_state_.get()
+                     manager:manager];
   }
 
   void SelectItem(int item, int section) {
@@ -111,6 +124,7 @@ class ClearBrowsingDataCollectionViewControllerTest
   std::unique_ptr<IdentityTestEnvironmentChromeBrowserStateAdaptor>
       identity_test_env_adaptor_;
   syncer::TestSyncService* test_sync_service_;
+  std::unique_ptr<BrowsingDataRemover> remover_;
 };
 
 // Tests ClearBrowsingDataCollectionViewControllerTest is set up with all
@@ -126,23 +140,22 @@ TEST_F(ClearBrowsingDataCollectionViewControllerTest, TestModel) {
     section_offset = 1;
   }
 
-  CheckTextCellTitleWithId(IDS_IOS_CLEAR_BROWSING_HISTORY, 0 + section_offset,
+  CheckTextCellTextWithId(IDS_IOS_CLEAR_BROWSING_HISTORY, 0 + section_offset,
                           0);
   CheckAccessoryType(MDCCollectionViewCellAccessoryCheckmark,
                      0 + section_offset, 0);
-  CheckTextCellTitleWithId(IDS_IOS_CLEAR_COOKIES, 0 + section_offset, 1);
+  CheckTextCellTextWithId(IDS_IOS_CLEAR_COOKIES, 0 + section_offset, 1);
   CheckAccessoryType(MDCCollectionViewCellAccessoryCheckmark,
                      0 + section_offset, 1);
-  CheckTextCellTitleWithId(IDS_IOS_CLEAR_CACHE, 0 + section_offset, 2);
+  CheckTextCellTextWithId(IDS_IOS_CLEAR_CACHE, 0 + section_offset, 2);
   CheckAccessoryType(MDCCollectionViewCellAccessoryCheckmark,
                      0 + section_offset, 2);
-  CheckTextCellTitleWithId(IDS_IOS_CLEAR_SAVED_PASSWORDS, 0 + section_offset,
-                           3);
+  CheckTextCellTextWithId(IDS_IOS_CLEAR_SAVED_PASSWORDS, 0 + section_offset, 3);
   CheckAccessoryType(MDCCollectionViewCellAccessoryNone, 0 + section_offset, 3);
-  CheckTextCellTitleWithId(IDS_IOS_CLEAR_AUTOFILL, 0 + section_offset, 4);
+  CheckTextCellTextWithId(IDS_IOS_CLEAR_AUTOFILL, 0 + section_offset, 4);
   CheckAccessoryType(MDCCollectionViewCellAccessoryNone, 0 + section_offset, 4);
 
-  CheckTextCellTitleWithId(IDS_IOS_CLEAR_BUTTON, 1 + section_offset, 0);
+  CheckTextCellTextWithId(IDS_IOS_CLEAR_BUTTON, 1 + section_offset, 0);
 
   CheckSectionFooterWithId(IDS_IOS_CLEAR_BROWSING_DATA_FOOTER_SAVED_SITE_DATA,
                            2 + section_offset);
@@ -169,7 +182,8 @@ TEST_F(ClearBrowsingDataCollectionViewControllerTest,
   EXPECT_EQ(1, NumberOfItemsInSection(1 + section_offset));
 
   EXPECT_EQ(1, NumberOfItemsInSection(2 + section_offset));
-  CheckSectionFooterWithId(IDS_IOS_CLEAR_BROWSING_DATA_FOOTER_ACCOUNT, 2);
+  CheckSectionFooterWithId(IDS_IOS_CLEAR_BROWSING_DATA_FOOTER_ACCOUNT,
+                           2 + section_offset);
 
   EXPECT_EQ(1, NumberOfItemsInSection(3 + section_offset));
   CheckSectionFooterWithId(IDS_IOS_CLEAR_BROWSING_DATA_FOOTER_SAVED_SITE_DATA,

ios/chrome/browser/ui/settings/clear_browsing_data/clear_browsing_data_manager.h

@@ -75,9 +75,9 @@ enum class ClearBrowsingDataListType {
                 TimeRangeSelectorTableViewControllerDelegate>
 
 // The manager's consumer.
-@property(nonatomic, assign) id<ClearBrowsingDataConsumer> consumer;
+@property(nonatomic, weak) id<ClearBrowsingDataConsumer> consumer;
 // Reference to the LinkDelegate for CollectionViewFooterItem.
-@property(nonatomic, strong) id<CollectionViewFooterLinkDelegate> linkDelegate;
+@property(nonatomic, weak) id<CollectionViewFooterLinkDelegate> linkDelegate;
 
 // Default init method. |browserState| can't be nil and
 // |listType| determines what kind of items to populate model with.