Skip to content

GitLab

T
tangled
Commit 7dc53d3b authored by Guillaume Jenkins's avatar Guillaume Jenkins Committed by Commit Bot
Browse files
Options

[iOS intents] Fix Search in Chrome shortcut when in foreground 

The Search in Chrome intent wasn't doing anything if Chrome was in
the foreground, because in that case a separate code path is used and
that code path did not check for |searchQuery| in the startup
parameters.

The fix is to have that code path properly check for and handle
|searchQuery|. Because performing a search requires the user's default
search engine, the current BrowserState must now be passed to the
user activity handler.

The code that generates the search URL from the user's default search
engine has been refactored into a reusable function so it can be called
from the foreground code path.

Bug: 1115998
Change-Id: I88899ef8236584b0f69ea58362960abf6924650f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2356669
Commit-Queue: Guillaume Jenkins <gujen@google.com>
Reviewed-by: default avatarJustin Cohen <justincohen@chromium.org>
Reviewed-by: default avatarMark Cogan <marq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#800239}
parent 55905f76
Hide whitespace changes
Inline Side-by-side
Showing with 165 additions and 108 deletions
ios/chrome/app/application_delegate/user_activity_handler.h
View file @ 7dc53d3b
......@@ -20,13 +20,16 @@ class ChromeBrowserState;
@interface UserActivityHandler : NSObject
// If the userActivity is a Handoff or an opening from Spotlight, opens a new
// tab or setup startupParameters to open it later.
// Returns wether it could continue userActivity.
// tab or setup startupParameters to open it later. If a new tab must be
// opened immediately (e.g. if a Siri Shortcut was triggered by the user while
// Chrome was already in the foreground), it will be done with the provided
// |browserState|. Returns wether it could continue userActivity.
+ (BOOL)continueUserActivity:(NSUserActivity*)userActivity
applicationIsActive:(BOOL)applicationIsActive
tabOpener:(id<TabOpening>)tabOpener
connectionInformation:(id<ConnectionInformation>)connectionInformation
startupInformation:(id<StartupInformation>)startupInformation;
startupInformation:(id<StartupInformation>)startupInformation
browserState:(ChromeBrowserState*)browserState;
// Handles the 3D touch application static items. If the First Run UI is active,
// |completionHandler| will be called with NO.
......
ios/chrome/app/application_delegate/user_activity_handler.mm
View file @ 7dc53d3b
......@@ -89,7 +89,8 @@ std::vector<GURL> createGURLVectorFromIntentURLs(NSArray<NSURL*>* intentURLs) {
applicationIsActive:(BOOL)applicationIsActive
tabOpener:(id<TabOpening>)tabOpener
connectionInformation:(id<ConnectionInformation>)connectionInformation
startupInformation:(id<StartupInformation>)startupInformation {
startupInformation:(id<StartupInformation>)startupInformation
browserState:(ChromeBrowserState*)browserState {
NSURL* webpageURL = userActivity.webpageURL;
if ([userActivity.activityType
......@@ -151,7 +152,8 @@ std::vector<GURL> createGURLVectorFromIntentURLs(NSArray<NSURL*>* intentURLs) {
applicationIsActive:isActive
tabOpener:tabOpener
connectionInformation:connectionInformation
startupInformation:startupInformation];
startupInformation:startupInformation
browserState:browserState];
});
});
return YES;
......@@ -179,6 +181,7 @@ std::vector<GURL> createGURLVectorFromIntentURLs(NSArray<NSURL*>* intentURLs) {
[connectionInformation setStartupParameters:startupParams];
webpageURL =
[NSURL URLWithString:base::SysUTF8ToNSString(kChromeUINewTabURL)];
} else if ([userActivity.activityType
isEqualToString:kSiriShortcutOpenInChrome]) {
base::RecordAction(UserMetricsAction("IOSLaunchedByOpenInChromeIntent"));
......@@ -235,14 +238,16 @@ std::vector<GURL> createGURLVectorFromIntentURLs(NSArray<NSURL*>* intentURLs) {
applicationIsActive:applicationIsActive
tabOpener:tabOpener
connectionInformation:connectionInformation
startupInformation:startupInformation];
startupInformation:startupInformation
browserState:browserState];
}
+ (BOOL)continueUserActivityURL:(NSURL*)webpageURL
applicationIsActive:(BOOL)applicationIsActive
tabOpener:(id<TabOpening>)tabOpener
connectionInformation:(id<ConnectionInformation>)connectionInformation
startupInformation:(id<StartupInformation>)startupInformation {
startupInformation:(id<StartupInformation>)startupInformation
browserState:(ChromeBrowserState*)browserState {
if (!webpageURL)
return NO;
......@@ -256,6 +261,15 @@ std::vector<GURL> createGURLVectorFromIntentURLs(NSArray<NSURL*>* intentURLs) {
ApplicationModeForTabOpening targetMode =
[[connectionInformation startupParameters] applicationMode];
UrlLoadParams params = UrlLoadParams::InNewTab(webpageGURL);
if (connectionInformation.startupParameters.textQuery) {
NSString* query = connectionInformation.startupParameters.textQuery;
GURL result = [self generateResultGURLFromSearchQuery:query
browserState:browserState];
params.web_params.url = result;
}
if (![[connectionInformation startupParameters] launchInIncognito] &&
[tabOpener URLIsOpenedInRegularMode:webpageGURL]) {
// Record metric.
......@@ -376,6 +390,25 @@ std::vector<GURL> createGURLVectorFromIntentURLs(NSArray<NSURL*>* intentURLs) {
[userActivityType isEqualToString:CSSearchableItemActionType]);
}
+ (GURL)generateResultGURLFromSearchQuery:(NSString*)searchQuery
browserState:(ChromeBrowserState*)browserState {
TemplateURLService* templateURLService =
ios::TemplateURLServiceFactory::GetForBrowserState(browserState);
const TemplateURL* defaultURL =
templateURLService->GetDefaultSearchProvider();
DCHECK(!defaultURL->url().empty());
DCHECK(
defaultURL->url_ref().IsValid(templateURLService->search_terms_data()));
base::string16 queryString = base::SysNSStringToUTF16(searchQuery);
TemplateURLRef::SearchTermsArgs search_args(queryString);
GURL result(defaultURL->url_ref().ReplaceSearchTerms(
search_args, templateURLService->search_terms_data()));
return result;
}
+ (void)handleStartupParametersWithTabOpener:(id<TabOpening>)tabOpener
connectionInformation:
(id<ConnectionInformation>)connectionInformation
......@@ -442,19 +475,8 @@ std::vector<GURL> createGURLVectorFromIntentURLs(NSArray<NSURL*>* intentURLs) {
} else if (connectionInformation.startupParameters.textQuery) {
NSString* query = connectionInformation.startupParameters.textQuery;
TemplateURLService* templateURLService =
ios::TemplateURLServiceFactory::GetForBrowserState(browserState);
const TemplateURL* defaultURL =
templateURLService->GetDefaultSearchProvider();
DCHECK(!defaultURL->url().empty());
DCHECK(defaultURL->url_ref().IsValid(
templateURLService->search_terms_data()));
base::string16 queryString = base::SysNSStringToUTF16(query);
TemplateURLRef::SearchTermsArgs search_args(queryString);
GURL result(defaultURL->url_ref().ReplaceSearchTerms(
search_args, templateURLService->search_terms_data()));
GURL result = [self generateResultGURLFromSearchQuery:query
browserState:browserState];
params.web_params.url = result;
}
......
ios/chrome/app/application_delegate/user_activity_handler_unittest.mm
View file @ 7dc53d3b
......@@ -90,6 +90,11 @@ typedef void (^startupParameterBlock)(id,
typedef void (^conditionBlock)(BOOL);
class UserActivityHandlerTest : public PlatformTest {
public:
UserActivityHandlerTest() {
interfaceProvider_ = [[StubBrowserInterfaceProvider alloc] init];
}
protected:
void swizzleHandleStartupParameters() {
handle_startup_parameters_has_been_called_ = NO;
......@@ -136,6 +141,10 @@ class UserActivityHandlerTest : public PlatformTest {
BOOL completionHandlerArgument() { return block_argument_; }
StubBrowserInterfaceProvider* GetInterfaceProvider() {
return interfaceProvider_;
}
private:
__block BOOL block_executed_;
__block BOOL block_argument_;
......@@ -143,6 +152,7 @@ class UserActivityHandlerTest : public PlatformTest {
startupParameterBlock swizzle_block_;
conditionBlock completion_block_;
__block BOOL handle_startup_parameters_has_been_called_;
StubBrowserInterfaceProvider* interfaceProvider_;
};
#pragma mark - Tests.
......@@ -198,12 +208,14 @@ TEST_F(UserActivityHandlerTest, ContinueUserActivityFromGarbage) {
[OCMockObject mockForProtocol:@protocol(ConnectionInformation)];
// Action.
BOOL result =
[UserActivityHandler continueUserActivity:userActivity
applicationIsActive:NO
tabOpener:tabOpenerMock
connectionInformation:connectionInformation
startupInformation:startupInformationMock];
BOOL result = [UserActivityHandler
continueUserActivity:userActivity
applicationIsActive:NO
tabOpener:tabOpenerMock
connectionInformation:connectionInformation
startupInformation:startupInformationMock
browserState:GetInterfaceProvider()
.currentInterface.browserState];
// Tests.
EXPECT_FALSE(result);
......@@ -225,12 +237,14 @@ TEST_F(UserActivityHandlerTest, ContinueUserActivityNoWebpage) {
[OCMockObject mockForProtocol:@protocol(StartupInformation)];
// Action.
BOOL result =
[UserActivityHandler continueUserActivity:userActivity
applicationIsActive:NO
tabOpener:tabOpenerMock
connectionInformation:connectionInformationMock
startupInformation:startupInformationMock];
BOOL result = [UserActivityHandler
continueUserActivity:userActivity
applicationIsActive:NO
tabOpener:tabOpenerMock
connectionInformation:connectionInformationMock
startupInformation:startupInformationMock
browserState:GetInterfaceProvider()
.currentInterface.browserState];
// Tests.
EXPECT_FALSE(result);
......@@ -266,12 +280,14 @@ TEST_F(UserActivityHandlerTest,
id connectionInformationMock =
[OCMockObject mockForProtocol:@protocol(ConnectionInformation)];
// Action.
BOOL result =
[UserActivityHandler continueUserActivity:userActivity
applicationIsActive:NO
tabOpener:tabOpenerMock
connectionInformation:connectionInformationMock
startupInformation:startupInformationMock];
BOOL result = [UserActivityHandler
continueUserActivity:userActivity
applicationIsActive:NO
tabOpener:tabOpenerMock
connectionInformation:connectionInformationMock
startupInformation:startupInformationMock
browserState:GetInterfaceProvider()
.currentInterface.browserState];
// Tests.
EXPECT_FALSE(result);
......@@ -303,12 +319,14 @@ TEST_F(UserActivityHandlerTest, ContinueUserActivityBackground) {
id tabOpenerMock = [OCMockObject mockForProtocol:@protocol(TabOpening)];
// Action.
BOOL result =
[UserActivityHandler continueUserActivity:userActivity
applicationIsActive:NO
tabOpener:tabOpenerMock
connectionInformation:connectionInformationMock
startupInformation:startupInformationMock];
BOOL result = [UserActivityHandler
continueUserActivity:userActivity
applicationIsActive:NO
tabOpener:tabOpenerMock
connectionInformation:connectionInformationMock
startupInformation:startupInformationMock
browserState:GetInterfaceProvider()
.currentInterface.browserState];
// Test.
EXPECT_OCMOCK_VERIFY(startupInformationMock);
......@@ -338,12 +356,14 @@ TEST_F(UserActivityHandlerTest, ContinueUserActivityForeground) {
startupParameters];
// Action.
BOOL result =
[UserActivityHandler continueUserActivity:userActivity
applicationIsActive:YES
tabOpener:tabOpener
connectionInformation:connectionInformationMock
startupInformation:startupInformationMock];
BOOL result = [UserActivityHandler
continueUserActivity:userActivity
applicationIsActive:YES
tabOpener:tabOpener
connectionInformation:connectionInformationMock
startupInformation:startupInformationMock
browserState:GetInterfaceProvider()
.currentInterface.browserState];
// Test.
EXPECT_EQ(gurl, tabOpener.urlLoadParams.web_params.url);
......@@ -369,12 +389,14 @@ TEST_F(UserActivityHandlerTest, ContinueUserActivityBrowsingWeb) {
FakeConnectionInformation* connectionInformationMock =
[[FakeConnectionInformation alloc] init];
BOOL result =
[UserActivityHandler continueUserActivity:userActivity
applicationIsActive:YES
tabOpener:tabOpener
connectionInformation:connectionInformationMock
startupInformation:fakeStartupInformation];
BOOL result = [UserActivityHandler
continueUserActivity:userActivity
applicationIsActive:YES
tabOpener:tabOpener
connectionInformation:connectionInformationMock
startupInformation:fakeStartupInformation
browserState:GetInterfaceProvider()
.currentInterface.browserState];
GURL newTabURL(kChromeUINewTabURL);
EXPECT_EQ(newTabURL, tabOpener.urlLoadParams.web_params.url);
......@@ -436,12 +458,14 @@ TEST_F(UserActivityHandlerTest, ContinueUserActivityShortcutActions) {
id tabOpenerMock = [OCMockObject mockForProtocol:@protocol(TabOpening)];
// Action.
BOOL result =
[UserActivityHandler continueUserActivity:userActivity
applicationIsActive:NO
tabOpener:tabOpenerMock
connectionInformation:connectionInformationMock
startupInformation:fakeStartupInformation];
BOOL result = [UserActivityHandler
continueUserActivity:userActivity
applicationIsActive:NO
tabOpener:tabOpenerMock
connectionInformation:connectionInformationMock
startupInformation:fakeStartupInformation
browserState:GetInterfaceProvider()
.currentInterface.browserState];
// Tests.
EXPECT_TRUE(result);
......@@ -499,12 +523,14 @@ TEST_F(UserActivityHandlerTest, ContinueUserActivityIntentIncognitoBackground) {
MockTabOpener* tabOpener = [[MockTabOpener alloc] init];
// Action.
BOOL result =
[UserActivityHandler continueUserActivity:userActivity
applicationIsActive:NO
tabOpener:tabOpener
connectionInformation:connectionInformationMock
startupInformation:startupInformationMock];
BOOL result = [UserActivityHandler
continueUserActivity:userActivity
applicationIsActive:NO
tabOpener:tabOpener
connectionInformation:connectionInformationMock
startupInformation:startupInformationMock
browserState:GetInterfaceProvider()
.currentInterface.browserState];
EXPECT_OCMOCK_VERIFY(startupInformationMock);
EXPECT_TRUE(result);
......@@ -551,12 +577,14 @@ TEST_F(UserActivityHandlerTest, ContinueUserActivityIntentBackground) {
id tabOpenerMock = [OCMockObject mockForProtocol:@protocol(TabOpening)];
// Action.
BOOL result =
[UserActivityHandler continueUserActivity:userActivity
applicationIsActive:NO
tabOpener:tabOpenerMock
connectionInformation:connectionInformationMock
startupInformation:startupInformationMock];
BOOL result = [UserActivityHandler
continueUserActivity:userActivity
applicationIsActive:NO
tabOpener:tabOpenerMock
connectionInformation:connectionInformationMock
startupInformation:startupInformationMock
browserState:GetInterfaceProvider()
.currentInterface.browserState];
// Test.
EXPECT_OCMOCK_VERIFY(startupInformationMock);
......@@ -620,12 +648,14 @@ TEST_F(UserActivityHandlerTest, ContinueUserActivityIntentIncognitoForeground) {
MockTabOpener* tabOpener = [[MockTabOpener alloc] init];
// Action.
BOOL result =
[UserActivityHandler continueUserActivity:userActivity
applicationIsActive:YES
tabOpener:tabOpener
connectionInformation:connectionInformationMock
startupInformation:startupInformationMock];
BOOL result = [UserActivityHandler
continueUserActivity:userActivity
applicationIsActive:YES
tabOpener:tabOpener
connectionInformation:connectionInformationMock
startupInformation:startupInformationMock
browserState:GetInterfaceProvider()
.currentInterface.browserState];
// Test.
EXPECT_OCMOCK_VERIFY(startupInformationMock);
......@@ -684,12 +714,14 @@ TEST_F(UserActivityHandlerTest, ContinueUserActivityIntentForeground) {
startupParameters];
// Action.
BOOL result =
[UserActivityHandler continueUserActivity:userActivity
applicationIsActive:YES
tabOpener:tabOpener
connectionInformation:connectionInformationMock
startupInformation:startupInformationMock];
BOOL result = [UserActivityHandler
continueUserActivity:userActivity
applicationIsActive:YES
tabOpener:tabOpener
connectionInformation:connectionInformationMock
startupInformation:startupInformationMock
browserState:GetInterfaceProvider()
.currentInterface.browserState];
// Test.
EXPECT_OCMOCK_VERIFY(startupInformationMock);
......@@ -724,16 +756,14 @@ TEST_F(UserActivityHandlerTest, HandleStartupParamsWithExternalFile) {
// The test will fail is a method of this object is called.
// id interfaceProviderMock =
// [OCMockObject mockForProtocol:@protocol(BrowserInterfaceProvider)];
StubBrowserInterfaceProvider* interfaceProvider =
[[StubBrowserInterfaceProvider alloc] init];
// Action.
[UserActivityHandler
handleStartupParametersWithTabOpener:tabOpener
connectionInformation:connectionInformationMock
startupInformation:startupInformationMock
browserState:interfaceProvider.currentInterface
.browserState];
browserState:GetInterfaceProvider()
.currentInterface.browserState];
[tabOpener completionBlock]();
// Tests.
......@@ -771,16 +801,14 @@ TEST_F(UserActivityHandlerTest, HandleStartupParamsNonU2F) {
// The test will fail is a method of this object is called.
// id interfaceProviderMock =
// [OCMockObject mockForProtocol:@protocol(BrowserInterfaceProvider)];
StubBrowserInterfaceProvider* interfaceProvider =
[[StubBrowserInterfaceProvider alloc] init];
// Action.
[UserActivityHandler
handleStartupParametersWithTabOpener:tabOpener
connectionInformation:connectionInformationMock
startupInformation:startupInformationMock
browserState:interfaceProvider.currentInterface
.browserState];
browserState:GetInterfaceProvider()
.currentInterface.browserState];
[tabOpener completionBlock]();
// Tests.
......@@ -892,8 +920,6 @@ TEST_F(UserActivityHandlerTest, PerformActionForShortcutItemWithRealShortcut) {
// The test will fail is a method of those objects is called.
id tabOpenerMock = [OCMockObject mockForProtocol:@protocol(TabOpening)];
StubBrowserInterfaceProvider* interfaceProvider =
[[StubBrowserInterfaceProvider alloc] init];
// Action.
[UserActivityHandler performActionForShortcutItem:shortcut
......@@ -901,7 +927,7 @@ TEST_F(UserActivityHandlerTest, PerformActionForShortcutItemWithRealShortcut) {
tabOpener:tabOpenerMock
connectionInformation:fakeConnectionInformation
startupInformation:fakeStartupInformation
interfaceProvider:interfaceProvider];
interfaceProvider:GetInterfaceProvider()];
// Tests.
EXPECT_EQ(gurlNewTab,
......
ios/chrome/app/main_application_delegate.mm
View file @ 7dc53d3b
......@@ -330,11 +330,14 @@
BOOL applicationIsActive =
[application applicationState] == UIApplicationStateActive;
return [UserActivityHandler continueUserActivity:userActivity
applicationIsActive:applicationIsActive
tabOpener:_tabOpener
connectionInformation:self.sceneController
startupInformation:_startupInformation];
return [UserActivityHandler
continueUserActivity:userActivity
applicationIsActive:applicationIsActive
tabOpener:_tabOpener
connectionInformation:self.sceneController
startupInformation:_startupInformation
browserState:_mainController.interfaceProvider.currentInterface
.browserState];
}
- (void)application:(UIApplication*)application
......
ios/chrome/browser/ui/main/scene_controller.mm
View file @ 7dc53d3b
......@@ -395,11 +395,13 @@ const char kMultiWindowOpenInNewWindowHistogram[] =
// Consider the scene as still not active at this point as the handling
// of startup parameters is not yet done (and will be later in this
// function).
[UserActivityHandler continueUserActivity:activityWithCompletion
applicationIsActive:NO
tabOpener:self
connectionInformation:self
startupInformation:self.mainController];
[UserActivityHandler
continueUserActivity:activityWithCompletion
applicationIsActive:NO
tabOpener:self
connectionInformation:self
startupInformation:self.mainController
browserState:self.currentInterface.browserState];
}
self.sceneState.connectionOptions = nil;
}
......@@ -471,7 +473,8 @@ const char kMultiWindowOpenInNewWindowHistogram[] =
applicationIsActive:sceneIsActive
tabOpener:self
connectionInformation:self
startupInformation:self.mainController];
startupInformation:self.mainController
browserState:self.currentInterface.browserState];
// It is necessary to reset the pendingUserActivity after handling it.
// Handle the reset asynchronously to avoid interfering with other observers.
dispatch_async(dispatch_get_main_queue(), ^{
......
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