Commit c4774e6c authored by Mark Cogan's avatar Mark Cogan Committed by Commit Bot

[iOS] Pass Browser into SettingsNavigationController initializers.

This is the first CL of several breaking crrev.com/c/1860025 into smaller
chunks.

This updates most of the SettingsNavigationController class inits to take
a Browser instead of a BrowserState. One of the inits (the import data
one) is called from AuthenticationFlowPerformer, which doesn't yet have
a Browser, so it's initialized with a BrowserState instead. Because of this,
SettingsNavigationController itself is initialized with a BrowserState.

There's also some cleanup of these init names. Prior to this change, none
of the class methods (for example: "newAccountsController:delegate:")
properly identified the first parameter. Some of the method signatures were
also impossible for clang-format to wrap to 80 characters. So:
 - The return type of all of these methods is now 'instancetype'.
 - The 'new' prefix is dropped from all of them, matching current style.
 - Methods that had 'WithBrowserState:' now have 'ForBrowser:', aside from
   -importDataControllerForBrowserState:delegate:importDataDelegate:
    fromEmail:toEmail:isSignedIn:.
 - Methods that didn't have a keyword for the BrowserState parameter also
   now have "ForBrowser:".
 - "syncEncryptionPassphrase" is shortened to "syncPassphrase", since
   "Encryption" doesn't add any useful information. The full method name is
   now +syncPassphraseControllerForBrowser:delegate:.
 - "autofillProfille" is corrected to "autofillProfile" (one "l" in
   "profile", The full method name is now
   +autofillProfileControllerForBrowser:delegate:
 - "settingsMainController" is reordered to "mainSettings". The full method
   name is now +mainSettingsControllerForBrowser:delegate:

BrowserInterface didn't have a 'browser' property, which it clearly needed.
This CL adds one.

Change-Id: I6b0a8c8583dc69485a0f65ac1d069de7b417d564
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1862915
Commit-Queue: Mark Cogan <marq@chromium.org>
Reviewed-by: default avatarGauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#706481}
parent 903f189e
......@@ -200,6 +200,7 @@ source_set("app_internal") {
"//ios/chrome/browser/geolocation:geolocation_internal",
"//ios/chrome/browser/history",
"//ios/chrome/browser/mailto:feature_flags",
"//ios/chrome/browser/main",
"//ios/chrome/browser/memory",
"//ios/chrome/browser/metrics",
"//ios/chrome/browser/metrics:metrics_internal",
......
......@@ -86,6 +86,7 @@
#include "ios/chrome/browser/geolocation/omnibox_geolocation_controller.h"
#include "ios/chrome/browser/ios_chrome_io_thread.h"
#include "ios/chrome/browser/mailto/features.h"
#include "ios/chrome/browser/main/browser.h"
#import "ios/chrome/browser/memory/memory_debugger_manager.h"
#include "ios/chrome/browser/metrics/first_user_action_recorder.h"
#import "ios/chrome/browser/metrics/previous_session_info.h"
......@@ -1605,8 +1606,9 @@ enum class EnterTabSwitcherSnapshotResult {
DCHECK(!self.signinInteractionCoordinator.isSettingsViewPresented);
if (_settingsNavigationController)
return;
_settingsNavigationController = [SettingsNavigationController
newAutofillProfilleController:_mainBrowserState
Browser* browser = self.interfaceProvider.mainInterface.browser;
_settingsNavigationController =
[SettingsNavigationController autofillProfileControllerForBrowser:browser
delegate:self];
[baseViewController presentViewController:_settingsNavigationController
animated:YES
......@@ -1623,8 +1625,9 @@ enum class EnterTabSwitcherSnapshotResult {
DCHECK(!self.signinInteractionCoordinator.isSettingsViewPresented);
if (_settingsNavigationController)
return;
_settingsNavigationController = [SettingsNavigationController
newUserFeedbackController:_mainBrowserState
Browser* browser = self.interfaceProvider.mainInterface.browser;
_settingsNavigationController =
[SettingsNavigationController userFeedbackControllerForBrowser:browser
delegate:self
feedbackDataSource:self
dispatcher:self];
......@@ -1726,8 +1729,10 @@ enum class EnterTabSwitcherSnapshotResult {
showAccountsSettingsFromViewController:baseViewController];
return;
}
_settingsNavigationController = [SettingsNavigationController
newAccountsController:self.currentBrowserState
Browser* browser = self.interfaceProvider.mainInterface.browser;
_settingsNavigationController =
[SettingsNavigationController accountsControllerForBrowser:browser
delegate:self];
[baseViewController presentViewController:_settingsNavigationController
animated:YES
......@@ -1751,10 +1756,9 @@ enum class EnterTabSwitcherSnapshotResult {
return;
}
ios::ChromeBrowserState* originalBrowserState =
self.currentBrowserState->GetOriginalChromeBrowserState();
_settingsNavigationController = [SettingsNavigationController
newGoogleServicesController:originalBrowserState
Browser* browser = self.interfaceProvider.mainInterface.browser;
_settingsNavigationController =
[SettingsNavigationController googleServicesControllerForBrowser:browser
delegate:self];
[baseViewController presentViewController:_settingsNavigationController
animated:YES
......@@ -1770,8 +1774,9 @@ enum class EnterTabSwitcherSnapshotResult {
showSyncPassphraseSettingsFromViewController:baseViewController];
return;
}
_settingsNavigationController = [SettingsNavigationController
newSyncEncryptionPassphraseController:_mainBrowserState
Browser* browser = self.interfaceProvider.mainInterface.browser;
_settingsNavigationController =
[SettingsNavigationController syncPassphraseControllerForBrowser:browser
delegate:self];
[baseViewController presentViewController:_settingsNavigationController
animated:YES
......@@ -1787,8 +1792,9 @@ enum class EnterTabSwitcherSnapshotResult {
showSavedPasswordsSettingsFromViewController:baseViewController];
return;
}
Browser* browser = self.interfaceProvider.mainInterface.browser;
_settingsNavigationController =
[SettingsNavigationController newSavePasswordsController:_mainBrowserState
[SettingsNavigationController savePasswordsControllerForBrowser:browser
delegate:self];
[baseViewController presentViewController:_settingsNavigationController
animated:YES
......@@ -1804,8 +1810,9 @@ enum class EnterTabSwitcherSnapshotResult {
showProfileSettingsFromViewController:baseViewController];
return;
}
_settingsNavigationController = [SettingsNavigationController
newAutofillProfilleController:_mainBrowserState
Browser* browser = self.interfaceProvider.mainInterface.browser;
_settingsNavigationController =
[SettingsNavigationController autofillProfileControllerForBrowser:browser
delegate:self];
[baseViewController presentViewController:_settingsNavigationController
animated:YES
......@@ -1821,8 +1828,9 @@ enum class EnterTabSwitcherSnapshotResult {
showCreditCardSettingsFromViewController:baseViewController];
return;
}
Browser* browser = self.interfaceProvider.mainInterface.browser;
_settingsNavigationController = [SettingsNavigationController
newAutofillCreditCardController:_mainBrowserState
autofillCreditCardControllerForBrowser:browser
delegate:self];
[baseViewController presentViewController:_settingsNavigationController
animated:YES
......@@ -2264,8 +2272,9 @@ enum class EnterTabSwitcherSnapshotResult {
[[DeferredInitializationRunner sharedInstance]
runBlockIfNecessary:kPrefObserverInit];
DCHECK(_localStatePrefObserverBridge);
_settingsNavigationController = [SettingsNavigationController
newSettingsMainControllerWithBrowserState:_mainBrowserState
Browser* browser = self.interfaceProvider.mainInterface.browser;
_settingsNavigationController =
[SettingsNavigationController mainSettingsControllerForBrowser:browser
delegate:self];
[baseViewController presentViewController:_settingsNavigationController
animated:YES
......
......@@ -270,8 +270,8 @@ const int64_t kAuthenticationFlowTimeoutSeconds = 10;
viewController:viewController];
return;
}
_navigationController =
[SettingsNavigationController newImportDataController:browserState
_navigationController = [SettingsNavigationController
importDataControllerForBrowserState:browserState
delegate:self
importDataDelegate:self
fromEmail:lastSignedInEmail
......
......@@ -9,6 +9,7 @@
#include "base/ios/block_types.h"
class Browser;
@class BrowserCoordinator;
@class BrowserViewController;
@class TabModel;
......@@ -39,6 +40,8 @@ class ChromeBrowserState;
@property(nonatomic, readonly) BrowserViewController* bvc;
// The tab model to which the current tab belongs.
@property(nonatomic, readonly) TabModel* tabModel;
// The active browser.
@property(nonatomic, readonly) Browser* browser;
// The browser state for this interface.
@property(nonatomic, readonly) ios::ChromeBrowserState* browserState;
// YES if the tab view is available for user interaction.
......
......@@ -61,6 +61,10 @@
return self.coordinator.tabModel;
}
- (Browser*)browser {
return self.coordinator.browser;
}
- (ios::ChromeBrowserState*)browserState {
return self.coordinator.viewController.browserState;
}
......
......@@ -9,6 +9,7 @@
#import "ios/chrome/browser/ui/main/browser_interface_provider.h"
class Browser;
@class BrowserViewController;
namespace ios {
class ChromeBrowserState;
......@@ -21,6 +22,7 @@ class ChromeBrowserState;
@property(nonatomic, readwrite) UIViewController* viewController;
@property(nonatomic, readwrite) BrowserViewController* bvc;
@property(nonatomic, readwrite) TabModel* tabModel;
@property(nonatomic, readwrite) Browser* browser;
@property(nonatomic, readwrite) ios::ChromeBrowserState* browserState;
@property(nonatomic, readwrite) BOOL incognito;
@end
......
......@@ -130,6 +130,7 @@ source_set("settings") {
"//ios/chrome/browser/feature_engagement",
"//ios/chrome/browser/history",
"//ios/chrome/browser/mailto:feature_flags",
"//ios/chrome/browser/main",
"//ios/chrome/browser/passwords",
"//ios/chrome/browser/search_engines",
"//ios/chrome/browser/signin",
......@@ -221,6 +222,7 @@ source_set("test_support") {
"//google_apis",
"//ios/chrome/browser/browser_state",
"//ios/chrome/browser/browser_state:test_support",
"//ios/chrome/browser/main:test_support",
"//ios/chrome/browser/prefs:browser_prefs",
"//ios/chrome/browser/signin",
"//ios/chrome/browser/signin:test_support",
......@@ -281,6 +283,7 @@ source_set("unit_tests") {
"//ios/chrome/browser/browsing_data:counters",
"//ios/chrome/browser/content_settings",
"//ios/chrome/browser/mailto:feature_flags",
"//ios/chrome/browser/main:test_support",
"//ios/chrome/browser/passwords",
"//ios/chrome/browser/prefs:browser_prefs",
"//ios/chrome/browser/search_engines",
......
......@@ -9,6 +9,7 @@
#import "ios/chrome/browser/ui/commands/application_commands.h"
class Browser;
@protocol BrowserCommands;
@protocol ImportDataControllerDelegate;
@protocol UserFeedbackDataSource;
......@@ -54,87 +55,100 @@ extern NSString* const kSettingsDoneButtonId;
@interface SettingsNavigationController
: UINavigationController<ApplicationSettingsCommands>
// Returns the browser state associated with this view controller;
@property(nonatomic, readonly) ios::ChromeBrowserState* mainBrowserState;
// Creates a new SettingsTableViewController and the chrome around it.
// |browserState| is used to personalize some settings aspects and should not be
// |browser| is the browser where settings are being displayed and should not be
// nil nor Off-the-Record. |delegate| may be nil.
+ (SettingsNavigationController*)
newSettingsMainControllerWithBrowserState:(ios::ChromeBrowserState*)browserState
delegate:
(id<SettingsNavigationControllerDelegate>)
+ (instancetype)
mainSettingsControllerForBrowser:(Browser*)browser
delegate:(id<SettingsNavigationControllerDelegate>)
delegate;
// Creates a new AccountsTableViewController and the chrome around it.
// |browserState| is used to personalize some settings aspects and should not be
// |browser| is the browser where settings are being displayed and should not be
// nil. |delegate| may be nil.
+ (SettingsNavigationController*)
newAccountsController:(ios::ChromeBrowserState*)browserState
delegate:(id<SettingsNavigationControllerDelegate>)delegate;
+ (instancetype)
accountsControllerForBrowser:(Browser*)browser
delegate:
(id<SettingsNavigationControllerDelegate>)delegate;
// Creates a new GoogleServicesSettingsCollectionViewController and the chrome
// around it. |browserState| is used to personalize some settings aspects and
// around it. |browser| is the browser where settings are being displayed and
// should not be nil. |delegate| may be nil.
+ (SettingsNavigationController*)
newGoogleServicesController:(ios::ChromeBrowserState*)browserState
+ (instancetype)
googleServicesControllerForBrowser:(Browser*)browser
delegate:
(id<SettingsNavigationControllerDelegate>)delegate;
(id<SettingsNavigationControllerDelegate>)
delegate;
// Creates a new SyncEncryptionPassphraseCollectionViewController and the chrome
// around it. |browserState| is used to personalize some settings aspects and
// around it. |browser| is the browser where settings are being displayed and
// should not be nil. |delegate| may be nil.
+ (SettingsNavigationController*)
newSyncEncryptionPassphraseController:(ios::ChromeBrowserState*)browserState
delegate:(id<SettingsNavigationControllerDelegate>)
+ (instancetype)
syncPassphraseControllerForBrowser:(Browser*)browser
delegate:
(id<SettingsNavigationControllerDelegate>)
delegate;
// Creates a new SavePasswordsCollectionViewController and the chrome around it.
// |browserState| is used to personalize some settings aspects and should not be
// |browser| is the browser where settings are being displayed and should not be
// nil. |delegate| may be nil.
+ (SettingsNavigationController*)
newSavePasswordsController:(ios::ChromeBrowserState*)browserState
delegate:(id<SettingsNavigationControllerDelegate>)delegate;
+ (instancetype)
savePasswordsControllerForBrowser:(Browser*)browser
delegate:(id<SettingsNavigationControllerDelegate>)
delegate;
// Creates and displays a new UIViewController for user to report an issue.
// |browserState| is used to personalize some settings aspects and should not
// be nil. |dataSource| is used to populate the UIViewController. |dispatcher|,
// |browser| is the browser where settings are being displayed and should not be
// nil. |dataSource| is used to populate the UIViewController. |dispatcher|,
// which can be nil, is an object that can perform operations for the view
// controller. |delegate| may be nil.
+ (SettingsNavigationController*)
newUserFeedbackController:(ios::ChromeBrowserState*)browserState
delegate:(id<SettingsNavigationControllerDelegate>)delegate
+ (instancetype)
userFeedbackControllerForBrowser:(Browser*)browser
delegate:(id<SettingsNavigationControllerDelegate>)
delegate
feedbackDataSource:(id<UserFeedbackDataSource>)dataSource
dispatcher:(id<ApplicationCommands>)dispatcher;
// Creates and displays a new ImportDataTableViewController. |browserState|
// Creates and displays a new ImportDataTableViewController. |browser|
// should not be nil.
+ (SettingsNavigationController*)
newImportDataController:(ios::ChromeBrowserState*)browserState
delegate:(id<SettingsNavigationControllerDelegate>)delegate
importDataDelegate:(id<ImportDataControllerDelegate>)importDataDelegate
+ (instancetype)
importDataControllerForBrowserState:(ios::ChromeBrowserState*)browserState
delegate:
(id<SettingsNavigationControllerDelegate>)
delegate
importDataDelegate:
(id<ImportDataControllerDelegate>)importDataDelegate
fromEmail:(NSString*)fromEmail
toEmail:(NSString*)toEmail
isSignedIn:(BOOL)isSignedIn;
// Creates a new AutofillProfileTableViewController and the chrome around
// it. |browserState| is used to personalize some settings aspects and should
// it. |browser| is the browser where settings are being displayed and should
// not be nil. |delegate| may be nil.
+ (SettingsNavigationController*)
newAutofillProfilleController:(ios::ChromeBrowserState*)browserState
+ (instancetype)
autofillProfileControllerForBrowser:(Browser*)browser
delegate:
(id<SettingsNavigationControllerDelegate>)delegate;
(id<SettingsNavigationControllerDelegate>)
delegate;
// Creates a new AutofillCreditCardCollectionViewController and the chrome
// around it. |browserState| is used to personalize some settings aspects and
// around it. |browser| is the browser where settings are being displayed and
// should not be nil. |delegate| may be nil.
+ (SettingsNavigationController*)
newAutofillCreditCardController:(ios::ChromeBrowserState*)browserState
+ (instancetype)
autofillCreditCardControllerForBrowser:(Browser*)browser
delegate:
(id<SettingsNavigationControllerDelegate>)delegate;
(id<SettingsNavigationControllerDelegate>)
delegate;
// Initializes the UINavigationController with |rootViewController|.
- (instancetype)
initWithRootViewController:(UIViewController*)rootViewController
initWithRootViewController:(UIViewController*)rootViewController
browserState:(ios::ChromeBrowserState*)browserState
delegate:(id<SettingsNavigationControllerDelegate>)delegate
delegate:
(id<SettingsNavigationControllerDelegate>)delegate
NS_DESIGNATED_INITIALIZER;
- (instancetype)initWithRootViewController:(UIViewController*)rootViewController
......@@ -150,9 +164,6 @@ initWithRootViewController:(UIViewController*)rootViewController
// owned by SettingsNavigationController.
- (UIBarButtonItem*)doneButton;
// Returns the current main browser state.
- (ios::ChromeBrowserState*)mainBrowserState;
// Notifies this |SettingsNavigationController| of a dismissal such
// that it has a possibility to do necessary clean up.
- (void)cleanUpSettings;
......
......@@ -12,6 +12,7 @@
#include "components/search_engines/template_url_service.h"
#include "ios/chrome/browser/browser_state/test_chrome_browser_state.h"
#include "ios/chrome/browser/browser_state/test_chrome_browser_state_manager.h"
#include "ios/chrome/browser/main/test_browser.h"
#include "ios/chrome/browser/search_engines/template_url_service_factory.h"
#import "ios/chrome/browser/signin/authentication_service.h"
#include "ios/chrome/browser/signin/authentication_service_factory.h"
......@@ -51,6 +52,9 @@ class SettingsNavigationControllerTest : public PlatformTest {
ios::TemplateURLServiceFactory::GetInstance(),
ios::TemplateURLServiceFactory::GetDefaultFactory());
chrome_browser_state_ = test_cbs_builder.Build();
WebStateList* web_state_list = nullptr;
browser_ = std::make_unique<TestBrowser>(chrome_browser_state_.get(),
web_state_list);
mockDelegate_ = [OCMockObject
niceMockForProtocol:@protocol(SettingsNavigationControllerDelegate)];
......@@ -80,6 +84,7 @@ class SettingsNavigationControllerTest : public PlatformTest {
web::WebTaskEnvironment task_environment_;
IOSChromeScopedTestingChromeBrowserStateManager scoped_browser_state_manager_;
std::unique_ptr<TestChromeBrowserState> chrome_browser_state_;
std::unique_ptr<Browser> browser_;
id mockDelegate_;
NSString* initialValueForSpdyProxyEnabled_;
};
......@@ -90,8 +95,7 @@ TEST_F(SettingsNavigationControllerTest, PopController) {
@autoreleasepool {
SettingsNavigationController* settingsController =
[SettingsNavigationController
newSettingsMainControllerWithBrowserState:chrome_browser_state_
.get()
mainSettingsControllerForBrowser:browser_.get()
delegate:nil];
UIViewController* viewController =
[[UIViewController alloc] initWithNibName:nil bundle:nil];
......@@ -112,8 +116,7 @@ TEST_F(SettingsNavigationControllerTest, DontPopRootController) {
@autoreleasepool {
SettingsNavigationController* settingsController =
[SettingsNavigationController
newSettingsMainControllerWithBrowserState:chrome_browser_state_
.get()
mainSettingsControllerForBrowser:browser_.get()
delegate:nil];
EXPECT_EQ(1U, [[settingsController viewControllers] count]);
......@@ -130,8 +133,7 @@ TEST_F(SettingsNavigationControllerTest,
@autoreleasepool {
SettingsNavigationController* settingsController =
[SettingsNavigationController
newSettingsMainControllerWithBrowserState:chrome_browser_state_
.get()
mainSettingsControllerForBrowser:browser_.get()
delegate:mockDelegate_];
UIViewController* viewController =
[[UIViewController alloc] initWithNibName:nil bundle:nil];
......@@ -153,8 +155,7 @@ TEST_F(SettingsNavigationControllerTest,
@autoreleasepool {
SettingsNavigationController* settingsController =
[SettingsNavigationController
newSettingsMainControllerWithBrowserState:chrome_browser_state_
.get()
mainSettingsControllerForBrowser:browser_.get()
delegate:mockDelegate_];
EXPECT_EQ(1U, [[settingsController viewControllers] count]);
[[mockDelegate_ expect] closeSettings];
......
......@@ -9,12 +9,10 @@
#import "ios/chrome/browser/ui/settings/settings_root_table_view_controller.h"
@protocol ApplicationCommands;
class Browser;
@protocol BrowserCommands;
@protocol SettingsMainPageCommands;
@class SigninInteractionController;
namespace ios {
class ChromeBrowserState;
} // namespace ios
// The accessibility identifier of the settings TableView.
extern NSString* const kSettingsTableViewId;
......@@ -40,11 +38,12 @@ extern NSString* const kSettingsVoiceSearchCellId;
@property(weak, nonatomic) id<SettingsMainPageCommands>
settingsMainPageDispatcher;
// Initializes a new SettingsTableViewController. |browserState| must not
// be nil and must not be an off-the-record browser state.
- (instancetype)initWithBrowserState:(ios::ChromeBrowserState*)browserState
dispatcher:(id<ApplicationCommands, BrowserCommands>)
dispatcher NS_DESIGNATED_INITIALIZER;
// Initializes a new SettingsTableViewController. |browser| must not
// be nil and must not be associated with an off the record browser state.
- (instancetype)initWithBrowser:(Browser*)browser
dispatcher:
(id<ApplicationCommands, BrowserCommands>)dispatcher
NS_DESIGNATED_INITIALIZER;
- (instancetype)initWithTableViewStyle:(UITableViewStyle)style
appBarStyle:
......
......@@ -24,6 +24,7 @@
#include "components/sync/driver/sync_service.h"
#include "ios/chrome/browser/application_context.h"
#include "ios/chrome/browser/browser_state/chrome_browser_state.h"
#include "ios/chrome/browser/main/browser.h"
#include "ios/chrome/browser/passwords/ios_chrome_password_store_factory.h"
#include "ios/chrome/browser/pref_names.h"
#include "ios/chrome/browser/search_engines/search_engine_observer_bridge.h"
......@@ -170,7 +171,9 @@ NSString* kDevViewSourceKey = @"DevViewSource";
SigninPresenter,
SigninPromoViewConsumer,
SyncObserverModelBridge> {
// The current browser state that hold the settings. Never off the record.
// The browser where the settings are being displayed.
Browser* _browser;
// The browser state for |_browser|. Never off the record.
ios::ChromeBrowserState* _browserState; // weak
// Bridge for TemplateURLServiceObserver.
std::unique_ptr<SearchEngineObserverBridge> _searchEngineObserverBridge;
......@@ -244,17 +247,19 @@ NSString* kDevViewSourceKey = @"DevViewSource";
#pragma mark Initialization
- (instancetype)initWithBrowserState:(ios::ChromeBrowserState*)browserState
dispatcher:(id<ApplicationCommands, BrowserCommands>)
dispatcher {
DCHECK(!browserState->IsOffTheRecord());
- (instancetype)initWithBrowser:(Browser*)browser
dispatcher:
(id<ApplicationCommands, BrowserCommands>)dispatcher {
DCHECK(browser);
DCHECK(!browser->GetBrowserState()->IsOffTheRecord());
UITableViewStyle style = base::FeatureList::IsEnabled(kSettingsRefresh)
? UITableViewStylePlain
: UITableViewStyleGrouped;
self = [super initWithTableViewStyle:style
appBarStyle:ChromeTableViewControllerStyleNoAppBar];
if (self) {
_browserState = browserState;
_browser = browser;
_browserState = _browser->GetBrowserState();
self.title = l10n_util::GetNSStringWithFixup(IDS_IOS_SETTINGS_TITLE);
_searchEngineObserverBridge.reset(new SearchEngineObserverBridge(
self,
......
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