Skip to content

Commit

Permalink
[Autofill Assistant] Add missing entries to the TriggerUiType enum.
Browse files Browse the repository at this point in the history 
There should be no actual code changes in this CL, it's just renaming
and adding some new enum entries. See bug for details.

Corresponding backend change: cl/374389863

Bug: b/188408689
Change-Id: I0f58805448ce68f673bd8b8f25ea0711a2aa41c5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2901525
Commit-Queue: Clemens Arbesser <arbesser@google.com>
Reviewed-by: Sandro Maggi <sandromaggi@google.com>
Cr-Commit-Position: refs/heads/master@{#883952}
  • Loading branch information
Clemens Arbesser authored and Chromium LUCI CQ committed May 18, 2021
1 parent c50ec2f commit aa0f6e7
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 61 deletions.
20 changes: 12 additions & 8 deletions components/autofill_assistant/browser/service.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -610,16 +610,20 @@ message TriggerScriptProto {

// Explicit trigger requests by some external trigger surface, i.e., a link
// or button.
CART_FIRST_TIME_USER = 1;
CART_RETURNING_USER = 2;
CHECKOUT_FIRST_TIME_USER = 3;
CHECKOUT_RETURNING_USER = 4;
SHOPPING_CART_FIRST_TIME_USER = 1;
SHOPPING_CART_RETURNING_USER = 2;
SHOPPING_CHECKOUT_FIRST_TIME_USER = 3;
SHOPPING_CHECKOUT_RETURNING_USER = 4;
FOOD_ORDERING_CART_FIRST_TIME_USER = 9;
FOOD_ORDERING_CART_RETURNING_USER = 10;

// Implicit trigger requests started by Chrome itself.
IN_CHROME_CART_FIRST_TIME_USER = 5;
IN_CHROME_CART_RETURNING_USER = 6;
IN_CHROME_CHECKOUT_FIRST_TIME_USER = 7;
IN_CHROME_CHECKOUT_RETURNING_USER = 8;
IN_CHROME_SHOPPING_CART_FIRST_TIME_USER = 5;
IN_CHROME_SHOPPING_CART_RETURNING_USER = 6;
IN_CHROME_SHOPPING_CHECKOUT_FIRST_TIME_USER = 7;
IN_CHROME_SHOPPING_CHECKOUT_RETURNING_USER = 8;
IN_CHROME_FOOD_ORDERING_CART_FIRST_TIME_USER = 11;
IN_CHROME_FOOD_ORDERING_CART_RETURNING_USER = 12;
}

// The |trigger_condition| must be true for the script to trigger.
Expand Down
30 changes: 17 additions & 13 deletions components/autofill_assistant/browser/starter_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -686,20 +686,22 @@ TEST_F(StarterTest, RpcTriggerScriptSucceeds) {
EXPECT_THAT(request.url(), Eq(GURL(kExampleDeeplink)));
EXPECT_FALSE(request.client_context().is_in_chrome_triggered());
std::move(callback).Run(
net::HTTP_OK, CreateTriggerScriptResponseForTest(
TriggerScriptProto::CART_FIRST_TIME_USER));
net::HTTP_OK,
CreateTriggerScriptResponseForTest(
TriggerScriptProto::SHOPPING_CART_FIRST_TIME_USER));
}));
GetTriggerScriptsResponseProto get_trigger_scripts_response;
get_trigger_scripts_response.ParseFromString(
CreateTriggerScriptResponseForTest(
TriggerScriptProto::CART_FIRST_TIME_USER));
TriggerScriptProto::SHOPPING_CART_FIRST_TIME_USER));
EXPECT_CALL(
mock_start_regular_script_callback_,
Run(GURL(kExampleDeeplink),
Pointee(AllOf(Property(&TriggerContext::GetOnboardingShown, true),
Property(&TriggerContext::GetInChromeTriggered, false),
Property(&TriggerContext::GetTriggerUIType,
TriggerScriptProto::CART_FIRST_TIME_USER))),
Pointee(AllOf(
Property(&TriggerContext::GetOnboardingShown, true),
Property(&TriggerContext::GetInChromeTriggered, false),
Property(&TriggerContext::GetTriggerUIType,
TriggerScriptProto::SHOPPING_CART_FIRST_TIME_USER))),
Optional(get_trigger_scripts_response.trigger_scripts(0))));

starter_->Start(std::make_unique<TriggerContext>(
Expand Down Expand Up @@ -1037,8 +1039,9 @@ TEST_F(StarterTest, ImplicitStartupOnSupportedDomain) {
Eq(GURL("https://www.some-website.com/cart")));
EXPECT_TRUE(request.client_context().is_in_chrome_triggered());
std::move(callback).Run(
net::HTTP_OK, CreateTriggerScriptResponseForTest(
TriggerScriptProto::CART_RETURNING_USER));
net::HTTP_OK,
CreateTriggerScriptResponseForTest(
TriggerScriptProto::SHOPPING_CART_RETURNING_USER));
}));
EXPECT_CALL(*mock_trigger_script_ui_delegate_, ShowTriggerScript)
.WillOnce([&]() {
Expand All @@ -1049,10 +1052,11 @@ TEST_F(StarterTest, ImplicitStartupOnSupportedDomain) {
EXPECT_CALL(
mock_start_regular_script_callback_,
Run(GURL("https://www.some-website.com/cart"),
Pointee(AllOf(Property(&TriggerContext::GetOnboardingShown, false),
Property(&TriggerContext::GetInChromeTriggered, true),
Property(&TriggerContext::GetTriggerUIType,
TriggerScriptProto::CART_RETURNING_USER))),
Pointee(AllOf(
Property(&TriggerContext::GetOnboardingShown, false),
Property(&TriggerContext::GetInChromeTriggered, true),
Property(&TriggerContext::GetTriggerUIType,
TriggerScriptProto::SHOPPING_CART_RETURNING_USER))),
testing::Ne(absl::nullopt)));

// Implicit startup by navigating to an autofill-assistant-enabled site.
Expand Down
9 changes: 5 additions & 4 deletions components/autofill_assistant/browser/trigger_context_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,9 @@ TEST(TriggerContextTest, Create) {

context.SetOnboardingShown(false);
EXPECT_FALSE(context.GetOnboardingShown());
context.SetTriggerUIType(TriggerScriptProto::CART_FIRST_TIME_USER);
context.SetTriggerUIType(TriggerScriptProto::SHOPPING_CART_FIRST_TIME_USER);
EXPECT_EQ(context.GetTriggerUIType(),
TriggerScriptProto::CART_FIRST_TIME_USER);
TriggerScriptProto::SHOPPING_CART_FIRST_TIME_USER);
}

TEST(TriggerContextTest, MergeEmpty) {
Expand Down Expand Up @@ -104,7 +104,8 @@ TEST(TriggerContextTest, MergeNonEmptyWithNonEmpty) {
/* is_direct_action = */ true,
/* initial_url = */ "https://www.example.com",
/* is_in_chrome_triggered = */ true};
context2.SetTriggerUIType(TriggerScriptProto::CHECKOUT_FIRST_TIME_USER);
context2.SetTriggerUIType(
TriggerScriptProto::SHOPPING_CHECKOUT_FIRST_TIME_USER);

// Adding empty to make sure empty contexts are properly skipped.
TriggerContext empty;
Expand All @@ -119,7 +120,7 @@ TEST(TriggerContextTest, MergeNonEmptyWithNonEmpty) {
EXPECT_EQ(merged.GetInitialUrl(), "https://www.example.com");
EXPECT_TRUE(merged.GetInChromeTriggered());
EXPECT_EQ(merged.GetTriggerUIType(),
TriggerScriptProto::CHECKOUT_FIRST_TIME_USER);
TriggerScriptProto::SHOPPING_CHECKOUT_FIRST_TIME_USER);
}

TEST(TriggerContextTest, HasExperimentId) {
Expand Down
58 changes: 30 additions & 28 deletions components/autofill_assistant/browser/trigger_scripts/trigger_script_coordinator_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -427,7 +427,7 @@ TEST_F(TriggerScriptCoordinatorTest, PerformTriggerScriptActionCancelSession) {
TriggerScriptProto* script = response.add_trigger_scripts();
*script->mutable_trigger_condition()->mutable_selector() =
ToSelectorProto("#selector");
script->set_trigger_ui_type(TriggerScriptProto::CART_RETURNING_USER);
script->set_trigger_ui_type(TriggerScriptProto::SHOPPING_CART_RETURNING_USER);
std::string serialized_response;
response.SerializeToString(&serialized_response);

Expand All @@ -448,7 +448,7 @@ TEST_F(TriggerScriptCoordinatorTest, PerformTriggerScriptActionCancelSession) {
EXPECT_CALL(*mock_ui_delegate_, HideTriggerScript).Times(1);
coordinator_->PerformTriggerScriptAction(TriggerScriptProto::CANCEL_SESSION);
AssertRecordedFinishedState(
TriggerScriptProto::CART_RETURNING_USER,
TriggerScriptProto::SHOPPING_CART_RETURNING_USER,
Metrics::TriggerScriptFinishedState::PROMPT_FAILED_CANCEL_SESSION);
}

Expand All @@ -457,7 +457,7 @@ TEST_F(TriggerScriptCoordinatorTest, PerformTriggerScriptActionCancelForever) {
TriggerScriptProto* script = response.add_trigger_scripts();
*script->mutable_trigger_condition()->mutable_selector() =
ToSelectorProto("#selector");
script->set_trigger_ui_type(TriggerScriptProto::CART_RETURNING_USER);
script->set_trigger_ui_type(TriggerScriptProto::SHOPPING_CART_RETURNING_USER);
std::string serialized_response;
response.SerializeToString(&serialized_response);

Expand All @@ -478,7 +478,7 @@ TEST_F(TriggerScriptCoordinatorTest, PerformTriggerScriptActionCancelForever) {
EXPECT_CALL(*mock_ui_delegate_, HideTriggerScript).Times(1);
coordinator_->PerformTriggerScriptAction(TriggerScriptProto::CANCEL_FOREVER);
AssertRecordedFinishedState(
TriggerScriptProto::CART_RETURNING_USER,
TriggerScriptProto::SHOPPING_CART_RETURNING_USER,
Metrics::TriggerScriptFinishedState::PROMPT_FAILED_CANCEL_FOREVER);
}

Expand All @@ -487,7 +487,8 @@ TEST_F(TriggerScriptCoordinatorTest, PerformTriggerScriptActionAccept) {
TriggerScriptProto* script = response.add_trigger_scripts();
*script->mutable_trigger_condition()->mutable_selector() =
ToSelectorProto("#selector");
script->set_trigger_ui_type(TriggerScriptProto::CHECKOUT_RETURNING_USER);
script->set_trigger_ui_type(
TriggerScriptProto::SHOPPING_CHECKOUT_RETURNING_USER);
std::string serialized_response;
response.SerializeToString(&serialized_response);

Expand All @@ -510,7 +511,7 @@ TEST_F(TriggerScriptCoordinatorTest, CancelOnNavigateAway) {
TriggerScriptProto* script = response.add_trigger_scripts();
*script->mutable_trigger_condition()->mutable_selector() =
ToSelectorProto("#selector");
script->set_trigger_ui_type(TriggerScriptProto::CART_RETURNING_USER);
script->set_trigger_ui_type(TriggerScriptProto::SHOPPING_CART_RETURNING_USER);
std::string serialized_response;
response.SerializeToString(&serialized_response);

Expand Down Expand Up @@ -543,7 +544,7 @@ TEST_F(TriggerScriptCoordinatorTest, CancelOnNavigateAway) {
Run(Metrics::TriggerScriptFinishedState::PROMPT_FAILED_NAVIGATE, _, _));
SimulateNavigateToUrl(GURL("https://example.different.com/page"));
AssertRecordedFinishedState(
TriggerScriptProto::CART_RETURNING_USER,
TriggerScriptProto::SHOPPING_CART_RETURNING_USER,
Metrics::TriggerScriptFinishedState::PROMPT_FAILED_NAVIGATE);
}

Expand All @@ -552,7 +553,7 @@ TEST_F(TriggerScriptCoordinatorTest, IgnoreNavigationEventsWhileNotStarted) {
TriggerScriptProto* script = response.add_trigger_scripts();
*script->mutable_trigger_condition()->mutable_selector() =
ToSelectorProto("#selector");
script->set_trigger_ui_type(TriggerScriptProto::CART_RETURNING_USER);
script->set_trigger_ui_type(TriggerScriptProto::SHOPPING_CART_RETURNING_USER);
std::string serialized_response;
response.SerializeToString(&serialized_response);

Expand Down Expand Up @@ -596,7 +597,7 @@ TEST_F(TriggerScriptCoordinatorTest, BottomSheetClosedWithSwipe) {
GetTriggerScriptsResponseProto response;
TriggerScriptProto* script = response.add_trigger_scripts();
script->set_on_swipe_to_dismiss(TriggerScriptProto::NOT_NOW);
script->set_trigger_ui_type(TriggerScriptProto::CART_RETURNING_USER);
script->set_trigger_ui_type(TriggerScriptProto::SHOPPING_CART_RETURNING_USER);
std::string serialized_response;
response.SerializeToString(&serialized_response);

Expand All @@ -611,7 +612,7 @@ TEST_F(TriggerScriptCoordinatorTest, BottomSheetClosedWithSwipe) {
EXPECT_CALL(*mock_ui_delegate_, HideTriggerScript).Times(1);
coordinator_->OnBottomSheetClosedWithSwipe();
AssertRecordedShownToUserState(
TriggerScriptProto::CART_RETURNING_USER,
TriggerScriptProto::SHOPPING_CART_RETURNING_USER,
Metrics::TriggerScriptShownToUser::SWIPE_DISMISSED, 1);
}

Expand All @@ -620,7 +621,8 @@ TEST_F(TriggerScriptCoordinatorTest, TimeoutAfterInvisibleForTooLong) {
TriggerScriptProto* script = response.add_trigger_scripts();
*script->mutable_trigger_condition()->mutable_selector() =
ToSelectorProto("#selector");
script->set_trigger_ui_type(TriggerScriptProto::CHECKOUT_RETURNING_USER);
script->set_trigger_ui_type(
TriggerScriptProto::SHOPPING_CHECKOUT_RETURNING_USER);
response.set_timeout_ms(3000);
response.set_trigger_condition_check_interval_ms(1000);
std::string serialized_response;
Expand Down Expand Up @@ -657,7 +659,7 @@ TEST_F(TriggerScriptCoordinatorTest, TimeoutResetsAfterTriggerScriptShown) {
TriggerScriptProto* script = response.add_trigger_scripts();
*script->mutable_trigger_condition()->mutable_selector() =
ToSelectorProto("#selector");
script->set_trigger_ui_type(TriggerScriptProto::CART_RETURNING_USER);
script->set_trigger_ui_type(TriggerScriptProto::SHOPPING_CART_RETURNING_USER);
response.set_timeout_ms(3000);
response.set_trigger_condition_check_interval_ms(1000);
std::string serialized_response;
Expand Down Expand Up @@ -861,7 +863,7 @@ TEST_F(TriggerScriptCoordinatorTest,
TEST_F(TriggerScriptCoordinatorTest, OnTriggerScriptFailedToShow) {
GetTriggerScriptsResponseProto response;
TriggerScriptProto* script = response.add_trigger_scripts();
script->set_trigger_ui_type(TriggerScriptProto::CART_RETURNING_USER);
script->set_trigger_ui_type(TriggerScriptProto::SHOPPING_CART_RETURNING_USER);
std::string serialized_response;
response.SerializeToString(&serialized_response);

Expand All @@ -879,7 +881,7 @@ TEST_F(TriggerScriptCoordinatorTest, OnTriggerScriptFailedToShow) {
coordinator_->Start(GURL(kFakeDeepLink), std::make_unique<TriggerContext>(),
mock_callback_.Get());
AssertRecordedFinishedState(
TriggerScriptProto::CART_RETURNING_USER,
TriggerScriptProto::SHOPPING_CART_RETURNING_USER,
Metrics::TriggerScriptFinishedState::FAILED_TO_SHOW);
}

Expand Down Expand Up @@ -956,7 +958,7 @@ TEST_F(TriggerScriptCoordinatorTest, PauseAndResumeOnTabSwitch) {
TEST_F(TriggerScriptCoordinatorTest, OnboardingShownAndAccepted) {
GetTriggerScriptsResponseProto response;
auto* script = response.add_trigger_scripts();
script->set_trigger_ui_type(TriggerScriptProto::CART_RETURNING_USER);
script->set_trigger_ui_type(TriggerScriptProto::SHOPPING_CART_RETURNING_USER);
std::string serialized_response;
response.SerializeToString(&serialized_response);

Expand All @@ -979,10 +981,10 @@ TEST_F(TriggerScriptCoordinatorTest, OnboardingShownAndAccepted) {

EXPECT_THAT(fake_platform_delegate_.num_show_onboarding_called_, Eq(1));
AssertRecordedTriggerScriptOnboardingState(
TriggerScriptProto::CART_RETURNING_USER,
TriggerScriptProto::SHOPPING_CART_RETURNING_USER,
Metrics::TriggerScriptOnboarding::ONBOARDING_SEEN_AND_ACCEPTED, 1);
AssertRecordedFinishedState(
TriggerScriptProto::CART_RETURNING_USER,
TriggerScriptProto::SHOPPING_CART_RETURNING_USER,
Metrics::TriggerScriptFinishedState::PROMPT_SUCCEEDED);
}

Expand All @@ -992,7 +994,7 @@ TEST_F(TriggerScriptCoordinatorTest,

GetTriggerScriptsResponseProto response;
auto* script = response.add_trigger_scripts();
script->set_trigger_ui_type(TriggerScriptProto::CART_RETURNING_USER);
script->set_trigger_ui_type(TriggerScriptProto::SHOPPING_CART_RETURNING_USER);
std::string serialized_response;
response.SerializeToString(&serialized_response);

Expand Down Expand Up @@ -1030,18 +1032,18 @@ TEST_F(TriggerScriptCoordinatorTest,

EXPECT_THAT(fake_platform_delegate_.num_show_onboarding_called_, Eq(4));
AssertRecordedTriggerScriptOnboardingState(
TriggerScriptProto::CART_RETURNING_USER,
TriggerScriptProto::SHOPPING_CART_RETURNING_USER,
Metrics::TriggerScriptOnboarding::ONBOARDING_SEEN_AND_REJECTED, 1);
AssertRecordedTriggerScriptOnboardingState(
TriggerScriptProto::CART_RETURNING_USER,
TriggerScriptProto::SHOPPING_CART_RETURNING_USER,
Metrics::TriggerScriptOnboarding::ONBOARDING_SEEN_AND_ACCEPTED, 1);
AssertRecordedTriggerScriptOnboardingState(
TriggerScriptProto::CART_RETURNING_USER,
TriggerScriptProto::SHOPPING_CART_RETURNING_USER,
Metrics::TriggerScriptOnboarding::
ONBOARDING_SEEN_AND_INTERRUPTED_BY_NAVIGATION,
1);
AssertRecordedFinishedState(
TriggerScriptProto::CART_RETURNING_USER,
TriggerScriptProto::SHOPPING_CART_RETURNING_USER,
Metrics::TriggerScriptFinishedState::PROMPT_SUCCEEDED);
}

Expand All @@ -1051,7 +1053,7 @@ TEST_F(TriggerScriptCoordinatorTest,

GetTriggerScriptsResponseProto response;
auto* script = response.add_trigger_scripts();
script->set_trigger_ui_type(TriggerScriptProto::CART_RETURNING_USER);
script->set_trigger_ui_type(TriggerScriptProto::SHOPPING_CART_RETURNING_USER);
std::string serialized_response;
response.SerializeToString(&serialized_response);

Expand All @@ -1076,17 +1078,17 @@ TEST_F(TriggerScriptCoordinatorTest,

EXPECT_THAT(fake_platform_delegate_.num_show_onboarding_called_, Eq(1));
AssertRecordedTriggerScriptOnboardingState(
TriggerScriptProto::CART_RETURNING_USER,
TriggerScriptProto::SHOPPING_CART_RETURNING_USER,
Metrics::TriggerScriptOnboarding::ONBOARDING_SEEN_AND_REJECTED, 1);
AssertRecordedFinishedState(
TriggerScriptProto::CART_RETURNING_USER,
TriggerScriptProto::SHOPPING_CART_RETURNING_USER,
Metrics::TriggerScriptFinishedState::BOTTOMSHEET_ONBOARDING_REJECTED);
}

TEST_F(TriggerScriptCoordinatorTest, OnboardingNotShown) {
GetTriggerScriptsResponseProto response;
auto* script = response.add_trigger_scripts();
script->set_trigger_ui_type(TriggerScriptProto::CART_RETURNING_USER);
script->set_trigger_ui_type(TriggerScriptProto::SHOPPING_CART_RETURNING_USER);
std::string serialized_response;
response.SerializeToString(&serialized_response);

Expand All @@ -1107,10 +1109,10 @@ TEST_F(TriggerScriptCoordinatorTest, OnboardingNotShown) {
coordinator_->PerformTriggerScriptAction(TriggerScriptProto::ACCEPT);

AssertRecordedTriggerScriptOnboardingState(
TriggerScriptProto::CART_RETURNING_USER,
TriggerScriptProto::SHOPPING_CART_RETURNING_USER,
Metrics::TriggerScriptOnboarding::ONBOARDING_ALREADY_ACCEPTED, 1);
AssertRecordedFinishedState(
TriggerScriptProto::CART_RETURNING_USER,
TriggerScriptProto::SHOPPING_CART_RETURNING_USER,
Metrics::TriggerScriptFinishedState::PROMPT_SUCCEEDED);
}

Expand Down
20 changes: 12 additions & 8 deletions tools/metrics/histograms/enums.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5142,14 +5142,18 @@ others/histograms.xml -->

<enum name="AutofillAssistantTriggerUIType">
<int value="0" label="UNSPECIFIED_TRIGGER_UI_TYPE"/>
<int value="1" label="CART_FIRST_TIME_USER"/>
<int value="2" label="CART_RETURNING_USER"/>
<int value="3" label="CHECKOUT_FIRST_TIME_USER"/>
<int value="4" label="CHECKOUT_RETURNING_TIME_USER"/>
<int value="5" label="IN_CHROME_CART_FIRST_TIME_USER"/>
<int value="6" label="IN_CHROME_CART_RETURNING_USER"/>
<int value="7" label="IN_CHROME_CHECKOUT_FIRST_TIME_USER"/>
<int value="8" label="IN_CHROME_CHECKOUT_RETURNING_USER"/>
<int value="1" label="(External) Shopping cart, first time user"/>
<int value="2" label="(External) Shopping cart, returning user"/>
<int value="3" label="(External) Shopping checkout, first time user"/>
<int value="4" label="(External) Shopping checkout, returning user"/>
<int value="5" label="(In-Chrome) Shopping cart, first time user"/>
<int value="6" label="(In-Chrome) Shopping cart, returning user"/>
<int value="7" label="(In-Chrome) Shopping checkout, first time user"/>
<int value="8" label="(In-Chrome) Shopping checkout, returning user"/>
<int value="9" label="(External) Food ordering cart, first time user"/>
<int value="10" label="(External) Food ordering cart, returning user"/>
<int value="11" label="(In-Chrome) Food ordering cart, first time user"/>
<int value="12" label="(In-Chrome) Food ordering cart, returning user"/>
</enum>

<enum name="AutofillAwGSuggestionAvailability">
Expand Down

0 comments on commit aa0f6e7

Please sign in to comment.