Skip to content

Commit

Permalink
webfeed: add internals button to refresh web feed
Browse files Browse the repository at this point in the history 
This CL changes one refresh feed button into two, so
that we can refresh each feed individually. This
makes it easier to see the load status, since only
one load status is shown on the internals page.

I also removed some parts of the internals page
that were not used in feed v2.

Bug: b/190566852, 1066230
Change-Id: Idc543d5468bdd5f26045e60e4a2fe39d9a4284f5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3122533
Auto-Submit: Dan H <harringtond@chromium.org>
Reviewed-by: Carlos Knippschild <carlosk@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Commit-Queue: Ken Buchanan <kenrb@chromium.org>
Cr-Commit-Position: refs/heads/main@{#915727}
  • Loading branch information
Dan Harrington authored and Chromium LUCI CQ committed Aug 26, 2021
1 parent 4890d7e commit a9a6576
Show file tree
Hide file tree
Showing 10 changed files with 36 additions and 212 deletions.
54 changes: 4 additions & 50 deletions chrome/browser/resources/feed_internals/feed_internals.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,31 +93,12 @@ <h2>Properties</h2>
</tr>
</table>

<h2>User Classifier</h2>
<table>
<tr>
<td>User Class Description</td>
<td id="user-class-description"></td>
</tr>
<tr>
<td>Average Hours Between Suggestion Views</td>
<td id="avg-hours-between-views"></td>
</tr>
<tr>
<td>Average Hours Between Suggestion Uses</td>
<td id="avg-hours-between-uses"></td>
</tr>
</table>
<button id="clear-user-classification">
Clear User Classification
</button>

<h2>Feed Library Actions</h2>
<button id="refresh-feed">
Refresh Feed
<button id="refresh-for-you">
Refresh For-You Feed
</button>
<button id="clear-cached-data">
Clear Cache & Refresh Feed
<button id="refresh-following">
Refresh Following Feed
</button>

<h2>Last Fetch</h2>
Expand Down Expand Up @@ -156,33 +137,6 @@ <h2>Last Action Upload</h2>
</tr>
</table>

<h2>Current Content</h2>
<div id="current-content">
<template id="suggestion-template">
<details>
<summary class="title"></summary>
<table>
<tr>
<td>Publisher Name</td>
<td class="publisher"></td>
</tr>
<tr>
<td>URL</td>
<td><a class="url"></a></td>
</tr>
<tr>
<td>Favicon URL</td>
<td><a class="favicon"></a></td>
</tr>
<tr>
<td>Image URL</td>
<td><a class="image"></a></td>
</tr>
</table>
</details>
</template>
</div>

<h2>Feed Histograms</h2>
<button id="load-feed-histograms">
Load Feed Histograms
Expand Down
59 changes: 4 additions & 55 deletions chrome/browser/resources/feed_internals/feed_internals.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,19 +50,6 @@ function updatePageWithProperties() {
});
}

/**
* Get and display user classifier properties.
*/
function updatePageWithUserClass() {
pageHandler.getUserClassifierProperties().then(response => {
/** @type {!feedInternals.mojom.UserClassifier} */
const properties = response.properties;
$('user-class-description').textContent = properties.userClassDescription;
$('avg-hours-between-views').textContent = properties.avgHoursBetweenViews;
$('avg-hours-between-uses').textContent = properties.avgHoursBetweenUses;
});
}

/**
* Get and display last fetch data.
*/
Expand All @@ -83,37 +70,6 @@ function updatePageWithLastFetchProperties() {
});
}

/**
* Get and display last known content.
*/
function updatePageWithCurrentContent() {
pageHandler.getCurrentContent().then(response => {
const before = $('current-content');
const after = before.cloneNode(false);

/** @type {!Array<feedInternals.mojom.Suggestion>} */
const suggestions = response.suggestions;

for (const suggestion of suggestions) {
// Create new content item from template.
const item = document.importNode($('suggestion-template').content, true);

// Populate template with text metadata.
item.querySelector('.title').textContent = suggestion.title;
item.querySelector('.publisher').textContent = suggestion.publisherName;

// Populate template with link metadata.
setLinkNode(item.querySelector('a.url'), suggestion.url.url);
setLinkNode(item.querySelector('a.image'), suggestion.imageUrl.url);
setLinkNode(item.querySelector('a.favicon'), suggestion.faviconUrl.url);

after.appendChild(item);
}

before.replaceWith(after);
});
}

/**
* Populate <a> node with hyperlinked URL.
*
Expand Down Expand Up @@ -141,17 +97,12 @@ function toDateString(timeSinceEpoch) {
* Hook up buttons to event listeners.
*/
function setupEventListeners() {
$('clear-user-classification').addEventListener('click', function() {
pageHandler.clearUserClassifierProperties();
updatePageWithUserClass();
});

$('clear-cached-data').addEventListener('click', function() {
pageHandler.clearCachedDataAndRefreshFeed();
$('refresh-for-you').addEventListener('click', function() {
pageHandler.refreshForYouFeed();
});

$('refresh-feed').addEventListener('click', function() {
pageHandler.refreshFeed();
$('refresh-following').addEventListener('click', function() {
pageHandler.refreshFollowingFeed();
});

$('dump-feed-process-scope').addEventListener('click', function() {
Expand Down Expand Up @@ -225,9 +176,7 @@ function setupEventListeners() {

function updatePage() {
updatePageWithProperties();
updatePageWithUserClass();
updatePageWithLastFetchProperties();
updatePageWithCurrentContent();
}

document.addEventListener('DOMContentLoaded', function() {
Expand Down
49 changes: 4 additions & 45 deletions chrome/browser/ui/webui/feed_internals/feed_internals.mojom
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,19 +48,6 @@ struct Properties {
FeedOrder following_feed_order;
};

struct UserClassifier {
// User class description.
string user_class_description;

// Estimated average length of time between two successive suggestion views of
// in hours.
float avg_hours_between_views;

// Estimated average length of time between two successive uses of suggestions
// in hours.
float avg_hours_between_uses;
};

struct LastFetchProperties {
// Last fetch status.
int32 last_fetch_status;
Expand All @@ -85,48 +72,20 @@ struct LastFetchProperties {
mojo_base.mojom.TimeDelta last_action_upload_time;
};

// Models a single suggestion in the Feed.
struct Suggestion {
// Title of the suggestion.
string title;

// URL of the suggested page.
url.mojom.Url url;

// Name of the content's publisher.
string publisher_name;

// URL of the image associated with the suggestion.
url.mojom.Url image_url;

// URL of the suggested page's favicon.
url.mojom.Url favicon_url;
};

// Browser interface for the page. Consists of calls for data and hooks for
// interactivity.
interface PageHandler {
// Get general property values.
GetGeneralProperties() => (Properties properties);

// Get user classifier property values.
GetUserClassifierProperties() => (UserClassifier properties);

// Clear stored properties for the user classifier.
ClearUserClassifierProperties();

// Get last fetch data.
GetLastFetchProperties() => (LastFetchProperties properties);

// Clear all data cached by the Feed library. Also triggers a refresh of the
// Feed.
ClearCachedDataAndRefreshFeed();

// Trigger a refresh of the Feed.
RefreshFeed();
// Trigger a refresh of the For-you Feed.
RefreshForYouFeed();

// Get the last known content with metadata.
GetCurrentContent() => (array<Suggestion> suggestions);
// Trigger a refresh of the Following Feed.
RefreshFollowingFeed();

// Internal state dump of the Feed library's process scope. Human-readable.
GetFeedProcessScopeDump() => (string dump);
Expand Down
33 changes: 4 additions & 29 deletions chrome/browser/ui/webui/feed_internals/feedv2_internals_page_handler.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,13 +76,6 @@ void FeedV2InternalsPageHandler::GetGeneralProperties(
std::move(callback).Run(std::move(properties));
}

void FeedV2InternalsPageHandler::GetUserClassifierProperties(
GetUserClassifierPropertiesCallback callback) {
// TODO(crbug.com/1066230): Either implement this or remove it.

std::move(callback).Run(feed_internals::mojom::UserClassifier::New());
}

void FeedV2InternalsPageHandler::GetLastFetchProperties(
GetLastFetchPropertiesCallback callback) {
auto properties = feed_internals::mojom::LastFetchProperties::New();
Expand All @@ -103,30 +96,12 @@ void FeedV2InternalsPageHandler::GetLastFetchProperties(
std::move(callback).Run(std::move(properties));
}

void FeedV2InternalsPageHandler::ClearUserClassifierProperties() {
// TODO(crbug.com/1066230): Remove or implement this.
}

void FeedV2InternalsPageHandler::ClearCachedDataAndRefreshFeed() {
// TODO(crbug.com/1066230): Not sure we need to clear cache since we don't
// retain data on refresh.
feed_stream_->ForceRefreshForDebugging();
}

void FeedV2InternalsPageHandler::RefreshFeed() {
feed_stream_->ForceRefreshForDebugging();
void FeedV2InternalsPageHandler::RefreshForYouFeed() {
feed_stream_->ForceRefreshForDebugging(feed::kForYouStream);
}

void FeedV2InternalsPageHandler::GetCurrentContent(
GetCurrentContentCallback callback) {
if (!IsFeedAllowed()) {
std::move(callback).Run({});
return;
}
// TODO(crbug.com/1066230): Content metadata is (yet?) available. I wasn't
// able to get this to work for v1 either, so maybe it's not that important
// to implement. We should remove |GetCurrentContent| if it's not needed.
std::move(callback).Run({});
void FeedV2InternalsPageHandler::RefreshFollowingFeed() {
feed_stream_->ForceRefreshForDebugging(feed::kWebFeedStream);
}

void FeedV2InternalsPageHandler::GetFeedProcessScopeDump(
Expand Down
8 changes: 2 additions & 6 deletions chrome/browser/ui/webui/feed_internals/feedv2_internals_page_handler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,9 @@ class FeedV2InternalsPageHandler : public feed_internals::mojom::PageHandler {

// feed_internals::mojom::PageHandler
void GetGeneralProperties(GetGeneralPropertiesCallback) override;
void GetUserClassifierProperties(
GetUserClassifierPropertiesCallback) override;
void ClearUserClassifierProperties() override;
void GetLastFetchProperties(GetLastFetchPropertiesCallback) override;
void ClearCachedDataAndRefreshFeed() override;
void RefreshFeed() override;
void GetCurrentContent(GetCurrentContentCallback) override;
void RefreshForYouFeed() override;
void RefreshFollowingFeed() override;
void GetFeedProcessScopeDump(GetFeedProcessScopeDumpCallback) override;
void GetFeedHistograms(GetFeedHistogramsCallback) override;
void OverrideFeedHost(const GURL& host) override;
Expand Down
12 changes: 4 additions & 8 deletions components/feed/core/v2/api_test/feed_api_stream_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -401,24 +401,20 @@ TEST_F(FeedApiTest, BackgroundRefreshDiscoFeedEnabled) {
EXPECT_EQ("loading -> 2 slices", surface.DescribeUpdates());
}

TEST_F(FeedApiTest, ForceRefreshForDebugging) {
TEST_P(FeedStreamTestForAllStreamTypes, ForceRefreshForDebugging) {
// WebFeed stream is only fetched when there's a subscription.
network_.InjectListWebFeedsResponse({MakeWireWebFeed("cats")});

// Force a refresh that results in a successful load of both feed types.
response_translator_.InjectResponse(MakeTypicalInitialModelState());
response_translator_.InjectResponse(MakeTypicalInitialModelState());
stream_->ForceRefreshForDebugging();

stream_->ForceRefreshForDebugging(GetStreamType());

WaitForIdleTaskQueue();

is_offline_ = true;

TestForYouSurface surface(stream_.get());
TestWebFeedSurface web_feed_surface(stream_.get());
TestSurface surface(stream_.get());
WaitForIdleTaskQueue();
EXPECT_EQ("2 slices", surface.DescribeState());
EXPECT_EQ("2 slices", web_feed_surface.DescribeState());
}

TEST_F(FeedApiTest, RefreshScheduleFlow) {
Expand Down
24 changes: 9 additions & 15 deletions components/feed/core/v2/feed_stream.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -653,12 +653,12 @@ DebugStreamData FeedStream::GetDebugStreamData() {
return ::feed::prefs::GetDebugStreamData(*profile_prefs_);
}

void FeedStream::ForceRefreshForDebugging() {
void FeedStream::ForceRefreshForDebugging(const StreamType& stream_type) {
// Avoid request throttling for debug refreshes.
feed::prefs::SetThrottlerRequestCounts({}, *profile_prefs_);
task_queue_.AddTask(
std::make_unique<offline_pages::ClosureTask>(base::BindOnce(
&FeedStream::ForceRefreshForDebuggingTask, base::Unretained(this))));
task_queue_.AddTask(std::make_unique<offline_pages::ClosureTask>(
base::BindOnce(&FeedStream::ForceRefreshForDebuggingTask,
base::Unretained(this), stream_type)));
}

void FeedStream::ForceRefreshTask(const StreamType& stream_type) {
Expand All @@ -672,19 +672,13 @@ void FeedStream::ForceRefreshTask(const StreamType& stream_type) {
}
}

void FeedStream::ForceRefreshForDebuggingTask() {
UnloadModel(kForYouStream);
store_->ClearStreamData(kForYouStream, base::DoNothing());
GetStream(kForYouStream)
void FeedStream::ForceRefreshForDebuggingTask(const StreamType& stream_type) {
UnloadModel(stream_type);
store_->ClearStreamData(stream_type, base::DoNothing());
GetStream(stream_type)
.surface_updater->launch_reliability_logger()
.LogFeedLaunchOtherStart();
TriggerStreamLoad(kForYouStream);

if (base::FeatureList::IsEnabled(kWebFeed)) {
UnloadModel(kWebFeedStream);
store_->ClearStreamData(kWebFeedStream, base::DoNothing());
// WebFeed is refreshed automatically after for-you.
}
TriggerStreamLoad(stream_type);
}

std::string FeedStream::DumpStateForDebugging() {
Expand Down
5 changes: 3 additions & 2 deletions components/feed/core/v2/feed_stream.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "components/feed/core/v2/persistent_key_value_store_impl.h"
#include "components/feed/core/v2/protocol_translator.h"
#include "components/feed/core/v2/public/feed_api.h"
#include "components/feed/core/v2/public/stream_type.h"
#include "components/feed/core/v2/request_throttler.h"
#include "components/feed/core/v2/scheduling.h"
#include "components/feed/core/v2/stream/notice_card_tracker.h"
Expand Down Expand Up @@ -131,7 +132,7 @@ class FeedStream : public FeedApi,
void ProcessViewAction(base::StringPiece data) override;
bool WasUrlRecentlyNavigatedFromFeed(const GURL& url) override;
DebugStreamData GetDebugStreamData() override;
void ForceRefreshForDebugging() override;
void ForceRefreshForDebugging(const StreamType& stream_type) override;
std::string DumpStateForDebugging() override;
void SetForcedStreamUpdateForDebugging(
const feedui::StreamUpdate& stream_update) override;
Expand Down Expand Up @@ -332,7 +333,7 @@ class FeedStream : public FeedApi,

// A single function task to delete stored feed data and force a refresh.
// To only be called from within a |Task|.
void ForceRefreshForDebuggingTask();
void ForceRefreshForDebuggingTask(const StreamType& stream_type);
void ForceRefreshTask(const StreamType& stream_type);

void ScheduleModelUnloadIfNoSurfacesAttached(const StreamType& stream_type);
Expand Down
Loading

0 comments on commit a9a6576

Please sign in to comment.