Commit 00b70ebf authored by Siyu An's avatar Siyu An Committed by Commit Bot

[Sticky Bubbles] Add logging for unknown bubble closing reason

We found in the metrics the number of (bubble shown) and number of
(bubble closed with reason) are not matched. I am suspecting in some cases
it will be logged before the close_reason is returned, and in this case
the reason will be unknown[1].

[1]https://source.chromium.org/chromium/chromium/src/+/master:chrome/browser/ui/views/autofill/payments/save_card_bubble_views.cc;l=123-134;drc=d9daac114a9f5c8a7bc99042d92f40dcd62c3d08

Added a logging to verify

Bug: 1070799
Change-Id: I8a0cb2eb4c4158d11fcd6593777ad568947812d2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2441014
Commit-Queue: Siyu An <siyua@chromium.org>
Reviewed-by: default avatarJared Saul <jsaul@google.com>
Reviewed-by: default avatarEvan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#812574}
parent bc802c69
...@@ -112,30 +112,29 @@ void LocalCardMigrationBubbleControllerImpl::OnBubbleClosed( ...@@ -112,30 +112,29 @@ void LocalCardMigrationBubbleControllerImpl::OnBubbleClosed(
// Log local card migration bubble result according to the closed reason. // Log local card migration bubble result according to the closed reason.
if (base::FeatureList::IsEnabled( if (base::FeatureList::IsEnabled(
features::kAutofillEnableFixedPaymentsBubbleLogging)) { features::kAutofillEnableFixedPaymentsBubbleLogging)) {
AutofillMetrics::LocalCardMigrationBubbleResultMetric metric;
switch (closed_reason) { switch (closed_reason) {
case PaymentsBubbleClosedReason::kAccepted: case PaymentsBubbleClosedReason::kAccepted:
AutofillMetrics::LogLocalCardMigrationBubbleResultMetric( metric = AutofillMetrics::LOCAL_CARD_MIGRATION_BUBBLE_ACCEPTED;
AutofillMetrics::LOCAL_CARD_MIGRATION_BUBBLE_ACCEPTED, is_reshow_); break;
return;
case PaymentsBubbleClosedReason::kClosed: case PaymentsBubbleClosedReason::kClosed:
AutofillMetrics::LogLocalCardMigrationBubbleResultMetric( metric = AutofillMetrics::LOCAL_CARD_MIGRATION_BUBBLE_CLOSED;
AutofillMetrics::LOCAL_CARD_MIGRATION_BUBBLE_CLOSED, is_reshow_); break;
return;
case PaymentsBubbleClosedReason::kNotInteracted: case PaymentsBubbleClosedReason::kNotInteracted:
AutofillMetrics::LogLocalCardMigrationBubbleResultMetric( metric = AutofillMetrics::LOCAL_CARD_MIGRATION_BUBBLE_NOT_INTERACTED;
AutofillMetrics::LOCAL_CARD_MIGRATION_BUBBLE_NOT_INTERACTED, break;
is_reshow_);
return;
case PaymentsBubbleClosedReason::kLostFocus: case PaymentsBubbleClosedReason::kLostFocus:
AutofillMetrics::LogLocalCardMigrationBubbleResultMetric( metric = AutofillMetrics::LOCAL_CARD_MIGRATION_BUBBLE_LOST_FOCUS;
AutofillMetrics::LOCAL_CARD_MIGRATION_BUBBLE_LOST_FOCUS, break;
is_reshow_);
return;
case PaymentsBubbleClosedReason::kUnknown: case PaymentsBubbleClosedReason::kUnknown:
metric = AutofillMetrics::LOCAL_CARD_MIGRATION_BUBBLE_RESULT_UNKNOWN;
break;
case PaymentsBubbleClosedReason::kCancelled: case PaymentsBubbleClosedReason::kCancelled:
NOTREACHED(); NOTREACHED();
return; return;
} }
AutofillMetrics::LogLocalCardMigrationBubbleResultMetric(metric,
is_reshow_);
} }
} }
......
...@@ -387,6 +387,21 @@ TEST_P(LocalCardMigrationBubbleLoggingTest, FirstShow_BubbleLostFocus) { ...@@ -387,6 +387,21 @@ TEST_P(LocalCardMigrationBubbleLoggingTest, FirstShow_BubbleLostFocus) {
} }
} }
TEST_P(LocalCardMigrationBubbleLoggingTest, FirstShow_Unknown) {
base::HistogramTester histogram_tester;
ShowBubble();
CloseBubble(PaymentsBubbleClosedReason::kUnknown);
if (GetParam()) {
histogram_tester.ExpectUniqueSample(
"Autofill.LocalCardMigrationBubbleResult.FirstShow",
AutofillMetrics::LOCAL_CARD_MIGRATION_BUBBLE_RESULT_UNKNOWN, 1);
} else {
histogram_tester.ExpectTotalCount(
"Autofill.LocalCardMigrationBubbleResult.FirstShow", 0);
}
}
TEST_P(LocalCardMigrationBubbleLoggingTest, Reshows_BubbleAccepted) { TEST_P(LocalCardMigrationBubbleLoggingTest, Reshows_BubbleAccepted) {
base::HistogramTester histogram_tester; base::HistogramTester histogram_tester;
ShowBubble(); ShowBubble();
...@@ -471,6 +486,27 @@ TEST_P(LocalCardMigrationBubbleLoggingTest, Reshows_BubbleLostFocus) { ...@@ -471,6 +486,27 @@ TEST_P(LocalCardMigrationBubbleLoggingTest, Reshows_BubbleLostFocus) {
} }
} }
TEST_P(LocalCardMigrationBubbleLoggingTest, Reshows_Unknown) {
base::HistogramTester histogram_tester;
ShowBubble();
CloseAndReshowBubble();
CloseBubble(PaymentsBubbleClosedReason::kUnknown);
if (GetParam()) {
histogram_tester.ExpectUniqueSample(
"Autofill.LocalCardMigrationBubbleResult.FirstShow",
AutofillMetrics::LOCAL_CARD_MIGRATION_BUBBLE_NOT_INTERACTED, 1);
histogram_tester.ExpectUniqueSample(
"Autofill.LocalCardMigrationBubbleResult.Reshows",
AutofillMetrics::LOCAL_CARD_MIGRATION_BUBBLE_RESULT_UNKNOWN, 1);
} else {
histogram_tester.ExpectTotalCount(
"Autofill.LocalCardMigrationBubbleResult.FirstShow", 0);
histogram_tester.ExpectTotalCount(
"Autofill.LocalCardMigrationBubbleResult.Reshows", 0);
}
}
INSTANTIATE_TEST_SUITE_P(LocalCardMigrationBubbleControllerImplTest, INSTANTIATE_TEST_SUITE_P(LocalCardMigrationBubbleControllerImplTest,
LocalCardMigrationBubbleLoggingTest, LocalCardMigrationBubbleLoggingTest,
::testing::Bool()); ::testing::Bool());
......
...@@ -515,8 +515,8 @@ void SaveCardBubbleControllerImpl::OnBubbleClosed( ...@@ -515,8 +515,8 @@ void SaveCardBubbleControllerImpl::OnBubbleClosed(
metric = AutofillMetrics::SAVE_CARD_PROMPT_LOST_FOCUS; metric = AutofillMetrics::SAVE_CARD_PROMPT_LOST_FOCUS;
break; break;
case PaymentsBubbleClosedReason::kUnknown: case PaymentsBubbleClosedReason::kUnknown:
NOTREACHED(); metric = AutofillMetrics::SAVE_CARD_PROMPT_RESULT_UNKNOWN;
return; break;
} }
AutofillMetrics::LogSaveCardPromptResultMetric( AutofillMetrics::LogSaveCardPromptResultMetric(
metric, is_upload_save_, is_reshow_, options_, metric, is_upload_save_, is_reshow_, options_,
......
...@@ -1140,6 +1140,19 @@ TEST_P(SaveCardBubbleLoggingTest, Metrics_LostFocus) { ...@@ -1140,6 +1140,19 @@ TEST_P(SaveCardBubbleLoggingTest, Metrics_LostFocus) {
AutofillMetrics::SAVE_CARD_PROMPT_LOST_FOCUS, 1); AutofillMetrics::SAVE_CARD_PROMPT_LOST_FOCUS, 1);
} }
TEST_P(SaveCardBubbleLoggingTest, Metrics_Unknown) {
if (!metrics_revamp_experiment_enabled_)
return;
base::HistogramTester histogram_tester;
TriggerFlow();
CloseBubble(PaymentsBubbleClosedReason::kUnknown);
histogram_tester.ExpectUniqueSample(
"Autofill.SaveCreditCardPromptResult" + GetHistogramNameSuffix(),
AutofillMetrics::SAVE_CARD_PROMPT_RESULT_UNKNOWN, 1);
}
TEST_P(SaveCardBubbleLoggingTest, Metrics_SecurityLevel) { TEST_P(SaveCardBubbleLoggingTest, Metrics_SecurityLevel) {
base::HistogramTester histogram_tester; base::HistogramTester histogram_tester;
controller()->set_security_level(security_state::SecurityLevel::SECURE); controller()->set_security_level(security_state::SecurityLevel::SECURE);
......
...@@ -255,6 +255,9 @@ class AutofillMetrics { ...@@ -255,6 +255,9 @@ class AutofillMetrics {
SAVE_CARD_PROMPT_NOT_INTERACTED, SAVE_CARD_PROMPT_NOT_INTERACTED,
// The prompt lost focus and was deactivated. // The prompt lost focus and was deactivated.
SAVE_CARD_PROMPT_LOST_FOCUS, SAVE_CARD_PROMPT_LOST_FOCUS,
// The reason why the prompt is closed is not clear. Possible reason is the
// logging function is invoked before the closed reason is correctly set.
SAVE_CARD_PROMPT_RESULT_UNKNOWN,
NUM_SAVE_CARD_PROMPT_RESULT_METRICS, NUM_SAVE_CARD_PROMPT_RESULT_METRICS,
}; };
...@@ -568,6 +571,9 @@ class AutofillMetrics { ...@@ -568,6 +571,9 @@ class AutofillMetrics {
LOCAL_CARD_MIGRATION_BUBBLE_NOT_INTERACTED = 2, LOCAL_CARD_MIGRATION_BUBBLE_NOT_INTERACTED = 2,
// The bubble lost its focus and was deactivated. // The bubble lost its focus and was deactivated.
LOCAL_CARD_MIGRATION_BUBBLE_LOST_FOCUS = 3, LOCAL_CARD_MIGRATION_BUBBLE_LOST_FOCUS = 3,
// The reason why the prompt is closed is not clear. Possible reason is the
// logging function is invoked before the closed reason is correctly set.
LOCAL_CARD_MIGRATION_BUBBLE_RESULT_UNKNOWN = 4,
NUM_LOCAL_CARD_MIGRATION_BUBBLE_RESULT_METRICS, NUM_LOCAL_CARD_MIGRATION_BUBBLE_RESULT_METRICS,
}; };
......
...@@ -5217,6 +5217,7 @@ Unknown properties are collapsed to zero. --> ...@@ -5217,6 +5217,7 @@ Unknown properties are collapsed to zero. -->
<int value="1" label="User explicitly closed the the bubble"/> <int value="1" label="User explicitly closed the the bubble"/>
<int value="2" label="User did not interact with the bubble"/> <int value="2" label="User did not interact with the bubble"/>
<int value="3" label="Bubble lost focus and was deactivated"/> <int value="3" label="Bubble lost focus and was deactivated"/>
<int value="4" label="Bubble result unknown"/>
</enum> </enum>
<enum name="AutofillLocalCardMigrationBubbleUserInteraction"> <enum name="AutofillLocalCardMigrationBubbleUserInteraction">
...@@ -5397,6 +5398,7 @@ Unknown properties are collapsed to zero. --> ...@@ -5397,6 +5398,7 @@ Unknown properties are collapsed to zero. -->
<int value="2" label="Closed"/> <int value="2" label="Closed"/>
<int value="3" label="Not interacted"/> <int value="3" label="Not interacted"/>
<int value="4" label="Lost focus"/> <int value="4" label="Lost focus"/>
<int value="5" label="Unknown"/>
</enum> </enum>
<enum name="AutofillSaveType"> <enum name="AutofillSaveType">
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