Commit 5b09754f authored by Clemens Arbesser's avatar Clemens Arbesser Committed by Chromium LUCI CQ

[Autofill Assistant] Remove initial domain check for trigger scripts.

Some trigger sites will not directly navigate to the desired target
domain, but will instead use a redirect to eventually arrive at the
target domain. This resulted in an early error for trigger scripts.

This CL removes the initial domain check. This should be a safe change
because we own and trust the trigger sites. The result is that the
initial redirect (if any) is ignored. All subsequent navigations are
handled as before.

Bug: b/175132318
Change-Id: Ie52271eb0bf70b8372d3290c9e6c174f3404a6ec
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2593102
Commit-Queue: Clemens Arbesser <arbesser@google.com>
Reviewed-by: default avatarMarian Fechete <marianfe@google.com>
Cr-Commit-Position: refs/heads/master@{#838005}
parent ec279be9
......@@ -75,12 +75,6 @@ void TriggerScriptCoordinator::Start(
const GURL& deeplink_url,
std::unique_ptr<TriggerContext> trigger_context) {
deeplink_url_ = deeplink_url;
if (!url_utils::IsInDomainOrSubDomain(GetCurrentURL(), deeplink_url_)) {
LOG(ERROR) << "Trigger script requested for domain other than the deeplink";
Stop(Metrics::LiteScriptFinishedState::LITE_SCRIPT_PROMPT_FAILED_NAVIGATE);
return;
}
trigger_context_ = std::make_unique<TriggerContextImpl>(
ExtractDebugScriptParameters(*trigger_context),
trigger_context->experiment_ids());
......
......@@ -42,7 +42,6 @@ class MockObserver : public TriggerScriptCoordinator::Observer {
};
const char kFakeDeepLink[] = "https://example.com/q?data=test";
const char kFakeSubdomainDeepLink[] = "https://b.example.com";
const char kFakeServerUrl[] =
"https://www.fake.backend.com/trigger_script_server";
......@@ -143,39 +142,6 @@ class TriggerScriptCoordinatorTest : public content::RenderViewHostTestHarness {
NiceMock<MockDynamicTriggerConditions>* mock_dynamic_trigger_conditions_;
};
TEST_F(TriggerScriptCoordinatorTest, StartSucceedsForCorrectDomain) {
SimulateNavigateToUrl(GURL(kFakeDeepLink));
EXPECT_CALL(*mock_request_sender_, OnSendRequest).Times(1);
coordinator_->Start(GURL(kFakeDeepLink),
std::make_unique<TriggerContextImpl>());
}
TEST_F(TriggerScriptCoordinatorTest, StartSucceedsWhileNoCommittedURL) {
content::WebContentsTester::For(web_contents())->SetLastCommittedURL(GURL());
EXPECT_CALL(*mock_request_sender_, OnSendRequest).Times(1);
coordinator_->Start(GURL(kFakeDeepLink),
std::make_unique<TriggerContextImpl>());
}
TEST_F(TriggerScriptCoordinatorTest, StartSucceedsForSubDomain) {
SimulateNavigateToUrl(GURL(kFakeSubdomainDeepLink));
EXPECT_CALL(*mock_request_sender_, OnSendRequest).Times(1);
coordinator_->Start(GURL(kFakeDeepLink),
std::make_unique<TriggerContextImpl>());
}
TEST_F(TriggerScriptCoordinatorTest, StartFailsForDifferentDomain) {
SimulateNavigateToUrl(GURL("https://different.com/example"));
EXPECT_CALL(*mock_request_sender_, OnSendRequest).Times(0);
EXPECT_CALL(mock_observer_,
OnTriggerScriptFinished(Metrics::LiteScriptFinishedState::
LITE_SCRIPT_PROMPT_FAILED_NAVIGATE));
coordinator_->Start(GURL(kFakeDeepLink),
std::make_unique<TriggerContextImpl>());
AssertRecordedFinishedState(
Metrics::LiteScriptFinishedState::LITE_SCRIPT_PROMPT_FAILED_NAVIGATE);
}
TEST_F(TriggerScriptCoordinatorTest, StartSendsOnlyApprovedFields) {
std::map<std::string, std::string> input_script_params{
{"keyA", "valueA"},
......@@ -568,18 +534,21 @@ TEST_F(TriggerScriptCoordinatorTest, IgnoreNavigationEventsWhileNotStarted) {
OnUpdate(mock_web_controller_, _))
.Times(0);
SimulateWebContentsVisibilityChanged(content::Visibility::HIDDEN);
// Note: in reality, it should be impossible to navigate on hidden tabs.
SimulateNavigateToUrl(GURL("https://example.different.com"));
SimulateNavigateToUrl(GURL("https://also-not-supported.com"));
// However, when the tab becomes visible again, resuming the trigger script
// will fail if the last committed URL is still on a non-supported domain.
EXPECT_CALL(
mock_observer_,
OnTriggerScriptFinished(
Metrics::LiteScriptFinishedState::LITE_SCRIPT_PROMPT_FAILED_NAVIGATE))
EXPECT_CALL(*mock_request_sender_, OnSendRequest(GURL(kFakeServerUrl), _, _))
.WillOnce(RunOnceCallback<2>(net::HTTP_OK, /* response = */ ""));
// However, when the tab becomes visible again, the trigger script is
// restarted and thus fails if the tab is still on an unsupported domain.
EXPECT_CALL(mock_observer_, OnTriggerScriptFinished(
Metrics::LiteScriptFinishedState::
LITE_SCRIPT_NO_TRIGGER_SCRIPT_AVAILABLE))
.Times(1);
SimulateWebContentsVisibilityChanged(content::Visibility::VISIBLE);
AssertRecordedFinishedState(
Metrics::LiteScriptFinishedState::LITE_SCRIPT_PROMPT_FAILED_NAVIGATE);
AssertRecordedFinishedState(Metrics::LiteScriptFinishedState::
LITE_SCRIPT_NO_TRIGGER_SCRIPT_AVAILABLE);
}
TEST_F(TriggerScriptCoordinatorTest, BottomSheetClosedWithSwipe) {
......
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