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

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

Take 2 of the change, original CL was reverted due to tests flakiness:
https://chromium-review.googlesource.com/c/chromium/src/+/2102542

In this new change, we're making less drastic changes to existing tests.
Rather, we add a new test case which covers exactly the bug's symptoms -
making sure its fix won't be regressed in the future.

Tests flakiness was occurring in the iphone-device builder. I've tested
this new patch against that builder ~3 times now and it looks much better
than when using the previous patch.

Bug: 1058388
Change-Id: I459f8c3f66e7ac2d6c11232a75ad95b8c6026cae
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2106802
Commit-Queue: Sebastien Lalancette <seblalancette@chromium.org>
Auto-Submit: Sebastien Lalancette <seblalancette@chromium.org>
Reviewed-by: default avatarPeter Lee <pkl@chromium.org>
Cr-Commit-Position: refs/heads/master@{#751161}
parent 601571f8
......@@ -215,7 +215,7 @@ bool AppLauncherTabHelper::ShouldAllowRequest(
web_state_->GetNavigationManager()->GetPendingItem();
GURL original_pending_url =
pending_item ? pending_item->GetOriginalRequestURL() : GURL::EmptyGURL();
bool is_link_transition = ui::PageTransitionTypeIncludingQualifiersIs(
bool is_link_transition = ui::PageTransitionCoreTypeIs(
request_info.transition_type, ui::PAGE_TRANSITION_LINK);
ChromeBrowserState* browser_state =
......
......@@ -122,11 +122,13 @@ class AppLauncherTabHelperTest : public PlatformTest {
bool TestShouldAllowRequest(NSString* url_string,
bool target_frame_is_main,
bool has_user_gesture) WARN_UNUSED_RESULT {
bool has_user_gesture,
ui::PageTransition transition_type =
ui::PageTransition::PAGE_TRANSITION_LINK)
WARN_UNUSED_RESULT {
NSURL* url = [NSURL URLWithString:url_string];
web::WebStatePolicyDecider::RequestInfo request_info(
ui::PageTransition::PAGE_TRANSITION_LINK, target_frame_is_main,
has_user_gesture);
transition_type, target_frame_is_main, has_user_gesture);
return tab_helper_->ShouldAllowRequest([NSURLRequest requestWithURL:url],
request_info);
}
......@@ -166,9 +168,8 @@ class AppLauncherTabHelperTest : public PlatformTest {
abuse_detector_.policy = is_app_blocked ? ExternalAppLaunchPolicyBlock
: ExternalAppLaunchPolicyAllow;
ui::PageTransition transition_type =
is_link_transition
? ui::PageTransition::PAGE_TRANSITION_LINK
: ui::PageTransition::PAGE_TRANSITION_CLIENT_REDIRECT;
is_link_transition ? ui::PageTransition::PAGE_TRANSITION_LINK
: ui::PageTransition::PAGE_TRANSITION_TYPED;
NSURL* url = [NSURL
URLWithString:@"itms-apps://itunes.apple.com/us/app/appname/id123"];
......@@ -410,4 +411,17 @@ TEST_F(AppLauncherTabHelperTest, UpdatingTheReadingList) {
EXPECT_EQ(2U, delegate_.countOfAppsLaunched);
}
// Tests that launching a SMS URL via a JavaScript redirect in the main frame
// is allowed. Covers the scenario for crbug.com/1058388
TEST_F(AppLauncherTabHelperTest, LaunchSmsApp_JavaScriptRedirect) {
NSString* sms_url_string = @"sms:?&body=Hello%20World";
ui::PageTransition page_transition = ui::PageTransitionFromInt(
ui::PageTransition::PAGE_TRANSITION_LINK |
ui::PageTransition::PAGE_TRANSITION_CLIENT_REDIRECT);
EXPECT_FALSE(
TestShouldAllowRequest(sms_url_string, /*target_frame_is_main=*/true,
/*has_user_gesture=*/false, page_transition));
EXPECT_EQ(1U, delegate_.countOfAppsLaunched);
}
} // 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