Commit 33d42766 authored by sauski's avatar sauski Committed by Commit Bot

Hats Next: Record histogram when survey load times out

To capture when a HaTS Next survey fails to load a histogram entry is
created when loading of the dialog times out.

Bug: 1110888
Change-Id: Ibe55385e557cb1da7debb156019995a4fac22c21
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2443733Reviewed-by: default avatarBret Sepulveda <bsep@chromium.org>
Commit-Queue: Theodore Olsauskas-Warren <sauski@google.com>
Cr-Commit-Position: refs/heads/master@{#813746}
parent 36977a2f
...@@ -38,6 +38,9 @@ constexpr char kHatsSurveyTriggerSettingsPrivacy[] = "settings-privacy"; ...@@ -38,6 +38,9 @@ constexpr char kHatsSurveyTriggerSettingsPrivacy[] = "settings-privacy";
constexpr char kHatsNextSurveyTriggerIDTesting[] = constexpr char kHatsNextSurveyTriggerIDTesting[] =
"zishSVViB0kPN8UwQ150VGjBKuBP"; "zishSVViB0kPN8UwQ150VGjBKuBP";
constexpr char kHatsShouldShowSurveyReasonHistogram[] =
"Feedback.HappinessTrackingSurvey.ShouldShowSurveyReason";
namespace { namespace {
const base::Feature* survey_features[] = { const base::Feature* survey_features[] = {
...@@ -86,30 +89,6 @@ std::string GetLastSurveyCheckTime(const std::string& trigger) { ...@@ -86,30 +89,6 @@ std::string GetLastSurveyCheckTime(const std::string& trigger) {
return trigger + ".last_survey_check_time"; return trigger + ".last_survey_check_time";
} }
constexpr char kHatsShouldShowSurveyReasonHistogram[] =
"Feedback.HappinessTrackingSurvey.ShouldShowSurveyReason";
// These values are persisted to logs. Entries should not be renumbered and
// numeric values should never be reused.
enum class ShouldShowSurveyReasons {
kYes = 0,
kNoOffline = 1,
kNoLastSessionCrashed = 2,
kNoReceivedSurveyInCurrentMilestone = 3,
kNoProfileTooNew = 4,
kNoLastSurveyTooRecent = 5,
kNoBelowProbabilityLimit = 6,
kNoTriggerStringMismatch = 7,
kNoNotRegularBrowser = 8,
kNoIncognitoDisabled = 9,
kNoCookiesBlocked = 10, // Unused.
kNoThirdPartyCookiesBlocked = 11, // Unused.
kNoSurveyUnreachable = 12,
kNoSurveyOverCapacity = 13,
kNoSurveyAlreadyInProgress = 14,
kMaxValue = kNoSurveyAlreadyInProgress,
};
} // namespace } // namespace
HatsService::SurveyMetadata::SurveyMetadata() = default; HatsService::SurveyMetadata::SurveyMetadata() = default;
......
...@@ -36,6 +36,10 @@ extern const char kHatsSurveyTriggerSettingsPrivacy[]; ...@@ -36,6 +36,10 @@ extern const char kHatsSurveyTriggerSettingsPrivacy[];
// and demo purposes when the migration feature flag is enabled. // and demo purposes when the migration feature flag is enabled.
extern const char kHatsNextSurveyTriggerIDTesting[]; extern const char kHatsNextSurveyTriggerIDTesting[];
// The name of the histogram which records if a survey was shown, or if not, the
// reason why not.
extern const char kHatsShouldShowSurveyReasonHistogram[];
// This class provides the client side logic for determining if a // This class provides the client side logic for determining if a
// survey should be shown for any trigger based on input from a finch // survey should be shown for any trigger based on input from a finch
// configuration. It is created on a per profile basis. // configuration. It is created on a per profile basis.
...@@ -97,6 +101,27 @@ class HatsService : public KeyedService { ...@@ -97,6 +101,27 @@ class HatsService : public KeyedService {
base::WeakPtrFactory<DelayedSurveyTask> weak_ptr_factory_{this}; base::WeakPtrFactory<DelayedSurveyTask> weak_ptr_factory_{this};
}; };
// These values are persisted to logs. Entries should not be renumbered and
// numeric values should never be reused.
enum class ShouldShowSurveyReasons {
kYes = 0,
kNoOffline = 1,
kNoLastSessionCrashed = 2,
kNoReceivedSurveyInCurrentMilestone = 3,
kNoProfileTooNew = 4,
kNoLastSurveyTooRecent = 5,
kNoBelowProbabilityLimit = 6,
kNoTriggerStringMismatch = 7,
kNoNotRegularBrowser = 8,
kNoIncognitoDisabled = 9,
kNoCookiesBlocked = 10, // Unused.
kNoThirdPartyCookiesBlocked = 11, // Unused.
kNoSurveyUnreachable = 12,
kNoSurveyOverCapacity = 13,
kNoSurveyAlreadyInProgress = 14,
kMaxValue = kNoSurveyAlreadyInProgress,
};
~HatsService() override; ~HatsService() override;
explicit HatsService(Profile* profile); explicit HatsService(Profile* profile);
......
...@@ -8,6 +8,7 @@ ...@@ -8,6 +8,7 @@
#include "base/files/file_path.h" #include "base/files/file_path.h"
#include "base/path_service.h" #include "base/path_service.h"
#include "base/run_loop.h" #include "base/run_loop.h"
#include "base/test/metrics/histogram_tester.h"
#include "base/test/scoped_feature_list.h" #include "base/test/scoped_feature_list.h"
#include "base/time/time.h" #include "base/time/time.h"
#include "base/util/values/values_util.h" #include "base/util/values/values_util.h"
...@@ -299,6 +300,7 @@ IN_PROC_BROWSER_TEST_F(HatsNextWebDialogBrowserTest, SurveyClosed) { ...@@ -299,6 +300,7 @@ IN_PROC_BROWSER_TEST_F(HatsNextWebDialogBrowserTest, SurveyClosed) {
// timeout the widget is closed. // timeout the widget is closed.
IN_PROC_BROWSER_TEST_F(HatsNextWebDialogBrowserTest, SurveyTimeout) { IN_PROC_BROWSER_TEST_F(HatsNextWebDialogBrowserTest, SurveyTimeout) {
ASSERT_TRUE(embedded_test_server()->Start()); ASSERT_TRUE(embedded_test_server()->Start());
base::HistogramTester histogram_tester;
EXPECT_CALL(*hats_service(), HatsNextDialogClosed); EXPECT_CALL(*hats_service(), HatsNextDialogClosed);
auto* dialog = new MockHatsNextWebDialog( auto* dialog = new MockHatsNextWebDialog(
...@@ -307,6 +309,9 @@ IN_PROC_BROWSER_TEST_F(HatsNextWebDialogBrowserTest, SurveyTimeout) { ...@@ -307,6 +309,9 @@ IN_PROC_BROWSER_TEST_F(HatsNextWebDialogBrowserTest, SurveyTimeout) {
base::TimeDelta::FromMilliseconds(1)); base::TimeDelta::FromMilliseconds(1));
dialog->WaitForClose(); dialog->WaitForClose();
histogram_tester.ExpectUniqueSample(
kHatsShouldShowSurveyReasonHistogram,
HatsService::ShouldShowSurveyReasons::kNoSurveyUnreachable, 1);
} }
IN_PROC_BROWSER_TEST_F(HatsNextWebDialogBrowserTest, UnknownURLFragment) { IN_PROC_BROWSER_TEST_F(HatsNextWebDialogBrowserTest, UnknownURLFragment) {
......
...@@ -6,6 +6,7 @@ ...@@ -6,6 +6,7 @@
#include "chrome/browser/ui/browser_dialogs.h" #include "chrome/browser/ui/browser_dialogs.h"
#include "base/metrics/histogram_functions.h"
#include "base/util/values/values_util.h" #include "base/util/values/values_util.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/browser/profiles/profile_destroyer.h" #include "chrome/browser/profiles/profile_destroyer.h"
...@@ -212,7 +213,7 @@ HatsNextWebDialog::HatsNextWebDialog(Browser* browser, ...@@ -212,7 +213,7 @@ HatsNextWebDialog::HatsNextWebDialog(Browser* browser,
web_view_->web_contents()->SetDelegate(web_contents_delegate_.get()); web_view_->web_contents()->SetDelegate(web_contents_delegate_.get());
loading_timer_.Start(FROM_HERE, timeout_, loading_timer_.Start(FROM_HERE, timeout_,
base::BindOnce(&HatsNextWebDialog::CloseWidget, base::BindOnce(&HatsNextWebDialog::LoadTimedOut,
weak_factory_.GetWeakPtr())); weak_factory_.GetWeakPtr()));
} }
...@@ -231,6 +232,13 @@ HatsNextWebDialog::~HatsNextWebDialog() { ...@@ -231,6 +232,13 @@ HatsNextWebDialog::~HatsNextWebDialog() {
web_view_->web_contents()->SetDelegate(nullptr); web_view_->web_contents()->SetDelegate(nullptr);
} }
void HatsNextWebDialog::LoadTimedOut() {
base::UmaHistogramEnumeration(
kHatsShouldShowSurveyReasonHistogram,
HatsService::ShouldShowSurveyReasons::kNoSurveyUnreachable);
CloseWidget();
}
void HatsNextWebDialog::OnSurveyStateUpdateReceived(std::string state) { void HatsNextWebDialog::OnSurveyStateUpdateReceived(std::string state) {
loading_timer_.AbandonAndStop(); loading_timer_.AbandonAndStop();
......
...@@ -70,9 +70,9 @@ class HatsNextWebDialog : public ui::WebDialogDelegate, ...@@ -70,9 +70,9 @@ class HatsNextWebDialog : public ui::WebDialogDelegate,
class WebContentsDelegate; class WebContentsDelegate;
class WebContentsObserver; class WebContentsObserver;
// Closes the HaTS Next widget and informs the service that the dialog was // Called by |loading_timer_| after |timeout_| time has passed without getting
// shut. // an appropriate response from the HaTS service after creating the widget.
void CloseWidgetAndInformService(); void LoadTimedOut();
// Fired by the observer when the survey page has pushed state to the window // Fired by the observer when the survey page has pushed state to the window
// via URL fragments. // via URL fragments.
......
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