Commit af057822 authored by Gauthier Ambard's avatar Gauthier Ambard Committed by Commit Bot

[iOS] Migrate BandwidthManagement to UITableView

This CL migrates the BandwidthManagementCollectionViewController to a
UITableViewController base class.
It also creates the SettingsRootViewControlling protocol to allow easier
migration between the CollectionVC and the TableVC in settings.

Bug: 894791, 894837
Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs
Change-Id: I7710c43f38a65359743336745afd8510ed8f9591
Reviewed-on: https://chromium-review.googlesource.com/c/1277788
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Reviewed-by: default avatarSergio Collazos <sczs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600728}
parent a9fb0358
...@@ -22,8 +22,8 @@ source_set("settings") { ...@@ -22,8 +22,8 @@ source_set("settings") {
"autofill_profile_collection_view_controller.mm", "autofill_profile_collection_view_controller.mm",
"autofill_profile_edit_collection_view_controller.h", "autofill_profile_edit_collection_view_controller.h",
"autofill_profile_edit_collection_view_controller.mm", "autofill_profile_edit_collection_view_controller.mm",
"bandwidth_management_collection_view_controller.h", "bandwidth_management_table_view_controller.h",
"bandwidth_management_collection_view_controller.mm", "bandwidth_management_table_view_controller.mm",
"bar_button_activity_indicator.h", "bar_button_activity_indicator.h",
"bar_button_activity_indicator.mm", "bar_button_activity_indicator.mm",
"block_popups_collection_view_controller.h", "block_popups_collection_view_controller.h",
...@@ -84,6 +84,7 @@ source_set("settings") { ...@@ -84,6 +84,7 @@ source_set("settings") {
"settings_root_collection_view_controller.mm", "settings_root_collection_view_controller.mm",
"settings_root_table_view_controller.h", "settings_root_table_view_controller.h",
"settings_root_table_view_controller.mm", "settings_root_table_view_controller.mm",
"settings_root_view_controlling.h",
"settings_utils.h", "settings_utils.h",
"settings_utils.mm", "settings_utils.mm",
"sync_create_passphrase_collection_view_controller.h", "sync_create_passphrase_collection_view_controller.h",
...@@ -271,7 +272,7 @@ source_set("unit_tests") { ...@@ -271,7 +272,7 @@ source_set("unit_tests") {
"autofill_credit_card_collection_view_controller_unittest.mm", "autofill_credit_card_collection_view_controller_unittest.mm",
"autofill_profile_collection_view_controller_unittest.mm", "autofill_profile_collection_view_controller_unittest.mm",
"autofill_profile_edit_collection_view_controller_unittest.mm", "autofill_profile_edit_collection_view_controller_unittest.mm",
"bandwidth_management_collection_view_controller_unittest.mm", "bandwidth_management_table_view_controller_unittest.mm",
"block_popups_collection_view_controller_unittest.mm", "block_popups_collection_view_controller_unittest.mm",
"clear_browsing_data_collection_view_controller_unittest.mm", "clear_browsing_data_collection_view_controller_unittest.mm",
"clear_browsing_data_manager_unittest.mm", "clear_browsing_data_manager_unittest.mm",
...@@ -344,6 +345,8 @@ source_set("unit_tests") { ...@@ -344,6 +345,8 @@ source_set("unit_tests") {
"//ios/chrome/browser/ui/icons", "//ios/chrome/browser/ui/icons",
"//ios/chrome/browser/ui/settings/cells", "//ios/chrome/browser/ui/settings/cells",
"//ios/chrome/browser/ui/settings/sync_utils", "//ios/chrome/browser/ui/settings/sync_utils",
"//ios/chrome/browser/ui/table_view",
"//ios/chrome/browser/ui/table_view:test_support",
"//ios/chrome/browser/voice", "//ios/chrome/browser/voice",
"//ios/chrome/browser/web", "//ios/chrome/browser/web",
"//ios/chrome/browser/web:test_support", "//ios/chrome/browser/web:test_support",
......
...@@ -2,10 +2,10 @@ ...@@ -2,10 +2,10 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
#ifndef IOS_CHROME_BROWSER_UI_SETTINGS_BANDWIDTH_MANAGEMENT_COLLECTION_VIEW_CONTROLLER_H_ #ifndef IOS_CHROME_BROWSER_UI_SETTINGS_BANDWIDTH_MANAGEMENT_TABLE_VIEW_CONTROLLER_H_
#define IOS_CHROME_BROWSER_UI_SETTINGS_BANDWIDTH_MANAGEMENT_COLLECTION_VIEW_CONTROLLER_H_ #define IOS_CHROME_BROWSER_UI_SETTINGS_BANDWIDTH_MANAGEMENT_TABLE_VIEW_CONTROLLER_H_
#import "ios/chrome/browser/ui/settings/settings_root_collection_view_controller.h" #import "ios/chrome/browser/ui/settings/settings_root_table_view_controller.h"
namespace ios { namespace ios {
class ChromeBrowserState; class ChromeBrowserState;
...@@ -13,15 +13,16 @@ class ChromeBrowserState; ...@@ -13,15 +13,16 @@ class ChromeBrowserState;
// Controller for the UI that allows the user to change settings that affect // Controller for the UI that allows the user to change settings that affect
// bandwidth usage: prefetching and the data reduction proxy. // bandwidth usage: prefetching and the data reduction proxy.
@interface BandwidthManagementCollectionViewController @interface BandwidthManagementTableViewController
: SettingsRootCollectionViewController : SettingsRootTableViewController
// The designated initializer. |browserState| must not be nil. // The designated initializer. |browserState| must not be nil.
- (instancetype)initWithBrowserState:(ios::ChromeBrowserState*)browserState - (instancetype)initWithBrowserState:(ios::ChromeBrowserState*)browserState
NS_DESIGNATED_INITIALIZER; NS_DESIGNATED_INITIALIZER;
- (instancetype)initWithLayout:(UICollectionViewLayout*)layout - (instancetype)initWithTableViewStyle:(UITableViewStyle)style
style:(CollectionViewControllerStyle)style appBarStyle:
(ChromeTableViewControllerStyle)appBarStyle
NS_UNAVAILABLE; NS_UNAVAILABLE;
@end @end
#endif // IOS_CHROME_BROWSER_UI_SETTINGS_BANDWIDTH_MANAGEMENT_COLLECTION_VIEW_CONTROLLER_H_ #endif // IOS_CHROME_BROWSER_UI_SETTINGS_BANDWIDTH_MANAGEMENT_TABLE_VIEW_CONTROLLER_H_
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
#import "ios/chrome/browser/ui/settings/bandwidth_management_collection_view_controller.h" #import "ios/chrome/browser/ui/settings/bandwidth_management_table_view_controller.h"
#include <memory> #include <memory>
...@@ -16,8 +16,9 @@ ...@@ -16,8 +16,9 @@
#include "ios/chrome/browser/browser_state/test_chrome_browser_state.h" #include "ios/chrome/browser/browser_state/test_chrome_browser_state.h"
#include "ios/chrome/browser/pref_names.h" #include "ios/chrome/browser/pref_names.h"
#include "ios/chrome/browser/prefs/browser_prefs.h" #include "ios/chrome/browser/prefs/browser_prefs.h"
#import "ios/chrome/browser/ui/collection_view/collection_view_controller_test.h"
#import "ios/chrome/browser/ui/settings/dataplan_usage_collection_view_controller.h" #import "ios/chrome/browser/ui/settings/dataplan_usage_collection_view_controller.h"
#import "ios/chrome/browser/ui/table_view/chrome_table_view_controller_test.h"
#import "ios/chrome/browser/ui/table_view/table_view_model.h"
#include "ios/chrome/grit/ios_strings.h" #include "ios/chrome/grit/ios_strings.h"
#include "ios/chrome/test/ios_chrome_scoped_testing_local_state.h" #include "ios/chrome/test/ios_chrome_scoped_testing_local_state.h"
#include "ios/chrome/test/testing_application_context.h" #include "ios/chrome/test/testing_application_context.h"
...@@ -32,14 +33,14 @@ ...@@ -32,14 +33,14 @@
namespace { namespace {
class BandwidthManagementCollectionViewControllerTest class BandwidthManagementTableViewControllerTest
: public CollectionViewControllerTest { : public ChromeTableViewControllerTest {
public: public:
BandwidthManagementCollectionViewControllerTest() {} BandwidthManagementTableViewControllerTest() {}
protected: protected:
void SetUp() override { void SetUp() override {
CollectionViewControllerTest::SetUp(); ChromeTableViewControllerTest::SetUp();
sync_preferences::PrefServiceMockFactory factory; sync_preferences::PrefServiceMockFactory factory;
scoped_refptr<user_prefs::PrefRegistrySyncable> registry( scoped_refptr<user_prefs::PrefRegistrySyncable> registry(
...@@ -56,11 +57,11 @@ class BandwidthManagementCollectionViewControllerTest ...@@ -56,11 +57,11 @@ class BandwidthManagementCollectionViewControllerTest
void TearDown() override { void TearDown() override {
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
CollectionViewControllerTest::TearDown(); ChromeTableViewControllerTest::TearDown();
} }
CollectionViewController* InstantiateController() override { ChromeTableViewController* InstantiateController() override {
return [[BandwidthManagementCollectionViewController alloc] return [[BandwidthManagementTableViewController alloc]
initWithBrowserState:chrome_browser_state_.get()]; initWithBrowserState:chrome_browser_state_.get()];
} }
...@@ -71,12 +72,11 @@ class BandwidthManagementCollectionViewControllerTest ...@@ -71,12 +72,11 @@ class BandwidthManagementCollectionViewControllerTest
std::unique_ptr<TestChromeBrowserState> chrome_browser_state_; std::unique_ptr<TestChromeBrowserState> chrome_browser_state_;
}; };
TEST_F(BandwidthManagementCollectionViewControllerTest, TestModel) { TEST_F(BandwidthManagementTableViewControllerTest, TestModel) {
CheckController(); CheckController();
EXPECT_EQ(2, NumberOfSections()); EXPECT_EQ(1, NumberOfSections());
const NSInteger action_section = 0; EXPECT_EQ(1, NumberOfItemsInSection(0));
EXPECT_EQ(1, NumberOfItemsInSection(action_section));
// Preload webpages item. // Preload webpages item.
NSString* expected_title = NSString* expected_title =
l10n_util::GetNSString(IDS_IOS_OPTIONS_PRELOAD_WEBPAGES); l10n_util::GetNSString(IDS_IOS_OPTIONS_PRELOAD_WEBPAGES);
...@@ -84,8 +84,8 @@ TEST_F(BandwidthManagementCollectionViewControllerTest, TestModel) { ...@@ -84,8 +84,8 @@ TEST_F(BandwidthManagementCollectionViewControllerTest, TestModel) {
currentLabelForPreference:chrome_browser_state_->GetPrefs() currentLabelForPreference:chrome_browser_state_->GetPrefs()
basePref:prefs::kNetworkPredictionEnabled basePref:prefs::kNetworkPredictionEnabled
wifiPref:prefs::kNetworkPredictionWifiOnly]; wifiPref:prefs::kNetworkPredictionWifiOnly];
CheckTextCellTitleAndSubtitle(expected_title, expected_subtitle, CheckTextCellTextAndDetailText(expected_title, expected_subtitle, 0, 0);
action_section, 0); EXPECT_NE(nil, [controller().tableViewModel footerForSection:0]);
} }
} // namespace } // namespace
...@@ -48,6 +48,7 @@ const CGFloat kMinDetailTextWidthRatio = 0.25f; ...@@ -48,6 +48,7 @@ const CGFloat kMinDetailTextWidthRatio = 0.25f;
self = [super initWithType:type]; self = [super initWithType:type];
if (self) { if (self) {
self.cellClass = [SettingsDetailCell class]; self.cellClass = [SettingsDetailCell class];
_cellBackgroundColor = [UIColor whiteColor];
} }
return self; return self;
} }
......
...@@ -46,7 +46,7 @@ ...@@ -46,7 +46,7 @@
#import "ios/chrome/browser/ui/settings/accounts_collection_view_controller.h" #import "ios/chrome/browser/ui/settings/accounts_collection_view_controller.h"
#import "ios/chrome/browser/ui/settings/autofill_credit_card_collection_view_controller.h" #import "ios/chrome/browser/ui/settings/autofill_credit_card_collection_view_controller.h"
#import "ios/chrome/browser/ui/settings/autofill_profile_collection_view_controller.h" #import "ios/chrome/browser/ui/settings/autofill_profile_collection_view_controller.h"
#import "ios/chrome/browser/ui/settings/bandwidth_management_collection_view_controller.h" #import "ios/chrome/browser/ui/settings/bandwidth_management_table_view_controller.h"
#import "ios/chrome/browser/ui/settings/cells/account_signin_item.h" #import "ios/chrome/browser/ui/settings/cells/account_signin_item.h"
#import "ios/chrome/browser/ui/settings/cells/legacy/legacy_settings_detail_item.h" #import "ios/chrome/browser/ui/settings/cells/legacy/legacy_settings_detail_item.h"
#import "ios/chrome/browser/ui/settings/cells/settings_switch_item.h" #import "ios/chrome/browser/ui/settings/cells/settings_switch_item.h"
...@@ -834,7 +834,7 @@ void IdentityObserverBridge::OnPrimaryAccountCleared( ...@@ -834,7 +834,7 @@ void IdentityObserverBridge::OnPrimaryAccountCleared(
NSInteger itemType = NSInteger itemType =
[self.collectionViewModel itemTypeForIndexPath:indexPath]; [self.collectionViewModel itemTypeForIndexPath:indexPath];
SettingsRootCollectionViewController* controller; UIViewController<SettingsRootViewControlling>* controller;
switch (itemType) { switch (itemType) {
case ItemTypeSignInButton: case ItemTypeSignInButton:
...@@ -883,7 +883,7 @@ void IdentityObserverBridge::OnPrimaryAccountCleared( ...@@ -883,7 +883,7 @@ void IdentityObserverBridge::OnPrimaryAccountCleared(
initWithBrowserState:_browserState]; initWithBrowserState:_browserState];
break; break;
case ItemTypeBandwidth: case ItemTypeBandwidth:
controller = [[BandwidthManagementCollectionViewController alloc] controller = [[BandwidthManagementTableViewController alloc]
initWithBrowserState:_browserState]; initWithBrowserState:_browserState];
break; break;
case ItemTypeAboutChrome: case ItemTypeAboutChrome:
......
...@@ -7,12 +7,12 @@ ...@@ -7,12 +7,12 @@
#import "ios/chrome/browser/ui/collection_view/cells/collection_view_footer_item.h" #import "ios/chrome/browser/ui/collection_view/cells/collection_view_footer_item.h"
#import "ios/chrome/browser/ui/collection_view/collection_view_controller.h" #import "ios/chrome/browser/ui/collection_view/collection_view_controller.h"
#import "ios/chrome/browser/ui/settings/settings_root_view_controlling.h"
@protocol ApplicationCommands;
// Root class for collection view controllers in settings. // Root class for collection view controllers in settings.
@interface SettingsRootCollectionViewController @interface SettingsRootCollectionViewController
: CollectionViewController<CollectionViewFooterLinkDelegate> : CollectionViewController<CollectionViewFooterLinkDelegate,
SettingsRootViewControlling>
// Creates an autoreleased Edit button for the collection view. Calls // Creates an autoreleased Edit button for the collection view. Calls
// |editButtonEnabled| to determine if the button should be enabled. // |editButtonEnabled| to determine if the button should be enabled.
...@@ -38,9 +38,6 @@ ...@@ -38,9 +38,6 @@
// The collection view accessibility identifier to be set on |viewDidLoad|. // The collection view accessibility identifier to be set on |viewDidLoad|.
@property(nonatomic, copy) NSString* collectionViewAccessibilityIdentifier; @property(nonatomic, copy) NSString* collectionViewAccessibilityIdentifier;
// The dispatcher used by this ViewController.
@property(nonatomic, weak) id<ApplicationCommands> dispatcher;
@end @end
// Subclasses of SettingsRootCollectionViewController should implement the // Subclasses of SettingsRootCollectionViewController should implement the
......
...@@ -8,13 +8,17 @@ ...@@ -8,13 +8,17 @@
#import <UIKit/UIKit.h> #import <UIKit/UIKit.h>
#import "ios/chrome/browser/ui/material_components/app_bar_view_controller_presenting.h" #import "ios/chrome/browser/ui/material_components/app_bar_view_controller_presenting.h"
#import "ios/chrome/browser/ui/settings/settings_root_view_controlling.h"
#import "ios/chrome/browser/ui/table_view/cells/table_view_link_header_footer_item.h"
#import "ios/chrome/browser/ui/table_view/chrome_table_view_controller.h" #import "ios/chrome/browser/ui/table_view/chrome_table_view_controller.h"
// SettingsRootTableViewController is a base class for integrating UITableViews // SettingsRootTableViewController is a base class for integrating UITableViews
// into the Settings UI. It handles the configuration and display of the MDC // into the Settings UI. It handles the configuration and display of the MDC
// AppBar. // AppBar.
@interface SettingsRootTableViewController @interface SettingsRootTableViewController
: ChromeTableViewController<AppBarViewControllerPresenting> : ChromeTableViewController<AppBarViewControllerPresenting,
SettingsRootViewControlling,
TableViewLinkHeaderFooterItemDelegate>
@end @end
#endif // IOS_CHROME_BROWSER_UI_SETTINGS_SETTINGS_ROOT_TABLE_VIEW_CONTROLLER_H_ #endif // IOS_CHROME_BROWSER_UI_SETTINGS_SETTINGS_ROOT_TABLE_VIEW_CONTROLLER_H_
...@@ -5,14 +5,28 @@ ...@@ -5,14 +5,28 @@
#import "ios/chrome/browser/ui/settings/settings_root_table_view_controller.h" #import "ios/chrome/browser/ui/settings/settings_root_table_view_controller.h"
#import "base/mac/foundation_util.h" #import "base/mac/foundation_util.h"
#import "ios/chrome/browser/ui/commands/application_commands.h"
#import "ios/chrome/browser/ui/commands/open_new_tab_command.h"
#import "ios/chrome/browser/ui/material_components/utils.h" #import "ios/chrome/browser/ui/material_components/utils.h"
#import "ios/chrome/browser/ui/settings/settings_navigation_controller.h" #import "ios/chrome/browser/ui/settings/settings_navigation_controller.h"
#import "ios/chrome/browser/ui/table_view/chrome_table_view_styler.h"
#import "ios/chrome/browser/ui/uikit_ui_util.h"
#if !defined(__has_feature) || !__has_feature(objc_arc) #if !defined(__has_feature) || !__has_feature(objc_arc)
#error "This file requires ARC support." #error "This file requires ARC support."
#endif #endif
@implementation SettingsRootTableViewController @implementation SettingsRootTableViewController
@synthesize dispatcher = _dispatcher;
- (void)viewDidLoad {
self.styler.tableViewBackgroundColor =
[UIColor groupTableViewBackgroundColor];
self.styler.tableViewSectionHeaderBlurEffect = nil;
[super viewDidLoad];
}
- (void)viewWillAppear:(BOOL)animated { - (void)viewWillAppear:(BOOL)animated {
[super viewWillAppear:animated]; [super viewWillAppear:animated];
...@@ -24,4 +38,13 @@ ...@@ -24,4 +38,13 @@
self.navigationItem.rightBarButtonItem = doneButton; self.navigationItem.rightBarButtonItem = doneButton;
} }
#pragma mark - TableViewLinkHeaderFooterItemDelegate
- (void)view:(TableViewLinkHeaderFooterView*)view didTapLinkURL:(GURL)URL {
// Subclass must have a valid dispatcher assigned.
DCHECK(self.dispatcher);
OpenNewTabCommand* command = [OpenNewTabCommand commandWithURLFromChrome:URL];
[self.dispatcher closeSettingsUIAndOpenURL:command];
}
@end @end
// 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_SETTINGS_SETTINGS_ROOT_VIEW_CONTROLLING_H_
#define IOS_CHROME_BROWSER_UI_SETTINGS_SETTINGS_ROOT_VIEW_CONTROLLING_H_
#import <Foundation/Foundation.h>
@protocol ApplicationCommands;
// TODO(crbug.com/894800): This protocol is added to have a common interface
// between the SettingsRootViewControllers for table views and collections.
// Remove it once it is completed.
@protocol SettingsRootViewControlling
// The dispatcher used by this ViewController.
@property(nonatomic, weak) id<ApplicationCommands> dispatcher;
@end
#endif // IOS_CHROME_BROWSER_UI_SETTINGS_SETTINGS_ROOT_VIEW_CONTROLLING_H_
...@@ -119,6 +119,8 @@ const int kTextColor = 0x8A8A8F; ...@@ -119,6 +119,8 @@ const int kTextColor = 0x8A8A8F;
- (void)prepareForReuse { - (void)prepareForReuse {
[super prepareForReuse]; [super prepareForReuse];
[self setText:nil]; [self setText:nil];
self.delegate = nil;
self.linkURL = GURL();
} }
#pragma mark - UITextViewDelegate #pragma mark - UITextViewDelegate
......
...@@ -56,7 +56,6 @@ void ChromeTableViewControllerTest::CheckController() { ...@@ -56,7 +56,6 @@ void ChromeTableViewControllerTest::CheckController() {
EXPECT_TRUE([controller_ view]); EXPECT_TRUE([controller_ view]);
EXPECT_TRUE([controller_ tableView]); EXPECT_TRUE([controller_ tableView]);
EXPECT_TRUE([controller_ tableViewModel]); EXPECT_TRUE([controller_ tableViewModel]);
EXPECT_EQ(controller_, [controller_ tableView].dataSource);
EXPECT_EQ(controller_, [controller_ tableView].delegate); EXPECT_EQ(controller_, [controller_ tableView].delegate);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment