Commit 7496904f authored by vitaliii's avatar vitaliii Committed by Commit bot

[NTP::SectionOrder] Track category index after it has been moved up.

Add a new metric to track category index after it has been moved up due
to a click. This is done in order to check how stable to order is and
whether it is becoming more stable.

BUG=687505

Review-Url: https://codereview.chromium.org/2668063004
Cr-Commit-Position: refs/heads/master@{#447968}
parent ca0b0f3c
......@@ -12,6 +12,7 @@
#include "base/strings/string_number_conversions.h"
#include "base/values.h"
#include "components/ntp_snippets/category_rankers/constant_category_ranker.h"
#include "components/ntp_snippets/content_suggestions_metrics.h"
#include "components/ntp_snippets/features.h"
#include "components/ntp_snippets/pref_names.h"
#include "components/prefs/pref_registry_simple.h"
......@@ -229,6 +230,8 @@ void ClickBasedCategoryRanker::OnSuggestionOpened(Category category) {
std::vector<RankedCategory>::iterator previous = current - 1;
const int passing_margin = GetPositionPassingMargin(previous);
if (current->clicks >= previous->clicks + passing_margin) {
const int new_index = previous - ordered_categories_.begin();
ntp_snippets::metrics::OnCategoryMovedUp(new_index);
// It is intended to move only by one position per click in order to avoid
// dramatic changes, which could confuse the user.
std::swap(*current, *previous);
......
......@@ -8,6 +8,7 @@
#include "base/memory/ptr_util.h"
#include "base/strings/string_number_conversions.h"
#include "base/test/histogram_tester.h"
#include "base/test/simple_test_clock.h"
#include "base/time/default_clock.h"
#include "base/time/time.h"
......@@ -20,8 +21,18 @@
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
using testing::ElementsAre;
using testing::IsEmpty;
namespace ntp_snippets {
namespace {
const char kHistogramMovedUpCategoryNewIndex[] =
"NewTabPage.ContentSuggestions.MovedUpCategoryNewIndex";
} // namespace
class ClickBasedCategoryRankerTest : public testing::Test {
public:
ClickBasedCategoryRankerTest()
......@@ -253,8 +264,7 @@ TEST_F(ClickBasedCategoryRankerTest, ShouldPersistOrderAndClicksWhenRestarted) {
EXPECT_TRUE(CompareCategories(third, second));
// Clicks must be preserved as well.
NotifyOnSuggestionOpened(
/*times=*/1, third);
NotifyOnSuggestionOpened(/*times=*/1, third);
EXPECT_TRUE(CompareCategories(third, first));
}
......@@ -449,8 +459,7 @@ TEST_F(ClickBasedCategoryRankerTest,
ASSERT_TRUE(CompareCategories(first, second));
NotifyOnSuggestionOpened(
/*times=*/1, second);
NotifyOnSuggestionOpened(/*times=*/1, second);
// This should reduce the click count back to 0.
NotifyOnCategoryDismissed(second);
......@@ -462,8 +471,7 @@ TEST_F(ClickBasedCategoryRankerTest,
EXPECT_TRUE(CompareCategories(first, second));
NotifyOnSuggestionOpened(
/*times=*/1, second);
NotifyOnSuggestionOpened(/*times=*/1, second);
EXPECT_FALSE(CompareCategories(first, second));
}
......@@ -487,8 +495,7 @@ TEST_F(ClickBasedCategoryRankerTest,
Category second = AddUnusedRemoteCategory();
Category third = AddUnusedRemoteCategory();
NotifyOnSuggestionOpened(
/*times=*/1, second);
NotifyOnSuggestionOpened(/*times=*/1, second);
// This should be ignored, because the penalty is set to 0.
NotifyOnCategoryDismissed(second);
......@@ -649,4 +656,94 @@ TEST_F(ClickBasedCategoryRankerTest,
EXPECT_TRUE(CompareCategories(recent_tabs, downloads));
}
TEST_F(ClickBasedCategoryRankerTest,
ShouldEmitNewIndexWhenCategoryMovedUpDueToClick) {
base::HistogramTester histogram_tester;
std::vector<KnownCategories> default_order =
ConstantCategoryRanker::GetKnownCategoriesDefaultOrder();
Category first = Category::FromKnownCategory(default_order[0]);
Category second = Category::FromKnownCategory(default_order[1]);
ASSERT_TRUE(CompareCategories(first, second));
// Increase the score of |second| until the order changes.
while (CompareCategories(first, second)) {
EXPECT_THAT(
histogram_tester.GetAllSamples(kHistogramMovedUpCategoryNewIndex),
IsEmpty());
ranker()->OnSuggestionOpened(second);
}
ASSERT_FALSE(CompareCategories(first, second));
EXPECT_THAT(histogram_tester.GetAllSamples(kHistogramMovedUpCategoryNewIndex),
ElementsAre(base::Bucket(/*min=*/0, /*count=*/1)));
}
TEST_F(ClickBasedCategoryRankerTest,
ShouldNotEmitNewIndexWhenCategoryDismissed) {
base::HistogramTester histogram_tester;
std::vector<KnownCategories> default_order =
ConstantCategoryRanker::GetKnownCategoriesDefaultOrder();
Category category = Category::FromKnownCategory(default_order[0]);
ASSERT_THAT(histogram_tester.GetAllSamples(kHistogramMovedUpCategoryNewIndex),
IsEmpty());
NotifyOnCategoryDismissed(category);
EXPECT_THAT(histogram_tester.GetAllSamples(kHistogramMovedUpCategoryNewIndex),
IsEmpty());
}
TEST_F(ClickBasedCategoryRankerTest,
ShouldNotEmitNewIndexOfMovedUpCategoryWhenHistoryCleared) {
std::vector<KnownCategories> default_order =
ConstantCategoryRanker::GetKnownCategoriesDefaultOrder();
Category first = Category::FromKnownCategory(default_order[0]);
Category second = Category::FromKnownCategory(default_order[1]);
ASSERT_TRUE(CompareCategories(first, second));
// Increase the score of |second| until the order changes.
while (CompareCategories(first, second)) {
ranker()->OnSuggestionOpened(second);
}
ASSERT_FALSE(CompareCategories(first, second));
// The histogram tester is created here to ignore previous events.
base::HistogramTester histogram_tester;
ranker()->ClearHistory(/*begin=*/base::Time(),
/*end=*/base::Time::Max());
// ClearHistory should restore the default order.
ASSERT_TRUE(CompareCategories(first, second));
EXPECT_THAT(histogram_tester.GetAllSamples(kHistogramMovedUpCategoryNewIndex),
IsEmpty());
}
TEST_F(ClickBasedCategoryRankerTest,
ShouldNotEmitNewIndexWhenCategoryPromoted) {
base::HistogramTester histogram_tester;
std::vector<KnownCategories> default_order =
ConstantCategoryRanker::GetKnownCategoriesDefaultOrder();
Category first = Category::FromKnownCategory(default_order[0]);
Category second = Category::FromKnownCategory(default_order[1]);
ASSERT_TRUE(CompareCategories(first, second));
ASSERT_THAT(histogram_tester.GetAllSamples(kHistogramMovedUpCategoryNewIndex),
IsEmpty());
SetPromotedCategoryVariationParam(second.id());
ResetRanker(base::MakeUnique<base::DefaultClock>());
ASSERT_FALSE(CompareCategories(first, second));
EXPECT_THAT(histogram_tester.GetAllSamples(kHistogramMovedUpCategoryNewIndex),
IsEmpty());
}
} // namespace ntp_snippets
......@@ -55,6 +55,8 @@ const char kHistogramMoreButtonShown[] =
"NewTabPage.ContentSuggestions.MoreButtonShown";
const char kHistogramMoreButtonClicked[] =
"NewTabPage.ContentSuggestions.MoreButtonClicked";
const char kHistogramMovedUpCategoryNewIndex[] =
"NewTabPage.ContentSuggestions.MovedUpCategoryNewIndex";
const char kHistogramCategoryDismissed[] =
"NewTabPage.ContentSuggestions.CategoryDismissed";
const char kHistogramContentSuggestionsTimeSinceLastBackgroundFetch[] =
......@@ -330,6 +332,11 @@ void OnSuggestionTargetVisited(Category category, base::TimeDelta visit_time) {
base::UmaHistogramLongTimes(name, visit_time);
}
void OnCategoryMovedUp(int new_index) {
UMA_HISTOGRAM_EXACT_LINEAR(kHistogramMovedUpCategoryNewIndex, new_index,
kMaxCategories);
}
void OnMoreButtonShown(Category category, int position) {
// The "more" card can appear in addition to the actual suggestions, so add
// one extra bucket to this histogram.
......
......@@ -49,6 +49,8 @@ void OnSuggestionDismissed(int global_position,
void OnSuggestionTargetVisited(Category category, base::TimeDelta visit_time);
void OnCategoryMovedUp(int new_index);
// Should only be called once per NTP for each "more" button.
void OnMoreButtonShown(Category category, int position);
......
......@@ -39323,6 +39323,19 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
</summary>
</histogram>
<histogram name="NewTabPage.ContentSuggestions.MovedUpCategoryNewIndex"
units="index">
<owner>vitaliii@chromium.org</owner>
<summary>
Android: The new index of a category on the NTP after it has been moved up
due to a click. The index of a category, which has been overtaken, is not
recorded here. This tracked index can be different from the position
observed by the user, e.g. empty categories are not shown. This metric
ignores all other order changes (e.g. dismissing a category or clearing
history).
</summary>
</histogram>
<histogram name="NewTabPage.ContentSuggestions.Notifications.Actions"
enum="ContentSuggestionsNotificationsAction">
<owner>sfiera@chromium.org</owner>
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