Skip to content

Commit

Permalink
[Feed Internals] Add 4 new properties to page
Browse files Browse the repository at this point in the history
This internals page is used to surface debugging information useful to
developers of the NTP Interest Feed.

Adds 4 additional simple properties to the debugging page:
- whether the Feed is visible
- whether the Feed is allowed (by policy)
- whether prefetching for offline use is enabled
- the reason for the last fetch

https://screenshot.googleplex.com/efYU09sWvNP

Bug: 913126
Change-Id: Ib9cad2a6fca27b292f4cb5198e1298fc72ad13ae
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1512267
Commit-Queue: Natalie Chouinard <chouinard@chromium.org>
Reviewed-by: Sky Malice <skym@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#639691}
  • Loading branch information
sudonatalie authored and Commit Bot committed Mar 11, 2019
1 parent 945e5ff commit 6bb741c
Show file tree
Hide file tree
Showing 7 changed files with 134 additions and 5 deletions.
16 changes: 16 additions & 0 deletions chrome/browser/resources/feed_internals/feed_internals.html
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,18 @@ <h2>Properties</h2>
<td>Is Feed Enabled</td>
<td id="is-feed-enabled"></td>
</tr>
<tr>
<td>Is Feed Visible</td>
<td id="is-feed-visible"></td>
</tr>
<tr>
<td>Is Feed Allowed</td>
<td id="is-feed-allowed"></td>
</tr>
<tr>
<td>Is Prefetching Enabled</td>
<td id="is-prefetching-enabled"></td>
</tr>
<tr>
<td>Feed Fetch URL</td>
<td id="feed-fetch-url"></td>
Expand Down Expand Up @@ -66,6 +78,10 @@ <h2>Last Fetch</h2>
<td>Last Fetch Status</td>
<td id="last-fetch-status"></td>
</tr>
<tr>
<td>Last Fetch Trigger</td>
<td id="last-fetch-trigger"></td>
</tr>
<tr>
<td>Last Fetch Time</td>
<td id="last-fetch-time"></td>
Expand Down
4 changes: 4 additions & 0 deletions chrome/browser/resources/feed_internals/feed_internals.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ function updatePageWithProperties() {
/** @type {!feedInternals.mojom.Properties} */
const properties = response.properties;
$('is-feed-enabled').textContent = properties.isFeedEnabled;
$('is-feed-visible').textContent = properties.isFeedVisible;
$('is-feed-allowed').textContent = properties.isFeedAllowed;
$('is-prefetching-enabled').textContent = properties.isPrefetchingEnabled;
$('feed-fetch-url').textContent = properties.feedFetchUrl.url;
});
}
Expand All @@ -45,6 +48,7 @@ function updatePageWithLastFetchProperties() {
/** @type {!feedInternals.mojom.LastFetchProperties} */
const properties = response.properties;
$('last-fetch-status').textContent = properties.lastFetchStatus;
$('last-fetch-trigger').textContent = properties.lastFetchTrigger;
$('last-fetch-time').textContent = toDateString(properties.lastFetchTime);
$('refresh-suppress-time').textContent =
toDateString(properties.refreshSuppressTime);
Expand Down
14 changes: 13 additions & 1 deletion chrome/browser/ui/webui/feed_internals/feed_internals.mojom
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,18 @@ import "url/mojom/url.mojom";

// General properties of Feed suggestions.
struct Properties {
// Whether the Feed is enabled.
// Whether the Feed feature flag is enabled.
bool is_feed_enabled;

// Whether suggested articles section is expanded.
bool is_feed_visible;

// Whether suggested articles are allowed. Typically set by policy.
bool is_feed_allowed;

// Whether prefetching for offline availability is enabled.
bool is_prefetching_enabled;

// Feed fetch URL.
url.mojom.Url feed_fetch_url;
};
Expand All @@ -32,6 +41,9 @@ struct LastFetchProperties {
// Last fetch status.
int32 last_fetch_status;

// Reason for the last fetch.
string last_fetch_trigger;

// Last fetch time.
Time? last_fetch_time;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "components/feed/core/pref_names.h"
#include "components/feed/core/user_classifier.h"
#include "components/feed/feed_feature_list.h"
#include "components/offline_pages/core/prefetch/prefetch_prefs.h"
#include "components/offline_pages/core/prefetch/suggestions_provider.h"
#include "components/prefs/pref_service.h"

Expand All @@ -27,6 +28,17 @@ feed_internals::mojom::TimePtr ToMojoTime(base::Time time) {
: feed_internals::mojom::Time::New(time.ToJsTime());
}

std::string TriggerTypeToString(feed::FeedSchedulerHost::TriggerType trigger) {
switch (trigger) {
case feed::FeedSchedulerHost::TriggerType::kNtpShown:
return "NTP Shown";
case feed::FeedSchedulerHost::TriggerType::kForegrounded:
return "Foregrounded";
case feed::FeedSchedulerHost::TriggerType::kFixedTimer:
return "Fixed Timer";
}
}

} // namespace

FeedInternalsPageHandler::FeedInternalsPageHandler(
Expand All @@ -47,6 +59,12 @@ void FeedInternalsPageHandler::GetGeneralProperties(

properties->is_feed_enabled =
base::FeatureList::IsEnabled(feed::kInterestFeedContentSuggestions);
properties->is_feed_visible =
pref_service_->GetBoolean(feed::prefs::kArticlesListVisible);
properties->is_feed_allowed =
pref_service_->GetBoolean(feed::prefs::kEnableSnippets);
properties->is_prefetching_enabled =
offline_pages::prefetch_prefs::IsEnabled(pref_service_);
properties->feed_fetch_url = feed::GetFeedFetchUrlForDebugging();

std::move(callback).Run(std::move(properties));
Expand Down Expand Up @@ -75,6 +93,8 @@ void FeedInternalsPageHandler::GetLastFetchProperties(

properties->last_fetch_status =
feed_scheduler_host_->GetLastFetchStatusForDebugging();
properties->last_fetch_trigger = TriggerTypeToString(
feed_scheduler_host_->GetLastFetchTriggerTypeForDebugging());
properties->last_fetch_time =
ToMojoTime(pref_service_->GetTime(feed::prefs::kLastFetchAttemptTime));
properties->refresh_suppress_time =
Expand Down
6 changes: 6 additions & 0 deletions components/feed/core/feed_scheduler_host.cc
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,10 @@ int FeedSchedulerHost::GetLastFetchStatusForDebugging() const {
return last_fetch_status_;
}

TriggerType FeedSchedulerHost::GetLastFetchTriggerTypeForDebugging() const {
return last_fetch_trigger_type_;
}

void FeedSchedulerHost::OnEulaAccepted() {
OnForegrounded();
}
Expand Down Expand Up @@ -598,6 +602,8 @@ FeedSchedulerHost::ShouldRefreshResult FeedSchedulerHost::ShouldRefresh(
clock_->Now() +
base::TimeDelta::FromSeconds(kTimeoutDurationSeconds.Get());

last_fetch_trigger_type_ = trigger;

return kShouldRefresh;
}

Expand Down
12 changes: 10 additions & 2 deletions components/feed/core/feed_scheduler_host.h
Original file line number Diff line number Diff line change
Expand Up @@ -143,15 +143,20 @@ class FeedSchedulerHost : web_resource::EulaAcceptedNotifier::Observer {
// Surface last_fetch_status_ for internals debugging page.
int GetLastFetchStatusForDebugging() const;

// Surface the TriggerType for the last ShouldRefresh check that resulted in
// kShouldRefresh. Callers of ShouldRefresh are presumed to follow with the
// actual refresh.
TriggerType GetLastFetchTriggerTypeForDebugging() const;

private:
FRIEND_TEST_ALL_PREFIXES(FeedSchedulerHostTest, GetTriggerThreshold);

// web_resource::EulaAcceptedNotifier::Observer:
void OnEulaAccepted() override;

// Determines whether a refresh should be performed for the given |trigger|.
// If this method is called and returns true we presume the refresh will
// happen, therefore we report metrics respectively and update
// If this method is called and returns kShouldRefresh we presume the refresh
// will happen, therefore we report metrics respectively and update
// |tracking_oustanding_request_|.
ShouldRefreshResult ShouldRefresh(TriggerType trigger);

Expand Down Expand Up @@ -228,6 +233,9 @@ class FeedSchedulerHost : web_resource::EulaAcceptedNotifier::Observer {
// Status of the last fetch for debugging.
int last_fetch_status_ = 0;

// Reason for last fetch for debugging.
TriggerType last_fetch_trigger_type_;

DISALLOW_COPY_AND_ASSIGN(FeedSchedulerHost);
};

Expand Down
67 changes: 65 additions & 2 deletions components/feed/core/feed_scheduler_host_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,9 @@ class FeedSchedulerHostTest : public ::testing::Test {
// By moving time forward from initial seed events, the user will be moved
// into kRareNtpUser classification.
test_clock()->Advance(TimeDelta::FromDays(7));

ASSERT_EQ(UserClassifier::UserClass::kRareSuggestionsViewer,
scheduler()->GetUserClassifierForDebugging()->GetUserClass());
}

// Note: Time will be advanced.
Expand All @@ -97,6 +100,12 @@ class FeedSchedulerHostTest : public ::testing::Test {
scheduler()->OnSuggestionConsumed();
test_clock()->Advance(TimeDelta::FromMinutes(31));
scheduler()->OnSuggestionConsumed();

// Depending on which events occurred over which period of time in the test
// before this function was called, it may not necessarily be sufficient to
// push the user into the active consumer class.
ASSERT_EQ(UserClassifier::UserClass::kActiveSuggestionsConsumer,
scheduler()->GetUserClassifierForDebugging()->GetUserClass());
}

// Many test cases want to ask the scheduler multiple times in a row to see
Expand Down Expand Up @@ -175,12 +184,12 @@ TEST_F(FeedSchedulerHostTest, GetTriggerThreshold) {
EXPECT_FALSE(scheduler()->GetTriggerThreshold(trigger).is_zero());
}

ClassifyAsRareNtpUser();
ClassifyAsActiveSuggestionsConsumer();
for (FeedSchedulerHost::TriggerType trigger : triggers) {
EXPECT_FALSE(scheduler()->GetTriggerThreshold(trigger).is_zero());
}

ClassifyAsActiveSuggestionsConsumer();
ClassifyAsRareNtpUser();
for (FeedSchedulerHost::TriggerType trigger : triggers) {
EXPECT_FALSE(scheduler()->GetTriggerThreshold(trigger).is_zero());
}
Expand Down Expand Up @@ -994,4 +1003,58 @@ TEST_F(FeedSchedulerHostTest, RefreshThrottler) {
}
}

TEST_F(FeedSchedulerHostTest, GetUserClassifierForDebuggingRareUser) {
ClassifyAsRareNtpUser();

EXPECT_EQ(UserClassifier::UserClass::kRareSuggestionsViewer,
scheduler()->GetUserClassifierForDebugging()->GetUserClass());
}

TEST_F(FeedSchedulerHostTest, GetUserClassifierForDebuggingActiveConsumer) {
ClassifyAsActiveSuggestionsConsumer();

EXPECT_EQ(UserClassifier::UserClass::kActiveSuggestionsConsumer,
scheduler()->GetUserClassifierForDebugging()->GetUserClass());
}

TEST_F(FeedSchedulerHostTest, GetSuppressRefreshesUntilForDebugging) {
EXPECT_TRUE(scheduler()->GetSuppressRefreshesUntilForDebugging().is_null());

scheduler()->OnArticlesCleared(/*suppress_refreshes*/ true);

EXPECT_EQ(test_clock()->Now() + TimeDelta::FromMinutes(30),
scheduler()->GetSuppressRefreshesUntilForDebugging());
}

TEST_F(FeedSchedulerHostTest, GetLastFetchStatusForDebugging) {
EXPECT_EQ(0, scheduler()->GetLastFetchStatusForDebugging());

scheduler()->OnReceiveNewContent(Time());

EXPECT_EQ(200, scheduler()->GetLastFetchStatusForDebugging());

scheduler()->OnRequestError(-100);

EXPECT_EQ(-100, scheduler()->GetLastFetchStatusForDebugging());
}

TEST_F(FeedSchedulerHostTest, GetLastFetchTriggerTypeForDebugging) {
scheduler()->OnForegrounded();

EXPECT_EQ(FeedSchedulerHost::TriggerType::kForegrounded,
scheduler()->GetLastFetchTriggerTypeForDebugging());

scheduler()->OnArticlesCleared(/*suppress_refreshes*/ false);

EXPECT_EQ(FeedSchedulerHost::TriggerType::kNtpShown,
scheduler()->GetLastFetchTriggerTypeForDebugging());

ClassifyAsActiveSuggestionsConsumer(); // Fixed timer at 48 hours.
test_clock()->Advance(TimeDelta::FromHours(49));
scheduler()->OnFixedTimer(base::OnceClosure());

EXPECT_EQ(FeedSchedulerHost::TriggerType::kFixedTimer,
scheduler()->GetLastFetchTriggerTypeForDebugging());
}

} // namespace feed

0 comments on commit 6bb741c

Please sign in to comment.