Commit 6bfd9932 authored by Gauthier Ambard's avatar Gauthier Ambard Committed by Commit Bot

Revert "[AppLauncher] Look at core page transition type rather than explicit."

This reverts commit 092dd8d9.

Reason for revert: Failing on device bots. See crbug.com/1061867

Original change's description:
> [AppLauncher] Look at core page transition type rather than explicit.
> 
> This allows page to open app URLs via JavaScript.
> 
> Bug: 1058388
> Change-Id: I3aabfab102c89156c342cf8f4fec72e69d69e2fc
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2087905
> Reviewed-by: Peter Lee <pkl@chromium.org>
> Commit-Queue: Sebastien Lalancette <seblalancette@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#750317}

TBR=pkl@chromium.org,seblalancette@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 1058388
Change-Id: I202ca98be0ab43a77dabd03d3e6e13a09c5f61a7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2102542Reviewed-by: default avatarGauthier Ambard <gambard@chromium.org>
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#750508}
parent 5fe08c11
...@@ -215,7 +215,7 @@ bool AppLauncherTabHelper::ShouldAllowRequest( ...@@ -215,7 +215,7 @@ bool AppLauncherTabHelper::ShouldAllowRequest(
web_state_->GetNavigationManager()->GetPendingItem(); web_state_->GetNavigationManager()->GetPendingItem();
GURL original_pending_url = GURL original_pending_url =
pending_item ? pending_item->GetOriginalRequestURL() : GURL::EmptyGURL(); pending_item ? pending_item->GetOriginalRequestURL() : GURL::EmptyGURL();
bool is_link_transition = ui::PageTransitionCoreTypeIs( bool is_link_transition = ui::PageTransitionTypeIncludingQualifiersIs(
request_info.transition_type, ui::PAGE_TRANSITION_LINK); request_info.transition_type, ui::PAGE_TRANSITION_LINK);
ChromeBrowserState* browser_state = ChromeBrowserState* browser_state =
......
...@@ -103,9 +103,7 @@ std::unique_ptr<KeyedService> BuildReadingListModel( ...@@ -103,9 +103,7 @@ std::unique_ptr<KeyedService> BuildReadingListModel(
} }
// Test fixture for AppLauncherTabHelper class. // Test fixture for AppLauncherTabHelper class.
class AppLauncherTabHelperTest class AppLauncherTabHelperTest : public PlatformTest {
: public PlatformTest,
public ::testing::WithParamInterface<ui::PageTransition> {
protected: protected:
AppLauncherTabHelperTest() AppLauncherTabHelperTest()
: abuse_detector_([[FakeAppLauncherAbuseDetector alloc] init]), : abuse_detector_([[FakeAppLauncherAbuseDetector alloc] init]),
...@@ -127,7 +125,8 @@ class AppLauncherTabHelperTest ...@@ -127,7 +125,8 @@ class AppLauncherTabHelperTest
bool has_user_gesture) WARN_UNUSED_RESULT { bool has_user_gesture) WARN_UNUSED_RESULT {
NSURL* url = [NSURL URLWithString:url_string]; NSURL* url = [NSURL URLWithString:url_string];
web::WebStatePolicyDecider::RequestInfo request_info( web::WebStatePolicyDecider::RequestInfo request_info(
GetParam(), target_frame_is_main, has_user_gesture); ui::PageTransition::PAGE_TRANSITION_LINK, target_frame_is_main,
has_user_gesture);
return tab_helper_->ShouldAllowRequest([NSURLRequest requestWithURL:url], return tab_helper_->ShouldAllowRequest([NSURLRequest requestWithURL:url],
request_info); request_info);
} }
...@@ -167,8 +166,9 @@ class AppLauncherTabHelperTest ...@@ -167,8 +166,9 @@ class AppLauncherTabHelperTest
abuse_detector_.policy = is_app_blocked ? ExternalAppLaunchPolicyBlock abuse_detector_.policy = is_app_blocked ? ExternalAppLaunchPolicyBlock
: ExternalAppLaunchPolicyAllow; : ExternalAppLaunchPolicyAllow;
ui::PageTransition transition_type = ui::PageTransition transition_type =
is_link_transition ? ui::PageTransition::PAGE_TRANSITION_LINK is_link_transition
: ui::PageTransition::PAGE_TRANSITION_TYPED; ? ui::PageTransition::PAGE_TRANSITION_LINK
: ui::PageTransition::PAGE_TRANSITION_CLIENT_REDIRECT;
NSURL* url = [NSURL NSURL* url = [NSURL
URLWithString:@"itms-apps://itunes.apple.com/us/app/appname/id123"]; URLWithString:@"itms-apps://itunes.apple.com/us/app/appname/id123"];
...@@ -194,7 +194,7 @@ class AppLauncherTabHelperTest ...@@ -194,7 +194,7 @@ class AppLauncherTabHelperTest
}; };
// Tests that a valid URL launches app. // Tests that a valid URL launches app.
TEST_P(AppLauncherTabHelperTest, AbuseDetectorPolicyAllowedForValidUrl) { TEST_F(AppLauncherTabHelperTest, AbuseDetectorPolicyAllowedForValidUrl) {
abuse_detector_.policy = ExternalAppLaunchPolicyAllow; abuse_detector_.policy = ExternalAppLaunchPolicyAllow;
EXPECT_FALSE(TestShouldAllowRequest(@"valid://1234", EXPECT_FALSE(TestShouldAllowRequest(@"valid://1234",
/*target_frame_is_main=*/true, /*target_frame_is_main=*/true,
...@@ -204,7 +204,7 @@ TEST_P(AppLauncherTabHelperTest, AbuseDetectorPolicyAllowedForValidUrl) { ...@@ -204,7 +204,7 @@ TEST_P(AppLauncherTabHelperTest, AbuseDetectorPolicyAllowedForValidUrl) {
} }
// Tests that a valid URL does not launch app when launch policy is to block. // Tests that a valid URL does not launch app when launch policy is to block.
TEST_P(AppLauncherTabHelperTest, AbuseDetectorPolicyBlockedForValidUrl) { TEST_F(AppLauncherTabHelperTest, AbuseDetectorPolicyBlockedForValidUrl) {
abuse_detector_.policy = ExternalAppLaunchPolicyBlock; abuse_detector_.policy = ExternalAppLaunchPolicyBlock;
EXPECT_FALSE(TestShouldAllowRequest(@"valid://1234", EXPECT_FALSE(TestShouldAllowRequest(@"valid://1234",
/*target_frame_is_main=*/true, /*target_frame_is_main=*/true,
...@@ -215,7 +215,7 @@ TEST_P(AppLauncherTabHelperTest, AbuseDetectorPolicyBlockedForValidUrl) { ...@@ -215,7 +215,7 @@ TEST_P(AppLauncherTabHelperTest, AbuseDetectorPolicyBlockedForValidUrl) {
// Tests that a valid URL shows an alert and launches app when launch policy is // Tests that a valid URL shows an alert and launches app when launch policy is
// to prompt and user accepts. // to prompt and user accepts.
TEST_P(AppLauncherTabHelperTest, ValidUrlPromptUserAccepts) { TEST_F(AppLauncherTabHelperTest, ValidUrlPromptUserAccepts) {
abuse_detector_.policy = ExternalAppLaunchPolicyPrompt; abuse_detector_.policy = ExternalAppLaunchPolicyPrompt;
delegate_.simulateUserAcceptingPrompt = YES; delegate_.simulateUserAcceptingPrompt = YES;
EXPECT_FALSE(TestShouldAllowRequest(@"valid://1234", EXPECT_FALSE(TestShouldAllowRequest(@"valid://1234",
...@@ -229,7 +229,7 @@ TEST_P(AppLauncherTabHelperTest, ValidUrlPromptUserAccepts) { ...@@ -229,7 +229,7 @@ TEST_P(AppLauncherTabHelperTest, ValidUrlPromptUserAccepts) {
// Tests that a valid URL does not launch app when launch policy is to prompt // Tests that a valid URL does not launch app when launch policy is to prompt
// and user rejects. // and user rejects.
TEST_P(AppLauncherTabHelperTest, ValidUrlPromptUserRejects) { TEST_F(AppLauncherTabHelperTest, ValidUrlPromptUserRejects) {
abuse_detector_.policy = ExternalAppLaunchPolicyPrompt; abuse_detector_.policy = ExternalAppLaunchPolicyPrompt;
delegate_.simulateUserAcceptingPrompt = NO; delegate_.simulateUserAcceptingPrompt = NO;
EXPECT_FALSE(TestShouldAllowRequest(@"valid://1234", EXPECT_FALSE(TestShouldAllowRequest(@"valid://1234",
...@@ -240,7 +240,7 @@ TEST_P(AppLauncherTabHelperTest, ValidUrlPromptUserRejects) { ...@@ -240,7 +240,7 @@ TEST_P(AppLauncherTabHelperTest, ValidUrlPromptUserRejects) {
// Tests that ShouldAllowRequest only launches apps for App Urls in main frame, // Tests that ShouldAllowRequest only launches apps for App Urls in main frame,
// or iframe when there was a recent user interaction. // or iframe when there was a recent user interaction.
TEST_P(AppLauncherTabHelperTest, ShouldAllowRequestWithAppUrl) { TEST_F(AppLauncherTabHelperTest, ShouldAllowRequestWithAppUrl) {
NSString* url_string = @"itms-apps://itunes.apple.com/us/app/appname/id123"; NSString* url_string = @"itms-apps://itunes.apple.com/us/app/appname/id123";
EXPECT_FALSE(TestShouldAllowRequest(url_string, /*target_frame_is_main=*/true, EXPECT_FALSE(TestShouldAllowRequest(url_string, /*target_frame_is_main=*/true,
/*has_user_gesture=*/false)); /*has_user_gesture=*/false));
...@@ -263,7 +263,7 @@ TEST_P(AppLauncherTabHelperTest, ShouldAllowRequestWithAppUrl) { ...@@ -263,7 +263,7 @@ TEST_P(AppLauncherTabHelperTest, ShouldAllowRequestWithAppUrl) {
// Tests that ShouldAllowRequest always allows requests and does not launch // Tests that ShouldAllowRequest always allows requests and does not launch
// apps for non App Urls. // apps for non App Urls.
TEST_P(AppLauncherTabHelperTest, ShouldAllowRequestWithNonAppUrl) { TEST_F(AppLauncherTabHelperTest, ShouldAllowRequestWithNonAppUrl) {
EXPECT_TRUE(TestShouldAllowRequest( EXPECT_TRUE(TestShouldAllowRequest(
@"http://itunes.apple.com/us/app/appname/id123", @"http://itunes.apple.com/us/app/appname/id123",
/*target_frame_is_main=*/true, /*has_user_gesture=*/false)); /*target_frame_is_main=*/true, /*has_user_gesture=*/false));
...@@ -283,7 +283,7 @@ TEST_P(AppLauncherTabHelperTest, ShouldAllowRequestWithNonAppUrl) { ...@@ -283,7 +283,7 @@ TEST_P(AppLauncherTabHelperTest, ShouldAllowRequestWithNonAppUrl) {
} }
// Tests that invalid Urls are completely blocked. // Tests that invalid Urls are completely blocked.
TEST_P(AppLauncherTabHelperTest, InvalidUrls) { TEST_F(AppLauncherTabHelperTest, InvalidUrls) {
EXPECT_FALSE(TestShouldAllowRequest(/*url_string=*/@"", EXPECT_FALSE(TestShouldAllowRequest(/*url_string=*/@"",
/*target_frame_is_main=*/true, /*target_frame_is_main=*/true,
/*has_user_gesture=*/false)); /*has_user_gesture=*/false));
...@@ -295,7 +295,7 @@ TEST_P(AppLauncherTabHelperTest, InvalidUrls) { ...@@ -295,7 +295,7 @@ TEST_P(AppLauncherTabHelperTest, InvalidUrls) {
// Tests that when the last committed URL is invalid, the URL is only opened // Tests that when the last committed URL is invalid, the URL is only opened
// when the last committed item is nil. // when the last committed item is nil.
TEST_P(AppLauncherTabHelperTest, ValidUrlInvalidCommittedURL) { TEST_F(AppLauncherTabHelperTest, ValidUrlInvalidCommittedURL) {
NSString* url_string = @"itms-apps://itunes.apple.com/us/app/appname/id123"; NSString* url_string = @"itms-apps://itunes.apple.com/us/app/appname/id123";
web_state_.SetCurrentURL(GURL()); web_state_.SetCurrentURL(GURL());
...@@ -316,7 +316,7 @@ TEST_P(AppLauncherTabHelperTest, ValidUrlInvalidCommittedURL) { ...@@ -316,7 +316,7 @@ TEST_P(AppLauncherTabHelperTest, ValidUrlInvalidCommittedURL) {
} }
// Tests that URLs with schemes that might be a security risk are blocked. // Tests that URLs with schemes that might be a security risk are blocked.
TEST_P(AppLauncherTabHelperTest, InsecureUrls) { TEST_F(AppLauncherTabHelperTest, InsecureUrls) {
EXPECT_FALSE(TestShouldAllowRequest(@"app-settings://", EXPECT_FALSE(TestShouldAllowRequest(@"app-settings://",
/*target_frame_is_main=*/true, /*target_frame_is_main=*/true,
/*has_user_gesture=*/false)); /*has_user_gesture=*/false));
...@@ -327,7 +327,7 @@ TEST_P(AppLauncherTabHelperTest, InsecureUrls) { ...@@ -327,7 +327,7 @@ TEST_P(AppLauncherTabHelperTest, InsecureUrls) {
// This test is using https://chromeiostesting-dot-u2fdemo.appspot.com URL which // This test is using https://chromeiostesting-dot-u2fdemo.appspot.com URL which
// is a URL allowed for the purpose of testing, but the test doesn't send any // is a URL allowed for the purpose of testing, but the test doesn't send any
// requests to the server. // requests to the server.
TEST_P(AppLauncherTabHelperTest, U2FUrls) { TEST_F(AppLauncherTabHelperTest, U2FUrls) {
// Add required tab helpers for the U2F check. // Add required tab helpers for the U2F check.
TabIdTabHelper::CreateForWebState(&web_state_); TabIdTabHelper::CreateForWebState(&web_state_);
std::unique_ptr<web::NavigationItem> item = web::NavigationItem::Create(); std::unique_ptr<web::NavigationItem> item = web::NavigationItem::Create();
...@@ -362,7 +362,7 @@ TEST_P(AppLauncherTabHelperTest, U2FUrls) { ...@@ -362,7 +362,7 @@ TEST_P(AppLauncherTabHelperTest, U2FUrls) {
} }
// Tests that URLs with Chrome Bundle schemes are blocked on iframes. // Tests that URLs with Chrome Bundle schemes are blocked on iframes.
TEST_P(AppLauncherTabHelperTest, ChromeBundleUrlScheme) { TEST_F(AppLauncherTabHelperTest, ChromeBundleUrlScheme) {
// Get the test bundle URL Scheme. // Get the test bundle URL Scheme.
NSString* scheme = [[ChromeAppConstants sharedInstance] getBundleURLScheme]; NSString* scheme = [[ChromeAppConstants sharedInstance] getBundleURLScheme];
NSString* url = [NSString stringWithFormat:@"%@://www.google.com", scheme]; NSString* url = [NSString stringWithFormat:@"%@://www.google.com", scheme];
...@@ -386,7 +386,7 @@ TEST_P(AppLauncherTabHelperTest, ChromeBundleUrlScheme) { ...@@ -386,7 +386,7 @@ TEST_P(AppLauncherTabHelperTest, ChromeBundleUrlScheme) {
// Tests that ShouldAllowRequest updates the reading list correctly for non-link // Tests that ShouldAllowRequest updates the reading list correctly for non-link
// transitions regardless of the app launching success when AppLauncherRefresh // transitions regardless of the app launching success when AppLauncherRefresh
// flag is enabled. // flag is enabled.
TEST_P(AppLauncherTabHelperTest, UpdatingTheReadingList) { TEST_F(AppLauncherTabHelperTest, UpdatingTheReadingList) {
// Update reading list if the transition is not a link transition. // Update reading list if the transition is not a link transition.
EXPECT_TRUE(TestReadingListUpdate(/*is_app_blocked=*/true, EXPECT_TRUE(TestReadingListUpdate(/*is_app_blocked=*/true,
/*is_link_transition*/ false, /*is_link_transition*/ false,
...@@ -410,13 +410,4 @@ TEST_P(AppLauncherTabHelperTest, UpdatingTheReadingList) { ...@@ -410,13 +410,4 @@ TEST_P(AppLauncherTabHelperTest, UpdatingTheReadingList) {
EXPECT_EQ(2U, delegate_.countOfAppsLaunched); EXPECT_EQ(2U, delegate_.countOfAppsLaunched);
} }
INSTANTIATE_TEST_SUITE_P(
/* no prefix */,
AppLauncherTabHelperTest,
::testing::Values(ui::PAGE_TRANSITION_LINK,
ui::PAGE_TRANSITION_LINK |
ui::PAGE_TRANSITION_CLIENT_REDIRECT,
ui::PAGE_TRANSITION_LINK |
ui::PAGE_TRANSITION_SERVER_REDIRECT));
} // namespace } // namespace
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