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

[iOS] Plumb Browser into RecentTabsViewController

Changes RecentTabsViewController to accept a Browser instead of separate
WebStateList and BrowserState inputs. This will make future refactoring
more straightforward.

(There remains a great deal of work to do to fully refactor Recent Tabs
to properly factor responsibilities into a mediator and coordinator).

Bug: 845192
Change-Id: I068b0a343752dba602d2a2226cd0bcf327873446
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2078452
Commit-Queue: Mark Cogan <marq@chromium.org>
Reviewed-by: default avatarGauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#745456}
parent 63cc1af7
...@@ -71,6 +71,7 @@ source_set("recent_tabs_ui") { ...@@ -71,6 +71,7 @@ source_set("recent_tabs_ui") {
"//components/sync", "//components/sync",
"//ios/chrome/app/strings", "//ios/chrome/app/strings",
"//ios/chrome/browser/browser_state", "//ios/chrome/browser/browser_state",
"//ios/chrome/browser/main:public",
"//ios/chrome/browser/metrics:metrics_internal", "//ios/chrome/browser/metrics:metrics_internal",
"//ios/chrome/browser/sessions", "//ios/chrome/browser/sessions",
"//ios/chrome/browser/sessions:serialisation", "//ios/chrome/browser/sessions:serialisation",
......
...@@ -47,14 +47,13 @@ ...@@ -47,14 +47,13 @@
// Initialize and configure RecentTabsTableViewController. // Initialize and configure RecentTabsTableViewController.
RecentTabsTableViewController* recentTabsTableViewController = RecentTabsTableViewController* recentTabsTableViewController =
[[RecentTabsTableViewController alloc] init]; [[RecentTabsTableViewController alloc] init];
recentTabsTableViewController.browserState = self.browserState; recentTabsTableViewController.browser = self.browser;
recentTabsTableViewController.loadStrategy = self.loadStrategy; recentTabsTableViewController.loadStrategy = self.loadStrategy;
CommandDispatcher* dispatcher = self.browser->GetCommandDispatcher(); CommandDispatcher* dispatcher = self.browser->GetCommandDispatcher();
id<ApplicationCommands> handler = id<ApplicationCommands> handler =
HandlerForProtocol(dispatcher, ApplicationCommands); HandlerForProtocol(dispatcher, ApplicationCommands);
recentTabsTableViewController.handler = handler; recentTabsTableViewController.handler = handler;
recentTabsTableViewController.presentationDelegate = self; recentTabsTableViewController.presentationDelegate = self;
recentTabsTableViewController.webStateList = self.browser->GetWebStateList();
// Adds the "Done" button and hooks it up to |stop|. // Adds the "Done" button and hooks it up to |stop|.
UIBarButtonItem* dismissButton = [[UIBarButtonItem alloc] UIBarButtonItem* dismissButton = [[UIBarButtonItem alloc]
......
...@@ -9,9 +9,8 @@ ...@@ -9,9 +9,8 @@
#import "ios/chrome/browser/ui/table_view/chrome_table_view_controller.h" #import "ios/chrome/browser/ui/table_view/chrome_table_view_controller.h"
#include "ui/base/window_open_disposition.h" #include "ui/base/window_open_disposition.h"
class ChromeBrowserState; class Browser;
enum class UrlLoadStrategy; enum class UrlLoadStrategy;
class WebStateList;
@protocol ApplicationCommands; @protocol ApplicationCommands;
@protocol RecentTabsTableViewControllerDelegate; @protocol RecentTabsTableViewControllerDelegate;
...@@ -21,8 +20,9 @@ class WebStateList; ...@@ -21,8 +20,9 @@ class WebStateList;
@interface RecentTabsTableViewController @interface RecentTabsTableViewController
: ChromeTableViewController <RecentTabsConsumer, : ChromeTableViewController <RecentTabsConsumer,
UIAdaptivePresentationControllerDelegate> UIAdaptivePresentationControllerDelegate>
// The coordinator's BrowserState. // The Browser for the tabs being restored. It's an error to pass a nullptr
@property(nonatomic, assign) ChromeBrowserState* browserState; // Browser.
@property(nonatomic, assign) Browser* browser;
// The command handler used by this ViewController. // The command handler used by this ViewController.
@property(nonatomic, weak) id<ApplicationCommands> handler; @property(nonatomic, weak) id<ApplicationCommands> handler;
// Opaque instructions on how to open urls. // Opaque instructions on how to open urls.
...@@ -31,8 +31,6 @@ class WebStateList; ...@@ -31,8 +31,6 @@ class WebStateList;
@property(nonatomic, assign) WindowOpenDisposition restoredTabDisposition; @property(nonatomic, assign) WindowOpenDisposition restoredTabDisposition;
// RecentTabsTableViewControllerDelegate delegate. // RecentTabsTableViewControllerDelegate delegate.
@property(nonatomic, weak) id<RecentTabsTableViewControllerDelegate> delegate; @property(nonatomic, weak) id<RecentTabsTableViewControllerDelegate> delegate;
// WebStateList for tabs restored by this object.
@property(nonatomic, assign) WebStateList* webStateList;
// Whether the updates of the RecentTabs should be ignored. Setting this to NO // Whether the updates of the RecentTabs should be ignored. Setting this to NO
// would trigger a reload of the TableView. // would trigger a reload of the TableView.
@property(nonatomic, assign) BOOL preventUpdates; @property(nonatomic, assign) BOOL preventUpdates;
......
...@@ -16,6 +16,7 @@ ...@@ -16,6 +16,7 @@
#include "components/sync_sessions/open_tabs_ui_delegate.h" #include "components/sync_sessions/open_tabs_ui_delegate.h"
#include "components/sync_sessions/session_sync_service.h" #include "components/sync_sessions/session_sync_service.h"
#include "ios/chrome/browser/browser_state/chrome_browser_state.h" #include "ios/chrome/browser/browser_state/chrome_browser_state.h"
#import "ios/chrome/browser/main/browser.h"
#import "ios/chrome/browser/metrics/new_tab_page_uma.h" #import "ios/chrome/browser/metrics/new_tab_page_uma.h"
#include "ios/chrome/browser/sessions/session_util.h" #include "ios/chrome/browser/sessions/session_util.h"
#include "ios/chrome/browser/sessions/tab_restore_service_delegate_impl_ios.h" #include "ios/chrome/browser/sessions/tab_restore_service_delegate_impl_ios.h"
...@@ -117,8 +118,13 @@ const int kRecentlyClosedTabsSectionIndex = 0; ...@@ -117,8 +118,13 @@ const int kRecentlyClosedTabsSectionIndex = 0;
// Handles displaying the context menu for all form factors. // Handles displaying the context menu for all form factors.
@property(nonatomic, strong) ContextMenuCoordinator* contextMenuCoordinator; @property(nonatomic, strong) ContextMenuCoordinator* contextMenuCoordinator;
@property(nonatomic, strong) SigninPromoViewMediator* signinPromoViewMediator; @property(nonatomic, strong) SigninPromoViewMediator* signinPromoViewMediator;
// The browser state used for many operations, derived from the one provided by
// |self.browser|.
@property(nonatomic, readonly) ChromeBrowserState* browserState;
// YES if this ViewController is being presented on incognito mode. // YES if this ViewController is being presented on incognito mode.
@property(nonatomic, assign, getter=isIncognito) BOOL incognito; @property(nonatomic, readonly, getter=isIncognito) BOOL incognito;
// Convenience getter for |self.browser|'s WebStateList
@property(nonatomic, readonly) WebStateList* webStateList;
@end @end
@implementation RecentTabsTableViewController : ChromeTableViewController @implementation RecentTabsTableViewController : ChromeTableViewController
...@@ -174,15 +180,20 @@ const int kRecentlyClosedTabsSectionIndex = 0; ...@@ -174,15 +180,20 @@ const int kRecentlyClosedTabsSectionIndex = 0;
#pragma mark - Setters & Getters #pragma mark - Setters & Getters
// Some RecentTabs services depend on objects not present in the OffTheRecord - (void)setBrowser:(Browser*)browser {
// BrowserState, in order to prevent crashes set |_browserState| to DCHECK(browser);
// |browserState|->OriginalChromeBrowserState. While doing this check if _browser = browser;
// incognito or not so that pages are loaded accordingly. ChromeBrowserState* browserState = browser->GetBrowserState();
- (void)setBrowserState:(ChromeBrowserState*)browserState { // Some RecentTabs services depend on objects not present in the OffTheRecord
if (browserState) { // BrowserState, in order to prevent crashes set |_browserState| to
_browserState = browserState->GetOriginalChromeBrowserState(); // |browserState|->OriginalChromeBrowserState. While doing this check if
_incognito = browserState->IsOffTheRecord(); // incognito or not so that pages are loaded accordingly.
} _browserState = browserState->GetOriginalChromeBrowserState();
_incognito = browserState->IsOffTheRecord();
}
- (WebStateList*)webStateList {
return self.browser->GetWebStateList();
} }
- (void)setPreventUpdates:(BOOL)preventUpdates { - (void)setPreventUpdates:(BOOL)preventUpdates {
......
...@@ -182,8 +182,7 @@ ...@@ -182,8 +182,7 @@
// TODO(crbug.com/845192) : Remove RecentTabsTableViewController dependency on // TODO(crbug.com/845192) : Remove RecentTabsTableViewController dependency on
// ChromeBrowserState so that we don't need to expose the view controller. // ChromeBrowserState so that we don't need to expose the view controller.
baseViewController.remoteTabsViewController.browserState = baseViewController.remoteTabsViewController.browser = self.regularBrowser;
regularBrowserState;
self.remoteTabsMediator = [[RecentTabsMediator alloc] init]; self.remoteTabsMediator = [[RecentTabsMediator alloc] init];
self.remoteTabsMediator.browserState = regularBrowserState; self.remoteTabsMediator.browserState = regularBrowserState;
self.remoteTabsMediator.consumer = baseViewController.remoteTabsConsumer; self.remoteTabsMediator.consumer = baseViewController.remoteTabsConsumer;
...@@ -201,8 +200,6 @@ ...@@ -201,8 +200,6 @@
UrlLoadStrategy::ALWAYS_NEW_FOREGROUND_TAB; UrlLoadStrategy::ALWAYS_NEW_FOREGROUND_TAB;
baseViewController.remoteTabsViewController.restoredTabDisposition = baseViewController.remoteTabsViewController.restoredTabDisposition =
WindowOpenDisposition::NEW_FOREGROUND_TAB; WindowOpenDisposition::NEW_FOREGROUND_TAB;
baseViewController.remoteTabsViewController.webStateList =
regularWebStateList;
baseViewController.remoteTabsViewController.presentationDelegate = self; baseViewController.remoteTabsViewController.presentationDelegate = self;
if (!base::FeatureList::IsEnabled(kContainedBVC)) { if (!base::FeatureList::IsEnabled(kContainedBVC)) {
......
...@@ -7,9 +7,11 @@ ...@@ -7,9 +7,11 @@
#import <UIKit/UIKit.h> #import <UIKit/UIKit.h>
#import "base/test/ios/wait_util.h" #import "base/test/ios/wait_util.h"
#import "ios/chrome/browser/main/test_browser.h"
#import "ios/chrome/browser/ui/commands/browsing_data_commands.h" #import "ios/chrome/browser/ui/commands/browsing_data_commands.h"
#import "ios/chrome/browser/ui/tab_grid/tab_switcher.h" #import "ios/chrome/browser/ui/tab_grid/tab_switcher.h"
#import "ios/chrome/test/block_cleanup_test.h" #import "ios/chrome/test/block_cleanup_test.h"
#include "ios/web/public/test/web_task_environment.h"
#include "testing/gtest_mac.h" #include "testing/gtest_mac.h"
#include "third_party/ocmock/OCMock/OCMock.h" #include "third_party/ocmock/OCMock/OCMock.h"
...@@ -39,6 +41,7 @@ namespace { ...@@ -39,6 +41,7 @@ namespace {
class TabGridCoordinatorTest : public BlockCleanupTest { class TabGridCoordinatorTest : public BlockCleanupTest {
public: public:
TabGridCoordinatorTest() { TabGridCoordinatorTest() {
browser_ = std::make_unique<TestBrowser>();
UIWindow* window = [UIApplication sharedApplication].keyWindow; UIWindow* window = [UIApplication sharedApplication].keyWindow;
coordinator_ = [[TabGridCoordinator alloc] coordinator_ = [[TabGridCoordinator alloc]
initWithWindow:window initWithWindow:window
...@@ -47,6 +50,7 @@ class TabGridCoordinatorTest : public BlockCleanupTest { ...@@ -47,6 +50,7 @@ class TabGridCoordinatorTest : public BlockCleanupTest {
browsingDataCommandEndpoint:OCMProtocolMock( browsingDataCommandEndpoint:OCMProtocolMock(
@protocol(BrowsingDataCommands))]; @protocol(BrowsingDataCommands))];
coordinator_.animationsDisabledForTesting = YES; coordinator_.animationsDisabledForTesting = YES;
coordinator_.regularBrowser = browser_.get();
// TabGirdCoordinator will make its view controller the root, so stash the // TabGirdCoordinator will make its view controller the root, so stash the
// original root view controller before starting |coordinator_|. // original root view controller before starting |coordinator_|.
original_root_view_controller_ = original_root_view_controller_ =
...@@ -72,9 +76,14 @@ class TabGridCoordinatorTest : public BlockCleanupTest { ...@@ -72,9 +76,14 @@ class TabGridCoordinatorTest : public BlockCleanupTest {
original_root_view_controller_; original_root_view_controller_;
original_root_view_controller_ = nil; original_root_view_controller_ = nil;
} }
[coordinator_ stop];
} }
protected: protected:
web::WebTaskEnvironment task_environment_;
// Browser for the coordinator.
std::unique_ptr<Browser> browser_;
// The TabGridCoordinator that is under test. The test fixture sets // The TabGridCoordinator that is under test. The test fixture sets
// this VC as the root VC for the window. // this VC as the root VC for the window.
TabGridCoordinator* coordinator_; TabGridCoordinator* coordinator_;
...@@ -206,7 +215,6 @@ TEST_F(TabGridCoordinatorTest, CompletionHandlers) { ...@@ -206,7 +215,6 @@ TEST_F(TabGridCoordinatorTest, CompletionHandlers) {
// Test that the tab grid coordinator sizes its view controller to the window. // Test that the tab grid coordinator sizes its view controller to the window.
TEST_F(TabGridCoordinatorTest, SizeTabGridCoordinatorViewController) { TEST_F(TabGridCoordinatorTest, SizeTabGridCoordinatorViewController) {
CGRect rect = [UIScreen mainScreen].bounds; CGRect rect = [UIScreen mainScreen].bounds;
[coordinator_ start];
EXPECT_TRUE( EXPECT_TRUE(
CGRectEqualToRect(rect, coordinator_.baseViewController.view.frame)); CGRectEqualToRect(rect, coordinator_.baseViewController.view.frame));
} }
......
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