Commit 81fcf533 authored by edchin's avatar edchin Committed by Commit Bot

[ios] Make history work from recent tabs in tab grid

A history coordinator that is started from main_controller won't work properly
from the tab grid. Using a local coordinator works better when hooked up with a
specialized URL loader and tab presentation delegate.

This CL introduces a specialized URL loader that behaves differently from the
one in BVC. Essentially, BVC's loader navigates the current WebState to the
new URL. In tab grid, we need to append a new WebState to the end of the list,
because there is no context of a current visible BVC.

Also in this CL is a change to how tab presentation is handled.
In the stand-alone recent tabs, the BVC is already presented so there is no
need to actively present it. However, in tab grid, there is no visible BVC.
To show the tab that was tapped in recent tabs, we needed a mechanism that
could present BVC when in the tab grid, but NO-OP in the stand-alone case.

Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: Id3b8a88e548de14356f997a8154fed9d613b18d0
Reviewed-on: https://chromium-review.googlesource.com/1092510
Commit-Queue: edchin <edchin@chromium.org>
Reviewed-by: default avataredchin <edchin@chromium.org>
Reviewed-by: default avatarMark Cogan <marq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565697}
parent 04bf4e52
......@@ -108,6 +108,7 @@ source_set("history_ui") {
"history_util.mm",
]
deps = [
"public",
"//base",
"//base:i18n",
"//components/browsing_data/core",
......
......@@ -12,6 +12,7 @@
@protocol ApplicationCommands;
@protocol BrowserCommands;
@protocol UrlLoader;
@protocol HistoryTabPresentationDelegate;
// Coordinator that presents History.
@interface HistoryCoordinator : ChromeCoordinator
......@@ -19,6 +20,9 @@
@property(nonatomic, weak) id<ApplicationCommands, BrowserCommands> dispatcher;
// URL loader being managed by this Coordinator.
@property(nonatomic, weak) id<UrlLoader> loader;
// Delegate used to make the Tab UI visible.
@property(nonatomic, weak) id<HistoryTabPresentationDelegate>
tabPresentationDelegate;
@end
#endif // IOS_CHROME_BROWSER_UI_HISTORY_HISTORY_COORDINATOR_H_
......@@ -49,6 +49,7 @@
@synthesize historyTransitioningDelegate = _historyTransitioningDelegate;
@synthesize loader = _loader;
@synthesize clearBrowsingDataCoordinator = _clearBrowsingDataCoordinator;
@synthesize tabPresentationDelegate = _tabPresentationDelegate;
- (void)start {
// Initialize and configure HistoryTableViewController.
......@@ -72,6 +73,8 @@
initWithTable:historyTableViewController];
self.historyNavigationController.toolbarHidden = NO;
historyTableViewController.localDispatcher = self;
historyTableViewController.tabPresentationDelegate =
self.tabPresentationDelegate;
self.historyTransitioningDelegate =
[[HistoryTransitioningDelegate alloc] init];
self.historyNavigationController.transitioningDelegate =
......
......@@ -14,6 +14,7 @@ class ChromeBrowserState;
}
@protocol HistoryLocalCommands;
@protocol HistoryTabPresentationDelegate;
@protocol UrlLoader;
// ChromeTableViewController for displaying history items.
......@@ -28,6 +29,9 @@ class ChromeBrowserState;
@property(nonatomic, weak) id<UrlLoader> loader;
// Delegate for this HistoryTableView.
@property(nonatomic, weak) id<HistoryLocalCommands> localDispatcher;
// Delegate used to make the Tab UI visible.
@property(nonatomic, weak) id<HistoryTabPresentationDelegate>
tabPresentationDelegate;
// Initializers.
- (instancetype)init NS_DESIGNATED_INITIALIZER;
......
......@@ -23,6 +23,7 @@
#include "ios/chrome/browser/ui/history/history_local_commands.h"
#import "ios/chrome/browser/ui/history/history_ui_constants.h"
#include "ios/chrome/browser/ui/history/history_util.h"
#import "ios/chrome/browser/ui/history/public/history_tab_presentation_delegate.h"
#import "ios/chrome/browser/ui/table_view/cells/table_view_cells_constants.h"
#import "ios/chrome/browser/ui/table_view/cells/table_view_text_item.h"
#import "ios/chrome/browser/ui/table_view/table_view_navigation_controller_constants.h"
......@@ -110,6 +111,7 @@ const CGFloat kSeparationSpaceBetweenSections = 9;
@synthesize searchController = _searchController;
@synthesize shouldShowNoticeAboutOtherFormsOfBrowsingHistory =
_shouldShowNoticeAboutOtherFormsOfBrowsingHistory;
@synthesize tabPresentationDelegate = _tabPresentationDelegate;
#pragma mark - ViewController Lifecycle.
......@@ -811,6 +813,7 @@ const CGFloat kSeparationSpaceBetweenSections = 9;
inIncognito:NO
inBackground:NO
appendTo:kLastTab];
[self.tabPresentationDelegate showActiveRegularTab];
}];
}
......@@ -823,6 +826,7 @@ const CGFloat kSeparationSpaceBetweenSections = 9;
inIncognito:YES
inBackground:NO
appendTo:kLastTab];
[self.tabPresentationDelegate showActiveIncognitoTab];
}];
}
......@@ -836,6 +840,7 @@ const CGFloat kSeparationSpaceBetweenSections = 9;
params.transition_type = ui::PAGE_TRANSITION_AUTO_BOOKMARK;
[self.localDispatcher dismissHistoryWithCompletion:^{
[self.loader loadURLWithParams:params];
[self.tabPresentationDelegate showActiveRegularTab];
}];
}
......
# 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.
source_set("public") {
configs += [ "//build/config/compiler:enable_arc" ]
sources = [
"history_tab_presentation_delegate.h",
]
}
// 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_HISTORY_PUBLIC_HISTORY_TAB_PRESENTATION_DELEGATE_H_
#define IOS_CHROME_BROWSER_UI_HISTORY_PUBLIC_HISTORY_TAB_PRESENTATION_DELEGATE_H_
// Delegate used to make the tab UI visible.
@protocol HistoryTabPresentationDelegate
// Tells the delegate to show the non-incognito tab UI. NO-OP if the correct tab
// UI is already visible.
- (void)showActiveRegularTab;
// Tells the delegate to show the incognito tab UI. NO-OP if the correct tab UI
// is already visible.
- (void)showActiveIncognitoTab;
@end
#endif // IOS_CHROME_BROWSER_UI_HISTORY_PUBLIC_HISTORY_TAB_PRESENTATION_DELEGATE_H_
......@@ -12,6 +12,8 @@ source_set("tab_grid") {
"tab_grid_coordinator.mm",
"tab_grid_mediator.h",
"tab_grid_mediator.mm",
"tab_grid_url_loader.h",
"tab_grid_url_loader.mm",
]
configs += [ "//build/config/compiler:enable_arc" ]
......@@ -29,6 +31,8 @@ source_set("tab_grid") {
"//ios/chrome/browser/ui",
"//ios/chrome/browser/ui/commands",
"//ios/chrome/browser/ui/coordinators:chrome_coordinators",
"//ios/chrome/browser/ui/history",
"//ios/chrome/browser/ui/history/public",
"//ios/chrome/browser/ui/main",
"//ios/chrome/browser/ui/recent_tabs",
"//ios/chrome/browser/ui/recent_tabs:recent_tabs_ui",
......
......@@ -9,6 +9,8 @@
#import "ios/chrome/browser/ui/commands/browser_commands.h"
#import "ios/chrome/browser/ui/commands/command_dispatcher.h"
#import "ios/chrome/browser/ui/commands/open_new_tab_command.h"
#import "ios/chrome/browser/ui/history/history_coordinator.h"
#import "ios/chrome/browser/ui/history/public/history_tab_presentation_delegate.h"
#import "ios/chrome/browser/ui/main/bvc_container_view_controller.h"
#import "ios/chrome/browser/ui/ntp/recent_tabs/recent_tabs_handset_view_controller.h"
#import "ios/chrome/browser/ui/recent_tabs/recent_tabs_mediator.h"
......@@ -17,6 +19,7 @@
#import "ios/chrome/browser/ui/tab_grid/tab_grid_mediator.h"
#import "ios/chrome/browser/ui/tab_grid/tab_grid_paging.h"
#import "ios/chrome/browser/ui/tab_grid/tab_grid_transition_handler.h"
#import "ios/chrome/browser/ui/tab_grid/tab_grid_url_loader.h"
#import "ios/chrome/browser/ui/tab_grid/tab_grid_view_controller.h"
#if !defined(__has_feature) || !__has_feature(objc_arc)
......@@ -24,6 +27,7 @@
#endif
@interface TabGridCoordinator ()<TabPresentationDelegate,
HistoryTabPresentationDelegate,
RecentTabsHandsetViewControllerCommand>
// Superclass property specialized for the class that this coordinator uses.
@property(nonatomic, weak) TabGridViewController* mainViewController;
......@@ -43,6 +47,11 @@
@property(nonatomic, strong) TabGridMediator* incognitoTabsMediator;
// Mediator for remote Tabs.
@property(nonatomic, strong) RecentTabsMediator* remoteTabsMediator;
// Coordinator for history, which can be started from recent tabs.
@property(nonatomic, strong) HistoryCoordinator* historyCoordinator;
// Specialized URL loader for tab grid, since tab grid has a different use case
// than BVC.
@property(nonatomic, strong) TabGridURLLoader* URLLoader;
@end
@implementation TabGridCoordinator
......@@ -60,6 +69,8 @@
@synthesize regularTabsMediator = _regularTabsMediator;
@synthesize incognitoTabsMediator = _incognitoTabsMediator;
@synthesize remoteTabsMediator = _remoteTabsMediator;
@synthesize historyCoordinator = _historyCoordinator;
@synthesize URLLoader = _URLLoader;
- (instancetype)initWithWindow:(nullable UIWindow*)window
applicationCommandEndpoint:
......@@ -169,7 +180,12 @@
self.remoteTabsMediator;
mainViewController.remoteTabsViewController.dispatcher =
static_cast<id<ApplicationCommands>>(self.dispatcher);
mainViewController.remoteTabsViewController.loader = self.regularTabsMediator;
self.URLLoader = [[TabGridURLLoader alloc]
initWithRegularWebStateList:self.regularTabModel.webStateList
incognitoWebStateList:self.incognitoTabModel.webStateList
regularBrowserState:self.regularTabModel.browserState
incognitoBrowserState:self.incognitoTabModel.browserState];
mainViewController.remoteTabsViewController.loader = self.URLLoader;
mainViewController.remoteTabsViewController.handsetCommandHandler = self;
// TODO(crbug.com/850387) : Currently, consumer calls from the mediator
......@@ -315,21 +331,38 @@
transition:ui::PAGE_TRANSITION_TYPED];
}
- (void)closeAllTabs {
}
- (void)closeAllIncognitoTabs {
}
#pragma mark - RecentTabsHandsetViewControllerCommand
// Normally, recent tabs is dismissed to reveal the tab view beneath it. In tab
// grid, the tab view is presented above the recent tabs panel.
- (void)dismissRecentTabsWithCompletion:(void (^)())completion {
// Trigger the transition through the TabSwitcher delegate. This will in turn
// call back into this coordinator via the ViewControllerSwapping protocol.
if (completion) {
// TODO(crbug.com/845192) : Don't use the completion block as an implicit
// signal to show history.
// A history coordinator from main_controller won't work properly from the
// tab grid. Using a local coordinator works better when hooked up with a
// specialized URL loader and tab presentation delegate.
self.historyCoordinator = [[HistoryCoordinator alloc]
initWithBaseViewController:self.mainViewController
browserState:self.regularTabModel.browserState];
self.historyCoordinator.loader = self.URLLoader;
self.historyCoordinator.tabPresentationDelegate = self;
[self.historyCoordinator start];
return;
}
[self showActiveRegularTab];
}
#pragma mark - HistoryTabPresentationDelegate
- (void)showActiveRegularTab {
[self.tabSwitcher.delegate tabSwitcher:self.tabSwitcher
shouldFinishWithActiveModel:self.regularTabModel];
}
- (void)showActiveIncognitoTab {
[self.tabSwitcher.delegate tabSwitcher:self.tabSwitcher
shouldFinishWithActiveModel:self.incognitoTabModel];
}
@end
......@@ -9,14 +9,12 @@
#import "ios/chrome/browser/ui/tab_grid/grid/grid_commands.h"
#import "ios/chrome/browser/ui/tab_grid/grid/grid_image_data_source.h"
#import "ios/chrome/browser/ui/url_loader.h"
@protocol GridConsumer;
@class TabModel;
// Mediates between model layer and tab grid UI layer.
@interface TabGridMediator
: NSObject<GridCommands, GridImageDataSource, UrlLoader>
@interface TabGridMediator : NSObject<GridCommands, GridImageDataSource>
// The source tab model.
@property(nonatomic, weak) TabModel* tabModel;
......
......@@ -9,10 +9,8 @@
#include "base/scoped_observer.h"
#include "base/strings/sys_string_conversions.h"
#include "components/favicon/ios/web_favicon_driver.h"
#include "components/sessions/core/session_types.h"
#include "ios/chrome/browser/browser_state/chrome_browser_state.h"
#include "ios/chrome/browser/chrome_url_constants.h"
#include "ios/chrome/browser/sessions/session_util.h"
#import "ios/chrome/browser/snapshots/snapshot_cache.h"
#import "ios/chrome/browser/snapshots/snapshot_cache_factory.h"
#import "ios/chrome/browser/snapshots/snapshot_tab_helper.h"
......@@ -85,15 +83,6 @@ web::WebState* GetWebStateWithId(WebStateList* web_state_list,
return nullptr;
}
// Appends and activates |web_state| to the end of |web_state_list|.
void AppendAndActivateWebState(WebStateList* web_state_list,
std::unique_ptr<web::WebState> web_state) {
web_state_list->InsertWebState(
web_state_list->count(), std::move(web_state),
(WebStateList::INSERT_FORCE_INDEX | WebStateList::INSERT_ACTIVATE),
WebStateOpener());
}
} // namespace
@interface TabGridMediator ()<CRWWebStateObserver, WebStateListObserving>
......@@ -308,54 +297,6 @@ void AppendAndActivateWebState(WebStateList* web_state_list,
[SnapshotCacheFactory::GetForBrowserState(browserState) removeMarkedImages];
}
#pragma mark - UrlLoader
// Loading a |sessionTab| normally means navigating on the currently visible tab
// view. This is not the case while in the tab grid. A new WebState is appended
// and activated instead of replacing the current active WebState.
- (void)loadSessionTab:(const sessions::SessionTab*)sessionTab {
DCHECK(self.tabModel.browserState);
std::unique_ptr<web::WebState> webState =
session_util::CreateWebStateWithNavigationEntries(
self.tabModel.browserState, sessionTab->current_navigation_index,
sessionTab->navigations);
AppendAndActivateWebState(self.webStateList, std::move(webState));
}
// In tab grid, |inBackground| is ignored, which means that the new WebState is
// activated. |appendTo| is also ignored, so the new WebState is always appended
// at the end of the list. The page transition type is explicit rather than
// linked.
- (void)webPageOrderedOpen:(const GURL&)URL
referrer:(const web::Referrer&)referrer
inBackground:(BOOL)inBackground
appendTo:(OpenPosition)appendTo {
DCHECK(self.tabModel.browserState);
web::WebState::CreateParams params(self.tabModel.browserState);
std::unique_ptr<web::WebState> webState = web::WebState::Create(params);
web::NavigationManager::WebLoadParams loadParams(URL);
loadParams.referrer = referrer;
loadParams.transition_type = ui::PAGE_TRANSITION_TYPED;
webState->GetNavigationManager()->LoadURLWithParams(loadParams);
AppendAndActivateWebState(self.webStateList, std::move(webState));
}
- (void)webPageOrderedOpen:(const GURL&)URL
referrer:(const web::Referrer&)referrer
inIncognito:(BOOL)inIncognito
inBackground:(BOOL)inBackground
appendTo:(OpenPosition)appendTo {
NOTREACHED() << "This is intentionally NO-OP in TabGridMediator.";
}
- (void)loadURLWithParams:(const web::NavigationManager::WebLoadParams&)params {
NOTREACHED() << "This is intentionally NO-OP in TabGridMediator.";
}
- (void)loadJavaScriptFromLocationBar:(NSString*)script {
NOTREACHED() << "This is intentionally NO-OP in TabGridMediator.";
}
#pragma mark - GridImageDataSource
- (void)snapshotForIdentifier:(NSString*)identifier
......
// 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_TAB_GRID_TAB_GRID_URL_LOADER_H_
#define IOS_CHROME_BROWSER_UI_TAB_GRID_TAB_GRID_URL_LOADER_H_
#import <Foundation/Foundation.h>
#import "ios/chrome/browser/ui/url_loader.h"
namespace ios {
class ChromeBrowserState;
}
class WebStateList;
// Specialized URLLoader for the tab grid, which behaves differently than BVC.
@interface TabGridURLLoader : NSObject<UrlLoader>
- (instancetype)
initWithRegularWebStateList:(WebStateList*)regularWebStateList
incognitoWebStateList:(WebStateList*)incognitoWebStateList
regularBrowserState:(ios::ChromeBrowserState*)regularBrowserState
incognitoBrowserState:(ios::ChromeBrowserState*)incognitoBrowserState
NS_DESIGNATED_INITIALIZER;
- (instancetype)init NS_UNAVAILABLE;
@end
#endif // IOS_CHROME_BROWSER_UI_TAB_GRID_TAB_GRID_URL_LOADER_H_
// 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.
#import "ios/chrome/browser/ui/tab_grid/tab_grid_url_loader.h"
#include "components/sessions/core/session_types.h"
#include "ios/chrome/browser/browser_state/chrome_browser_state.h"
#include "ios/chrome/browser/sessions/session_util.h"
#import "ios/chrome/browser/web_state_list/web_state_list.h"
#include "ios/chrome/browser/web_state_list/web_state_opener.h"
#include "ios/web/public/web_state/web_state.h"
#if !defined(__has_feature) || !__has_feature(objc_arc)
#error "This file requires ARC support."
#endif
namespace {
// Appends and activates |web_state| to the end of |web_state_list|.
void AppendAndActivateWebState(WebStateList* web_state_list,
std::unique_ptr<web::WebState> web_state) {
web_state_list->InsertWebState(
web_state_list->count(), std::move(web_state),
(WebStateList::INSERT_FORCE_INDEX | WebStateList::INSERT_ACTIVATE),
WebStateOpener());
}
} // namespace
@interface TabGridURLLoader ()
@property(nonatomic, assign) WebStateList* regularWebStateList;
@property(nonatomic, assign) WebStateList* incognitoWebStateList;
@property(nonatomic, assign) ios::ChromeBrowserState* regularBrowserState;
@property(nonatomic, assign) ios::ChromeBrowserState* incognitoBrowserState;
@end
@implementation TabGridURLLoader
@synthesize regularWebStateList = _regularWebStateList;
@synthesize incognitoWebStateList = _incognitoWebStateList;
@synthesize regularBrowserState = _regularBrowserState;
@synthesize incognitoBrowserState = _incognitoBrowserState;
- (instancetype)
initWithRegularWebStateList:(WebStateList*)regularWebStateList
incognitoWebStateList:(WebStateList*)incognitoWebStateList
regularBrowserState:(ios::ChromeBrowserState*)regularBrowserState
incognitoBrowserState:(ios::ChromeBrowserState*)incognitoBrowserState {
if (self = [super init]) {
_regularWebStateList = regularWebStateList;
_regularBrowserState = regularBrowserState;
_incognitoWebStateList = incognitoWebStateList;
_incognitoBrowserState = incognitoBrowserState;
}
return self;
}
#pragma mark - UrlLoader
// Loading a |sessionTab| normally means navigating on the currently visible tab
// view. This is not the case while in the tab grid. A new WebState is appended
// and activated instead of replacing the current active WebState.
- (void)loadSessionTab:(const sessions::SessionTab*)sessionTab {
DCHECK(self.regularBrowserState);
std::unique_ptr<web::WebState> webState =
session_util::CreateWebStateWithNavigationEntries(
self.regularBrowserState, sessionTab->current_navigation_index,
sessionTab->navigations);
AppendAndActivateWebState(self.regularWebStateList, std::move(webState));
}
// Opens |URL| in a new regular tab.
- (void)webPageOrderedOpen:(const GURL&)URL
referrer:(const web::Referrer&)referrer
inBackground:(BOOL)inBackground
appendTo:(OpenPosition)appendTo {
[self webPageOrderedOpen:URL
referrer:referrer
inIncognito:NO
inBackground:inBackground
appendTo:appendTo];
}
// In tab grid, |inBackground| is ignored, which means that the new WebState is
// activated. |appendTo| is also ignored, so the new WebState is always appended
// at the end of the list. The page transition type is explicit rather than
// linked.
- (void)webPageOrderedOpen:(const GURL&)URL
referrer:(const web::Referrer&)referrer
inIncognito:(BOOL)inIncognito
inBackground:(BOOL)inBackground
appendTo:(OpenPosition)appendTo {
WebStateList* webStateList;
ios::ChromeBrowserState* browserState;
if (inIncognito) {
webStateList = self.incognitoWebStateList;
browserState = self.incognitoBrowserState;
} else {
webStateList = self.regularWebStateList;
browserState = self.regularBrowserState;
}
DCHECK(webStateList);
DCHECK(browserState);
web::WebState::CreateParams params(browserState);
std::unique_ptr<web::WebState> webState = web::WebState::Create(params);
web::NavigationManager::WebLoadParams loadParams(URL);
loadParams.referrer = referrer;
loadParams.transition_type = ui::PAGE_TRANSITION_TYPED;
webState->GetNavigationManager()->LoadURLWithParams(loadParams);
AppendAndActivateWebState(webStateList, std::move(webState));
}
// Opens |loadParams| in a new regular tab.
- (void)loadURLWithParams:
(const web::NavigationManager::WebLoadParams&)loadParams {
DCHECK(self.regularBrowserState);
web::WebState::CreateParams params(self.regularBrowserState);
std::unique_ptr<web::WebState> webState = web::WebState::Create(params);
webState->GetNavigationManager()->LoadURLWithParams(loadParams);
AppendAndActivateWebState(self.regularWebStateList, std::move(webState));
}
- (void)loadJavaScriptFromLocationBar:(NSString*)script {
NOTREACHED() << "This is intentionally NO-OP in TabGridMediator.";
}
@end
......@@ -109,8 +109,7 @@ NSUInteger GetPageIndexFromPage(TabGridPage page) {
if (self = [super init]) {
_regularTabsViewController = [[GridViewController alloc] init];
_incognitoTabsViewController = [[GridViewController alloc] init];
_remoteTabsViewController = [[RecentTabsTableViewController alloc]
initWithoutRecentlyClosedSection];
_remoteTabsViewController = [[RecentTabsTableViewController alloc] init];
}
return 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