Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2665313002:
Add a counter for clicking on Form-Not-Secure warnings (Closed)
|
Created:
3 years, 10 months ago by estark Modified:
3 years, 10 months ago CC:
chromium-reviews, vabr+watchlistpasswordmanager_chromium.org, rouslan+autofill_chromium.org, rogerm+autofillwatch_chromium.org, sebsg+autofillwatch_chromium.org, browser-components-watch_chromium.org, mathp+autofillwatch_chromium.org, asvitkine+watch_chromium.org, vabr+watchlistautofill_chromium.org, estade+watch_chromium.org, gcasto+watchlist_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionAdd counters for clicking on Form-Not-Secure warnings
This will measure how often the Form-Not-Secure warnings ("Login not secure" or
"Payment not secure" in the autofill dropdown) are clicked on to
show more information about the warning.
BUG=687823
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
Review-Url: https://codereview.chromium.org/2665313002
Cr-Commit-Position: refs/heads/master@{#447909}
Committed: https://chromium.googlesource.com/chromium/src/+/b56160fa32a999a2b88a7987ca84efe277defe5a
Patch Set 1 : Record UMA metrics for clicking on "Learn More" #
Total comments: 2
Patch Set 2 : use UserActions, and separate for password/autofill #
Total comments: 2
Patch Set 3 : clarify comment #Patch Set 4 : clarify comment more #Patch Set 5 : rebase #Patch Set 6 : rebase fixup #Patch Set 7 : rebase fixup again #Messages
Total messages: 51 (34 generated)
The CQ bit was checked by estark@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from
==========
Add counters for showing Form-Not-Secure warnings
This will measure how often the Form-Not-Secure warnings ("Login not secure" or
"Payment not secure" in the autofill dropdown) are shown.
BUG=677291
==========
to
==========
Add a counter for clicking on Form-Not-Secure warnings
This will measure how often the Form-Not-Secure warnings ("Login not secure -
Learn more" or "Payment not secure - learn more" in the autofill dropdown) are
clicked on.
BUG=677291
==========
Description was changed from
==========
Add a counter for clicking on Form-Not-Secure warnings
This will measure how often the Form-Not-Secure warnings ("Login not secure -
Learn more" or "Payment not secure - learn more" in the autofill dropdown) are
clicked on.
BUG=677291
==========
to
==========
Add counters for showing Form-Not-Secure warnings
This will measure how often the Form-Not-Secure warnings ("Login not secure" or
"Payment not secure" in the autofill dropdown) are shown.
BUG=677291
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========
Patchset #2 (id:20001) has been deleted
Description was changed from
==========
Add counters for showing Form-Not-Secure warnings
This will measure how often the Form-Not-Secure warnings ("Login not secure" or
"Payment not secure" in the autofill dropdown) are shown.
BUG=677291
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========
to
==========
Add counters for clicking on Form-Not-Secure warnings
This will measure how often the Form-Not-Secure warnings ("Login not secure" or
"Payment not secure" in the autofill dropdown) are shown.
BUG=677291
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========
Patchset #1 (id:1) has been deleted
Description was changed from
==========
Add counters for clicking on Form-Not-Secure warnings
This will measure how often the Form-Not-Secure warnings ("Login not secure" or
"Payment not secure" in the autofill dropdown) are shown.
BUG=677291
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========
to
==========
Add counters for clicking on Form-Not-Secure warnings
This will measure how often the Form-Not-Secure warnings ("Login not secure" or
"Payment not secure" in the autofill dropdown) are clicked on to
show more information about the warning.
BUG=677291
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========
The CQ bit was checked by estark@chromium.org to run a CQ dry run
estark@chromium.org changed reviewers: + holte@chromium.org, mathp@chromium.org
mathp for autofill holte for histograms.xml Thanks!
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Can you test this in autofill_metrics_unittest.cc? https://codereview.chromium.org/2665313002/diff/40001/chrome/browser/ui/autof... File chrome/browser/ui/autofill/chrome_autofill_client.cc (right): https://codereview.chromium.org/2665313002/diff/40001/chrome/browser/ui/autof... chrome/browser/ui/autofill/chrome_autofill_client.cc:382: UMA_HISTOGRAM_BOOLEAN("Autofill.ShowHttpNotSecureExplanation", true); consider using a user action: https://cs.chromium.org/chromium/src/components/autofill/core/browser/autofil... Also, create a function in autofill_metrics and call it from here.
The CQ bit was checked by estark@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Thanks mathp. I've now split this metric into two UserActions, one for password manager and one for autofill. That makes it easier to unit-test both of them, and also it can't hurt to have the metric split out so that we can see if there's a difference in Login-not-secure clicks vs Payment-not-secure clicks. https://codereview.chromium.org/2665313002/diff/40001/chrome/browser/ui/autof... File chrome/browser/ui/autofill/chrome_autofill_client.cc (right): https://codereview.chromium.org/2665313002/diff/40001/chrome/browser/ui/autof... chrome/browser/ui/autofill/chrome_autofill_client.cc:382: UMA_HISTOGRAM_BOOLEAN("Autofill.ShowHttpNotSecureExplanation", true); On 2017/02/01 13:45:39, Mathieu Perreault wrote: > consider using a user action: > https://cs.chromium.org/chromium/src/components/autofill/core/browser/autofil... Oh I've always wondered why something like that doesn't exist, TIL! Thanks! > > Also, create a function in autofill_metrics and call it from here. Done.
Thanks for making the change! lgtm for autofill
estark@chromium.org changed reviewers: + vasilii@chromium.org
+vasilii for the password_manager changes
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
actions.xml lgtm
Description was changed from
==========
Add counters for clicking on Form-Not-Secure warnings
This will measure how often the Form-Not-Secure warnings ("Login not secure" or
"Payment not secure" in the autofill dropdown) are clicked on to
show more information about the warning.
BUG=677291
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========
to
==========
Add counters for clicking on Form-Not-Secure warnings
This will measure how often the Form-Not-Secure warnings ("Login not secure" or
"Payment not secure" in the autofill dropdown) are clicked on to
show more information about the warning.
BUG=687823
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========
lgtm components/password_manager/ https://codereview.chromium.org/2665313002/diff/60001/components/password_man... File components/password_manager/core/browser/password_manager_metrics_util.h (right): https://codereview.chromium.org/2665313002/diff/60001/components/password_man... components/password_manager/core/browser/password_manager_metrics_util.h:245: // Log when the user selects the Form-Not-Secure warning to show more "Login not secure"? Selects where? in the passwords drop-down or in the omnibox too?
Thanks all! https://codereview.chromium.org/2665313002/diff/60001/components/password_man... File components/password_manager/core/browser/password_manager_metrics_util.h (right): https://codereview.chromium.org/2665313002/diff/60001/components/password_man... components/password_manager/core/browser/password_manager_metrics_util.h:245: // Log when the user selects the Form-Not-Secure warning to show more On 2017/02/02 13:33:01, vasilii wrote: > "Login not secure"? > Selects where? in the passwords drop-down or in the omnibox too? Just the dropdown. Clarified the comment.
The CQ bit was checked by estark@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from mathp@chromium.org, vasilii@chromium.org, holte@chromium.org Link to the patchset: https://codereview.chromium.org/2665313002/#ps80001 (title: "clarify comment")
The CQ bit was unchecked by estark@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by estark@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from mathp@chromium.org, vasilii@chromium.org, holte@chromium.org Link to the patchset: https://codereview.chromium.org/2665313002/#ps100001 (title: "clarify comment more")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Failed to apply patch for components/autofill/core/browser/autofill_metrics.cc:
While running git apply --index -p1;
error: patch failed: components/autofill/core/browser/autofill_metrics.cc:675
error: components/autofill/core/browser/autofill_metrics.cc: patch does not
apply
Patch: components/autofill/core/browser/autofill_metrics.cc
Index: components/autofill/core/browser/autofill_metrics.cc
diff --git a/components/autofill/core/browser/autofill_metrics.cc
b/components/autofill/core/browser/autofill_metrics.cc
index
bbceb4fe39aea517b0b9eb5bd4f510199f08c99e..8121ea313869e18f04ddafdfa755a66864dc1585
100644
--- a/components/autofill/core/browser/autofill_metrics.cc
+++ b/components/autofill/core/browser/autofill_metrics.cc
@@ -675,6 +675,12 @@ void AutofillMetrics::LogIsQueriedCreditCardFormSecure(bool
is_secure) {
UMA_HISTOGRAM_BOOLEAN("Autofill.QueriedCreditCardFormIsSecure", is_secure);
}
+// static
+void AutofillMetrics::LogShowedHttpNotSecureExplanation() {
+ base::RecordAction(
+ base::UserMetricsAction("Autofill_ShowedHttpNotSecureExplanation"));
+}
+
AutofillMetrics::FormEventLogger::FormEventLogger(bool is_for_credit_card)
: is_for_credit_card_(is_for_credit_card),
is_server_data_available_(false),
The CQ bit was checked by estark@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
The CQ bit was checked by estark@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from mathp@chromium.org, vasilii@chromium.org, holte@chromium.org Link to the patchset: https://codereview.chromium.org/2665313002/#ps140001 (title: "rebase fixup")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: ios-device on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...) ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
The CQ bit was checked by estark@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from mathp@chromium.org, vasilii@chromium.org, holte@chromium.org Link to the patchset: https://codereview.chromium.org/2665313002/#ps160001 (title: "rebase fixup again")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 160001, "attempt_start_ts": 1486080545267880,
"parent_rev": "7a81c252f5f9cf8981f5df770a5acbb8d736eb6e", "commit_rev":
"b56160fa32a999a2b88a7987ca84efe277defe5a"}
Message was sent while issue was closed.
Description was changed from
==========
Add counters for clicking on Form-Not-Secure warnings
This will measure how often the Form-Not-Secure warnings ("Login not secure" or
"Payment not secure" in the autofill dropdown) are clicked on to
show more information about the warning.
BUG=687823
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========
to
==========
Add counters for clicking on Form-Not-Secure warnings
This will measure how often the Form-Not-Secure warnings ("Login not secure" or
"Payment not secure" in the autofill dropdown) are clicked on to
show more information about the warning.
BUG=687823
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
Review-Url: https://codereview.chromium.org/2665313002
Cr-Commit-Position: refs/heads/master@{#447909}
Committed:
https://chromium.googlesource.com/chromium/src/+/b56160fa32a999a2b88a7987ca84...
==========
Message was sent while issue was closed.
Committed patchset #7 (id:160001) as https://chromium.googlesource.com/chromium/src/+/b56160fa32a999a2b88a7987ca84... |
