Commit 0c78d03c authored by Sebastien Lalancette's avatar Sebastien Lalancette Committed by Commit Bot

[iOS] Don't Dismiss RecentTabs When Opening Entry In New Window

For some reason, opening in new window was explicitly closing
RecentTabs. Removed that piece of logic, and cleaned-up the protocol.

Bug: 1140517
Change-Id: Ic619dfb14057e468b9694e6d922883c8351c0438
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2544364
Commit-Queue: Sebastien Lalancette <seblalancette@chromium.org>
Reviewed-by: default avatarMark Cogan <marq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#828227}
parent 30397ab8
......@@ -2292,11 +2292,10 @@ std::vector<GURL> GetUrlsToOpen(const std::vector<const BookmarkNode*>& nodes) {
if (IsMultipleScenesSupported()) {
[menuElements
addObject:
[actionFactory
actionToOpenInNewWindowWithURL:node->url()
activityOrigin:WindowActivityBookmarksOrigin
completion:nil]];
addObject:[actionFactory
actionToOpenInNewWindowWithURL:node->url()
activityOrigin:
WindowActivityBookmarksOrigin]];
}
[menuElements addObject:[actionFactory actionToCopyURL:node->url()]];
......
......@@ -623,13 +623,11 @@
}]];
if (IsMultipleScenesSupported()) {
[menuElements
addObject:
[actionFactory
actionToOpenInNewWindowWithURL:item.URL
activityOrigin:
WindowActivityContentSuggestionsOrigin
completion:nil]];
UIAction* newWindowAction = [actionFactory
actionToOpenInNewWindowWithURL:item.URL
activityOrigin:
WindowActivityContentSuggestionsOrigin];
[menuElements addObject:newWindowAction];
}
[menuElements addObject:[actionFactory actionToCopyURL:item.URL]];
......
......@@ -227,8 +227,7 @@
addObject:
[actionFactory
actionToOpenInNewWindowWithURL:item.URL
activityOrigin:WindowActivityHistoryOrigin
completion:nil]];
activityOrigin:WindowActivityHistoryOrigin]];
}
[menuElements addObject:[actionFactory actionToCopyURL:item.URL]];
......
......@@ -72,11 +72,10 @@ API_AVAILABLE(ios(13.0))
- (UIAction*)actionToOpenInNewIncognitoTabWithBlock:(ProceduralBlock)block;
// Creates a UIAction instance configured for opening the |URL| in a new window
// from |activityOrigin|, and which will invoke the given |completion| block
// after execution.
// from |activityOrigin|.
- (UIAction*)actionToOpenInNewWindowWithURL:(const GURL)URL
activityOrigin:(WindowActivityOrigin)activityOrigin
completion:(ProceduralBlock)completion;
activityOrigin:
(WindowActivityOrigin)activityOrigin;
// Creates a UIAction instance configured for suppression which will invoke
// the given delete |block| when executed.
......
......@@ -138,8 +138,8 @@
}
- (UIAction*)actionToOpenInNewWindowWithURL:(const GURL)URL
activityOrigin:(WindowActivityOrigin)activityOrigin
completion:(ProceduralBlock)completion {
activityOrigin:
(WindowActivityOrigin)activityOrigin {
id<ApplicationCommands> windowOpener = HandlerForProtocol(
self.browser->GetCommandDispatcher(), ApplicationCommands);
NSUserActivity* activity = ActivityToLoadURL(activityOrigin, URL);
......@@ -149,9 +149,6 @@
type:MenuActionType::OpenInNewWindow
block:^{
[windowOpener openNewWindowWithActivity:activity];
if (completion) {
completion();
}
}];
}
......
......@@ -213,8 +213,7 @@ TEST_F(ActionFactoryTest, OpenInNewWindowAction) {
UIAction* action =
[factory actionToOpenInNewWindowWithURL:testURL
activityOrigin:WindowActivityToolsOrigin
completion:nil];
activityOrigin:WindowActivityToolsOrigin];
EXPECT_TRUE([expectedTitle isEqualToString:action.title]);
EXPECT_EQ(expectedImage, action.image);
......
......@@ -485,11 +485,11 @@ animationControllerForDismissedController:(UIViewController*)dismissed {
if (IsMultipleScenesSupported()) {
[menuElements
addObject:[actionFactory
actionToOpenInNewWindowWithURL:item.entryURL
activityOrigin:
WindowActivityReadingListOrigin
completion:nil]];
addObject:
[actionFactory
actionToOpenInNewWindowWithURL:item.entryURL
activityOrigin:
WindowActivityReadingListOrigin]];
}
if ([accessibilityDelegate isItemRead:item]) {
......
......@@ -181,16 +181,12 @@
[self showActiveRegularTabFromRecentTabs];
}
- (void)dismissRecentTabs {
- (void)showActiveRegularTabFromRecentTabs {
// Stopping this coordinator reveals the tab UI underneath.
self.completion = nil;
[self stop];
}
- (void)showActiveRegularTabFromRecentTabs {
[self dismissRecentTabs];
}
- (void)showHistoryFromRecentTabs {
// Dismiss recent tabs before presenting history.
CommandDispatcher* dispatcher = self.browser->GetCommandDispatcher();
......
......@@ -79,30 +79,24 @@
[actionFactory
actionToOpenInNewTabWithURL:item.URL
completion:^{
[strongSelf.recentTabsPresentationDelegate
[weakSelf.recentTabsPresentationDelegate
showActiveRegularTabFromRecentTabs];
}]];
if (IsMultipleScenesSupported()) {
[menuElements
addObject:
[actionFactory
actionToOpenInNewWindowWithURL:item.URL
activityOrigin:WindowActivityRecentTabsOrigin
completion:^{
[strongSelf
.recentTabsPresentationDelegate
dismissRecentTabs];
}]];
addObject:[actionFactory
actionToOpenInNewWindowWithURL:item.URL
activityOrigin:
WindowActivityRecentTabsOrigin]];
}
[menuElements addObject:[actionFactory actionToCopyURL:item.URL]];
[menuElements addObject:[actionFactory actionToShareWithBlock:^{
[strongSelf.recentTabsContextMenuDelegate
shareURL:item.URL
title:item.title
fromView:view];
[weakSelf.recentTabsContextMenuDelegate shareURL:item.URL
title:item.title
fromView:view];
}]];
return [UIMenu menuWithTitle:@"" children:menuElements];
......
......@@ -14,9 +14,6 @@ class DistantSession;
// Presentation commands that depend on the context from which they are
// presented.
@protocol RecentTabsPresentationDelegate
// Tells the receiver to dismiss recent tabs. This may be used by a keyboard
// escape shortcut. Receiver may choose to ignore this message.
- (void)dismissRecentTabs;
// Tells the receiver to show the tab UI for regular tabs. NO-OP if the correct
// tab UI is already visible. Receiver may also dismiss recent tabs.
- (void)showActiveRegularTabFromRecentTabs;
......
......@@ -1427,8 +1427,7 @@ API_AVAILABLE(ios(13.0))
- (void)presentationControllerDidDismiss:
(UIPresentationController*)presentationController {
base::RecordAction(base::UserMetricsAction("IOSRecentTabsCloseWithSwipe"));
// Call dismissRecentTabs so the Coordinator cleans up any state it needs to.
[self.presentationDelegate dismissRecentTabs];
[self.presentationDelegate showActiveRegularTabFromRecentTabs];
}
#pragma mark - Accessibility
......
......@@ -501,11 +501,6 @@
#pragma mark - RecentTabsPresentationDelegate
- (void)dismissRecentTabs {
// It is valid for tab grid to ignore this since recent tabs is embedded and
// will not be dismissed.
}
- (void)showHistoryFromRecentTabs {
// A history coordinator from main_controller won't work properly from the
// tab grid. Using a local coordinator works better and we need to set
......
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