Revert of Replace TabModel with WebStateList in GoogleLandingController. (patchset #7 id:110001 of https://codereview.chromium.org/2833513002/ )
Reason for revert:
Broke downstream eg tests, needs some minor fixes.
Original issue's description:
> Replace TabModel with WebStateList in GoogleLandingController.
>
> BUG=694750
>
> Review-Url: https://codereview.chromium.org/2833513002
> Cr-Commit-Position: refs/heads/master@{#466542}
> Committed: https://chromium.googlesource.com/chromium/src/+/892edbbf57698e574b6b2b15481239af1c5c4ede
TBR=rohitrao@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=694750
Review-Url: https://codereview.chromium.org/2835033002
Cr-Commit-Position: refs/heads/master@{#466563}
diff --git a/ios/chrome/browser/ui/ntp/BUILD.gn b/ios/chrome/browser/ui/ntp/BUILD.gn
index 2bc28e6..8c223db 100644
--- a/ios/chrome/browser/ui/ntp/BUILD.gn
+++ b/ios/chrome/browser/ui/ntp/BUILD.gn
@@ -187,7 +187,6 @@
"//ios/chrome/browser/ui/overscroll_actions",
"//ios/chrome/browser/ui/toolbar",
"//ios/chrome/browser/ui/toolbar:resource_macros",
- "//ios/chrome/browser/web_state_list",
"//ios/chrome/common",
"//ios/public/provider/chrome/browser",
"//ios/public/provider/chrome/browser/images",
@@ -237,12 +236,8 @@
"//ios/chrome/browser/browser_state:test_support",
"//ios/chrome/browser/search_engines",
"//ios/chrome/browser/sessions",
- "//ios/chrome/browser/sessions:test_support",
- "//ios/chrome/browser/tabs",
"//ios/chrome/browser/ui",
"//ios/chrome/browser/ui/commands",
- "//ios/chrome/browser/web_state_list:test_support",
- "//ios/chrome/browser/web_state_list:web_state_list",
"//ios/chrome/test:test_support",
"//ios/public/provider/chrome/browser/images",
"//ios/web:test_support",
diff --git a/ios/chrome/browser/ui/ntp/google_landing_consumer.h b/ios/chrome/browser/ui/ntp/google_landing_consumer.h
index a3ccc87..9c6b9ea 100644
--- a/ios/chrome/browser/ui/ntp/google_landing_consumer.h
+++ b/ios/chrome/browser/ui/ntp/google_landing_consumer.h
@@ -41,9 +41,6 @@
// |YES| if a what's new promo can be displayed.
- (void)setPromoCanShow:(BOOL)promoCanShow;
-// The number of tabs to show in the google landing fake toolbar.
-- (void)setTabCount:(int)tabCount;
-
// TODO(crbug.com/694750): This should be replaced with consumer suitable data
// type property.
// Tells the consumer to that most visited data updated.
diff --git a/ios/chrome/browser/ui/ntp/google_landing_controller.mm b/ios/chrome/browser/ui/ntp/google_landing_controller.mm
index 6775b12..7f1d3c7 100644
--- a/ios/chrome/browser/ui/ntp/google_landing_controller.mm
+++ b/ios/chrome/browser/ui/ntp/google_landing_controller.mm
@@ -190,9 +190,6 @@
// |YES| if a what's new promo can be displayed.
@property(nonatomic, assign) BOOL promoCanShow;
-// The number of tabs to show in the google landing fake toolbar.
-@property(nonatomic, assign) int tabCount;
-
// iPhone landscape uses a slightly different layout for the doodle and search
// field frame. Returns the proper frame from |frames| based on orientation,
// centered in the view.
@@ -262,7 +259,6 @@
@synthesize promoIcon = _promoIcon;
@synthesize promoCanShow = _promoCanShow;
@synthesize maximumMostVisitedSitesShown = _maximumMostVisitedSitesShown;
-@synthesize tabCount = _tabCount;
@synthesize voiceSearchIsEnabled = _voiceSearchIsEnabled;
- (void)loadView {
@@ -999,7 +995,6 @@
// iPhone header also contains a toolbar since the normal toolbar is
// hidden.
[_headerView addToolbarWithDataSource:self.dataSource];
- [_headerView setToolbarTabCount:self.tabCount];
}
[_supplementaryViews addObject:_headerView];
}
@@ -1489,9 +1484,4 @@
[_mostVisitedView reloadItemsAtIndexPaths:@[ indexPath ]];
}
-- (void)setTabCount:(int)tabCount {
- _tabCount = tabCount;
- [_headerView setToolbarTabCount:self.tabCount];
-}
-
@end
diff --git a/ios/chrome/browser/ui/ntp/google_landing_controller_unittest.mm b/ios/chrome/browser/ui/ntp/google_landing_controller_unittest.mm
index b34dce3..3a711ff 100644
--- a/ios/chrome/browser/ui/ntp/google_landing_controller_unittest.mm
+++ b/ios/chrome/browser/ui/ntp/google_landing_controller_unittest.mm
@@ -12,8 +12,6 @@
#include "ios/chrome/browser/sessions/ios_chrome_tab_restore_service_factory.h"
#import "ios/chrome/browser/ui/ntp/google_landing_controller.h"
#import "ios/chrome/browser/ui/ntp/google_landing_mediator.h"
-#include "ios/chrome/browser/web_state_list/fake_web_state_list_delegate.h"
-#include "ios/chrome/browser/web_state_list/web_state_list.h"
#include "ios/chrome/test/block_cleanup_test.h"
#include "ios/chrome/test/ios_chrome_scoped_testing_local_state.h"
#include "ios/web/public/test/test_web_thread.h"
@@ -57,14 +55,13 @@
// Set up stub UrlLoader.
mockUrlLoader_ = [OCMockObject mockForProtocol:@protocol(UrlLoader)];
controller_ = [[GoogleLandingController alloc] init];
- webStateList_ = base::MakeUnique<WebStateList>(&webStateListDelegate_);
mediator_ = [[GoogleLandingMediator alloc]
initWithConsumer:controller_
browserState:chrome_browser_state_.get()
loader:(id<UrlLoader>)mockUrlLoader_
focuser:nil
webToolbarDelegate:nil
- webStateList:webStateList_.get()];
+ tabModel:nil];
};
base::MessageLoopForUI message_loop_;
@@ -72,8 +69,6 @@
web::TestWebThread io_thread_;
IOSChromeScopedTestingLocalState local_state_;
std::unique_ptr<TestChromeBrowserState> chrome_browser_state_;
- FakeWebStateListDelegate webStateListDelegate_;
- std::unique_ptr<WebStateList> webStateList_;
OCMockObject* mockUrlLoader_;
GoogleLandingMediator* mediator_;
GoogleLandingController* controller_;
diff --git a/ios/chrome/browser/ui/ntp/google_landing_data_source.h b/ios/chrome/browser/ui/ntp/google_landing_data_source.h
index b8621d1..48e78e7e7 100644
--- a/ios/chrome/browser/ui/ntp/google_landing_data_source.h
+++ b/ios/chrome/browser/ui/ntp/google_landing_data_source.h
@@ -65,6 +65,8 @@
// Gets the toolbar delegate.
- (id<WebToolbarDelegate>)toolbarDelegate;
+// Gets the tab model.
+- (TabModel*)tabModel;
@end
diff --git a/ios/chrome/browser/ui/ntp/google_landing_mediator.h b/ios/chrome/browser/ui/ntp/google_landing_mediator.h
index 6bfcfae..95112f9 100644
--- a/ios/chrome/browser/ui/ntp/google_landing_mediator.h
+++ b/ios/chrome/browser/ui/ntp/google_landing_mediator.h
@@ -12,7 +12,6 @@
@protocol GoogleLandingConsumer;
@protocol OmniboxFocuser;
@protocol UrlLoader;
-class WebStateList;
namespace ios {
class ChromeBrowserState;
@@ -26,8 +25,7 @@
loader:(id<UrlLoader>)loader
focuser:(id<OmniboxFocuser>)focuser
webToolbarDelegate:(id<WebToolbarDelegate>)webToolbarDelegate
- webStateList:(WebStateList*)webStateList
- NS_DESIGNATED_INITIALIZER;
+ tabModel:(TabModel*)tabModel NS_DESIGNATED_INITIALIZER;
- (instancetype)init NS_UNAVAILABLE;
// Get the maximum number of sites shown.
diff --git a/ios/chrome/browser/ui/ntp/google_landing_mediator.mm b/ios/chrome/browser/ui/ntp/google_landing_mediator.mm
index 0d5ce31..aa41b9e 100644
--- a/ios/chrome/browser/ui/ntp/google_landing_mediator.mm
+++ b/ios/chrome/browser/ui/ntp/google_landing_mediator.mm
@@ -31,8 +31,6 @@
#import "ios/chrome/browser/ui/ntp/notification_promo_whats_new.h"
#import "ios/chrome/browser/ui/toolbar/web_toolbar_controller.h"
#import "ios/chrome/browser/ui/url_loader.h"
-#import "ios/chrome/browser/web_state_list/web_state_list.h"
-#import "ios/chrome/browser/web_state_list/web_state_list_observer_bridge.h"
#include "ios/public/provider/chrome/browser/chrome_browser_provider.h"
#include "ios/public/provider/chrome/browser/voice/voice_search_provider.h"
@@ -81,8 +79,7 @@
} // namespace google_landing
-@interface GoogleLandingMediator ()<MostVisitedSitesObserving,
- WebStateListObserving> {
+@interface GoogleLandingMediator ()<MostVisitedSitesObserving> {
// The ChromeBrowserState associated with this mediator.
ios::ChromeBrowserState* _browserState; // Weak.
@@ -115,9 +112,7 @@
base::WeakNSProtocol<id<WebToolbarDelegate>> _webToolbarDelegate;
- // Observes the WebStateList so that this mediator can update the UI when the
- // active WebState changes.
- std::unique_ptr<WebStateListObserverBridge> _webStateListObserver;
+ base::scoped_nsobject<TabModel> _tabModel;
// What's new promo.
std::unique_ptr<NotificationPromoWhatsNew> _notification_promo;
@@ -126,9 +121,6 @@
// Consumer to handle google landing update notifications.
@property(nonatomic) id<GoogleLandingConsumer> consumer;
-// The WebStateList that is being observed by this mediator.
-@property(nonatomic, assign) WebStateList* webStateList;
-
// Perform initial setup.
- (void)setUp;
@@ -137,14 +129,13 @@
@implementation GoogleLandingMediator
@synthesize consumer = _consumer;
-@synthesize webStateList = _webStateList;
- (instancetype)initWithConsumer:(id<GoogleLandingConsumer>)consumer
browserState:(ios::ChromeBrowserState*)browserState
loader:(id<UrlLoader>)loader
focuser:(id<OmniboxFocuser>)focuser
webToolbarDelegate:(id<WebToolbarDelegate>)webToolbarDelegate
- webStateList:(WebStateList*)webStateList {
+ tabModel:(TabModel*)tabModel {
self = [super init];
if (self) {
_consumer = consumer;
@@ -152,18 +143,13 @@
_loader = loader;
_focuser.reset(focuser);
_webToolbarDelegate.reset(webToolbarDelegate);
- _webStateList = webStateList;
-
- _webStateListObserver = base::MakeUnique<WebStateListObserverBridge>(self);
- _webStateList->AddObserver(_webStateListObserver.get());
-
+ _tabModel.reset([tabModel retain]);
[self setUp];
}
return self;
}
- (void)dealloc {
- _webStateList->RemoveObserver(_webStateListObserver.get());
[[NSNotificationCenter defaultCenter] removeObserver:self.consumer];
[super dealloc];
}
@@ -175,7 +161,6 @@
->IsVoiceSearchEnabled()];
[_consumer
setMaximumMostVisitedSitesShown:[GoogleLandingMediator maxSitesShown]];
- [_consumer setTabCount:self.webStateList->count()];
// Set up template URL service to listen for default search engine changes.
_templateURLService =
@@ -265,20 +250,6 @@
}
}
-#pragma mark - WebStateListObserving
-
-- (void)webStateList:(WebStateList*)webStateList
- didInsertWebState:(web::WebState*)webState
- atIndex:(int)index {
- [self.consumer setTabCount:self.webStateList->count()];
-}
-
-- (void)webStateList:(WebStateList*)webStateList
- didDetachWebState:(web::WebState*)webState
- atIndex:(int)atIndex {
- [self.consumer setTabCount:self.webStateList->count()];
-}
-
#pragma mark - GoogleLandingDataSource
- (void)addBlacklistedURL:(const GURL&)url {
@@ -322,6 +293,10 @@
return _webToolbarDelegate;
}
+- (TabModel*)tabModel {
+ return _tabModel;
+}
+
- (void)promoViewed {
DCHECK(_notification_promo);
_notification_promo->HandleViewed();
diff --git a/ios/chrome/browser/ui/ntp/new_tab_page_controller.mm b/ios/chrome/browser/ui/ntp/new_tab_page_controller.mm
index 691cb7d..e7a91792 100644
--- a/ios/chrome/browser/ui/ntp/new_tab_page_controller.mm
+++ b/ios/chrome/browser/ui/ntp/new_tab_page_controller.mm
@@ -543,7 +543,7 @@
loader:loader_
focuser:focuser_
webToolbarDelegate:webToolbarDelegate_
- webStateList:[tabModel_ webStateList]]);
+ tabModel:tabModel_]);
[googleLandingController_ setDataSource:googleLandingMediator_];
}
panelController = googleLandingController_;
diff --git a/ios/chrome/browser/ui/ntp/new_tab_page_controller_unittest.mm b/ios/chrome/browser/ui/ntp/new_tab_page_controller_unittest.mm
index 0c54bfe5..1bae95d 100644
--- a/ios/chrome/browser/ui/ntp/new_tab_page_controller_unittest.mm
+++ b/ios/chrome/browser/ui/ntp/new_tab_page_controller_unittest.mm
@@ -17,8 +17,6 @@
#include "ios/chrome/browser/chrome_url_constants.h"
#include "ios/chrome/browser/search_engines/template_url_service_factory.h"
#include "ios/chrome/browser/sessions/ios_chrome_tab_restore_service_factory.h"
-#import "ios/chrome/browser/sessions/test_session_service.h"
-#import "ios/chrome/browser/tabs/tab_model.h"
#import "ios/chrome/browser/ui/ntp/new_tab_page_view.h"
#include "ios/chrome/browser/ui/ui_util.h"
#include "ios/chrome/test/block_cleanup_test.h"
@@ -89,10 +87,6 @@
chrome_browser_state_.get()));
GURL url(kChromeUINewTabURL);
parentViewController_ = [[UIViewController alloc] init];
- tabModel_ = [[TabModel alloc]
- initWithSessionWindow:nil
- sessionService:[[TestSessionService alloc] init]
- browserState:chrome_browser_state_.get()];
controller_ =
[[NewTabPageController alloc] initWithUrl:url
loader:nil
@@ -101,7 +95,7 @@
browserState:chrome_browser_state_.get()
colorCache:nil
webToolbarDelegate:nil
- tabModel:tabModel_
+ tabModel:nil
parentViewController:parentViewController_];
incognitoController_ = [[NewTabPageController alloc]
@@ -121,8 +115,6 @@
incognitoController_ = nil;
controller_ = nil;
parentViewController_ = nil;
- [tabModel_ browserStateDestroyed];
- tabModel_ = nil;
// There may be blocks released below that have weak references to |profile|
// owned by chrome_browser_state_. Ensure BlockCleanupTest::TearDown() is
@@ -134,7 +126,6 @@
web::TestWebThreadBundle thread_bundle_;
IOSChromeScopedTestingLocalState local_state_;
std::unique_ptr<TestChromeBrowserState> chrome_browser_state_;
- TabModel* tabModel_;
UIViewController* parentViewController_;
NewTabPageController* controller_;
NewTabPageController* incognitoController_;
diff --git a/ios/chrome/browser/ui/ntp/new_tab_page_header_view.h b/ios/chrome/browser/ui/ntp/new_tab_page_header_view.h
index e4c9354..b7a2fd7 100644
--- a/ios/chrome/browser/ui/ntp/new_tab_page_header_view.h
+++ b/ios/chrome/browser/ui/ntp/new_tab_page_header_view.h
@@ -40,8 +40,6 @@
// Hide toolbar subviews that should not be displayed on the new tab page.
- (void)hideToolbarViewsForNewTabPage;
-// Updates the toolbar tab count;
-- (void)setToolbarTabCount:(int)tabCount;
@end
#endif // IOS_CHROME_BROWSER_UI_NTP_NEW_TAB_PAGE_HEADER_VIEW_H_
diff --git a/ios/chrome/browser/ui/ntp/new_tab_page_header_view.mm b/ios/chrome/browser/ui/ntp/new_tab_page_header_view.mm
index a9a5194..4659015 100644
--- a/ios/chrome/browser/ui/ntp/new_tab_page_header_view.mm
+++ b/ios/chrome/browser/ui/ntp/new_tab_page_header_view.mm
@@ -25,8 +25,9 @@
} // namespace
-@interface NewTabPageHeaderView () {
+@interface NewTabPageHeaderView ()<TabModelObserver> {
base::scoped_nsobject<NewTabPageToolbarController> _toolbarController;
+ base::scoped_nsobject<TabModel> _tabModel;
base::scoped_nsobject<UIImageView> _searchBoxBorder;
base::scoped_nsobject<UIImageView> _shadow;
}
@@ -44,6 +45,7 @@
}
- (void)dealloc {
+ [_tabModel removeObserver:self];
[super dealloc];
}
@@ -72,6 +74,9 @@
initWithToolbarDelegate:[dataSource toolbarDelegate]
focuser:dataSource]);
_toolbarController.get().readingListModel = [dataSource readingListModel];
+ [_tabModel removeObserver:self];
+ _tabModel.reset([[dataSource tabModel] retain]);
+ [self addTabModelObserver];
UIView* toolbarView = [_toolbarController view];
CGRect toolbarFrame = self.bounds;
@@ -88,8 +93,9 @@
[_toolbarController hideViewsForNewTabPage:YES];
};
-- (void)setToolbarTabCount:(int)tabCount {
- [_toolbarController setTabCount:tabCount];
+- (void)addTabModelObserver {
+ [_tabModel addObserver:self];
+ [_toolbarController setTabCount:[_tabModel count]];
}
- (void)addViewsToSearchField:(UIView*)searchField {
@@ -116,6 +122,11 @@
[_shadow setAlpha:0];
}
+- (void)tabModelDidChangeTabCount:(TabModel*)model {
+ DCHECK(model == _tabModel);
+ [_toolbarController setTabCount:[_tabModel count]];
+}
+
- (void)updateSearchField:(UIView*)searchField
withInitialFrame:(CGRect)initialFrame
subviewConstraints:(NSArray*)constraints
diff --git a/ios/clean/chrome/browser/ui/ntp/ntp_home_coordinator.mm b/ios/clean/chrome/browser/ui/ntp/ntp_home_coordinator.mm
index 278b6378..352000bf 100644
--- a/ios/clean/chrome/browser/ui/ntp/ntp_home_coordinator.mm
+++ b/ios/clean/chrome/browser/ui/ntp/ntp_home_coordinator.mm
@@ -36,7 +36,7 @@
loader:self.mediator
focuser:self.mediator
webToolbarDelegate:nil
- webStateList:&self.browser->web_state_list()];
+ tabModel:nil];
self.viewController.dataSource = self.googleLandingMediator;
[super start];
}
