Remove feed session based refresh
Land after downstream usage removal in https://crrev.com/i/6840074.
This CL is related to https://crrev.com/c/5173288, which cleans up
all ACR flag related code. But I extracted this "session" piece
separately because the removal was more complicated. This CL should
be a lot more readable than if I had jumbled it together.
Bug: 1479655
Change-Id: I6941b727ba708554ad8d94936964708bf001e360
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5172027
Reviewed-by: Sergio Collazos <sczs@chromium.org>
Commit-Queue: edchin <edchin@google.com>
Cr-Commit-Position: refs/heads/main@{#1243720}
diff --git a/ios/chrome/app/feed_app_agent.mm b/ios/chrome/app/feed_app_agent.mm
index 6ef0d01..cdd41543 100644
--- a/ios/chrome/app/feed_app_agent.mm
+++ b/ios/chrome/app/feed_app_agent.mm
@@ -206,13 +206,6 @@
});
};
- // The `engagedWithLatestRefreshedContent` criteria only applies to background
- // app close. Early return if criteria is not met.
- if (IsFeedAppCloseBackgroundRefreshEnabled() &&
- ![self.feedMetricsRecorder hasEngagedWithLatestRefreshedContent]) {
- return;
- }
-
// Cold starts are killed earlier in this method, so warm and cold starts
// cannot be recorded at the same time.
[self recordWarmStartMetrics];
@@ -245,9 +238,6 @@
// Record refresh trigger for warm start.
- (void)recordWarmStartMetrics {
- CHECK(!IsFeedAppCloseBackgroundRefreshEnabled() ||
- [self.feedMetricsRecorder hasEngagedWithLatestRefreshedContent]);
-
if (IsFeedAppCloseBackgroundRefreshEnabled()) {
// This is recorded if both app close and regular background refreshes are
// enabled.
diff --git a/ios/chrome/browser/shared/public/features/features.h b/ios/chrome/browser/shared/public/features/features.h
index 541ec390..7f808f1 100644
--- a/ios/chrome/browser/shared/public/features/features.h
+++ b/ios/chrome/browser/shared/public/features/features.h
@@ -326,17 +326,6 @@
// signed in non syncing users.
BASE_DECLARE_FEATURE(kLinkAccountSettingsToPrivacyFooter);
-// Engagement criteria type for a feed refresh.
-enum class FeedRefreshEngagementCriteriaType {
- // Any scroll or interaction.
- kSimpleEngagement = 0,
- // Meets minimum scroll criteria or any interaction.
- kEngagement = 1,
- // Meets good visit criteria.
- kGoodVisit = 2,
- kMaxValue = kGoodVisit,
-};
-
// Feature flag to enable feed background refresh.
// Use IsFeedBackgroundRefreshEnabled() instead of this constant directly.
BASE_DECLARE_FEATURE(kEnableFeedBackgroundRefresh);
@@ -410,10 +399,6 @@
extern const char kBackgroundRefreshMaxAgeInSeconds[];
// Feature param under `kEnableFeedInvisibleForegroundRefresh` to enable refresh
-// following a Feed session.
-extern const char kEnableFeedSessionCloseForegroundRefresh[];
-
-// Feature param under `kEnableFeedInvisibleForegroundRefresh` to enable refresh
// on app backgrounding.
extern const char kEnableFeedAppCloseForegroundRefresh[];
@@ -422,17 +407,9 @@
extern const char kEnableFeedAppCloseBackgroundRefresh[];
// Feature param under `kEnableFeedInvisibleForegroundRefresh` for the
-// engagement criteria type to refresh the feed.
-extern const char kFeedRefreshEngagementCriteriaType[];
-
-// Feature param under `kEnableFeedInvisibleForegroundRefresh` for the
// background refresh interval in seconds.
extern const char kAppCloseBackgroundRefreshIntervalInSeconds[];
-// Feature param under `kEnableFeedInvisibleForegroundRefresh` for the time
-// interval used to set the refresh timer.
-extern const char kFeedRefreshTimerTimeoutInSeconds[];
-
// Feature param under `kEnableFeedInvisibleForegroundRefresh` for the refresh
// threshold when the last refresh was seen.
extern const char kFeedSeenRefreshThresholdInSeconds[];
@@ -519,11 +496,6 @@
// Whether feed can be refreshed while not visible.
bool IsFeedInvisibleForegroundRefreshEnabled();
-// Whether feed is refreshed after the user ends a Feed session, but while the
-// app is still in the foreground (e.g., user switches tabs, user navigates away
-// from Feed in current tab).
-bool IsFeedSessionCloseForegroundRefreshEnabled();
-
// Whether feed is refreshed at the moment the app is backgrounding. This is
// different from background refresh.
bool IsFeedAppCloseForegroundRefreshEnabled();
@@ -532,16 +504,10 @@
// backgrounded, and the capability was enabled at startup.
bool IsFeedAppCloseBackgroundRefreshEnabled();
-// Returns the engagement criteria type for a feed refresh.
-FeedRefreshEngagementCriteriaType GetFeedRefreshEngagementCriteriaType();
-
// The earliest interval to refresh in the background after app enters the
// background in app close background refresh.
double GetAppCloseBackgroundRefreshIntervalInSeconds();
-// Returns the time interval used to set the session end timer.
-double GetFeedRefreshTimerTimeoutInSeconds();
-
// Returns the refresh threshold (aka feed expiration) for a feed that has been
// seen.
double GetFeedSeenRefreshThresholdInSeconds();
diff --git a/ios/chrome/browser/shared/public/features/features.mm b/ios/chrome/browser/shared/public/features/features.mm
index 5aebf9c1..c6b4523b 100644
--- a/ios/chrome/browser/shared/public/features/features.mm
+++ b/ios/chrome/browser/shared/public/features/features.mm
@@ -449,18 +449,12 @@
"BackgroundRefreshIntervalInSeconds";
const char kBackgroundRefreshMaxAgeInSeconds[] =
"BackgroundRefreshMaxAgeInSeconds";
-const char kEnableFeedSessionCloseForegroundRefresh[] =
- "EnableFeedSessionCloseForegroundRefresh";
const char kEnableFeedAppCloseForegroundRefresh[] =
"EnableFeedAppCloseForegroundRefresh";
const char kEnableFeedAppCloseBackgroundRefresh[] =
"EnableFeedAppCloseBackgroundRefresh";
-const char kFeedRefreshEngagementCriteriaType[] =
- "FeedRefreshEngagementCriteriaType";
const char kAppCloseBackgroundRefreshIntervalInSeconds[] =
"AppCloseBackgroundRefreshIntervalInSeconds";
-const char kFeedRefreshTimerTimeoutInSeconds[] =
- "FeedRefreshTimerTimeoutInSeconds";
const char kFeedSeenRefreshThresholdInSeconds[] =
"FeedSeenRefreshThresholdInSeconds";
const char kFeedUnseenRefreshThresholdInSeconds[] =
@@ -606,13 +600,6 @@
return base::FeatureList::IsEnabled(kEnableFeedInvisibleForegroundRefresh);
}
-bool IsFeedSessionCloseForegroundRefreshEnabled() {
- return base::GetFieldTrialParamByFeatureAsBool(
- kEnableFeedInvisibleForegroundRefresh,
- kEnableFeedSessionCloseForegroundRefresh,
- /*default=*/false);
-}
-
bool IsFeedAppCloseForegroundRefreshEnabled() {
return base::GetFieldTrialParamByFeatureAsBool(
kEnableFeedInvisibleForegroundRefresh,
@@ -625,15 +612,6 @@
IsFeedAppCloseBackgroundRefreshEnabledOnly();
}
-FeedRefreshEngagementCriteriaType GetFeedRefreshEngagementCriteriaType() {
- return (FeedRefreshEngagementCriteriaType)
- base::GetFieldTrialParamByFeatureAsInt(
- kEnableFeedInvisibleForegroundRefresh,
- kFeedRefreshEngagementCriteriaType,
- /*default_value=*/
- (int)FeedRefreshEngagementCriteriaType::kSimpleEngagement);
-}
-
double GetAppCloseBackgroundRefreshIntervalInSeconds() {
double override_value = [[NSUserDefaults standardUserDefaults]
doubleForKey:@"AppCloseBackgroundRefreshIntervalInSeconds"];
@@ -646,17 +624,6 @@
/*default=*/base::Minutes(5).InSecondsF());
}
-double GetFeedRefreshTimerTimeoutInSeconds() {
- double override_value = [[NSUserDefaults standardUserDefaults]
- doubleForKey:@"FeedRefreshTimerTimeoutInSeconds"];
- if (override_value > 0.0) {
- return override_value;
- }
- return base::GetFieldTrialParamByFeatureAsDouble(
- kEnableFeedInvisibleForegroundRefresh, kFeedRefreshTimerTimeoutInSeconds,
- /*default=*/base::Minutes(5).InSecondsF());
-}
-
double GetFeedSeenRefreshThresholdInSeconds() {
double override_value = [[NSUserDefaults standardUserDefaults]
doubleForKey:@"FeedSeenRefreshThresholdInSeconds"];
diff --git a/ios/chrome/browser/ui/ntp/metrics/BUILD.gn b/ios/chrome/browser/ui/ntp/metrics/BUILD.gn
index debf09e..7e045e4 100644
--- a/ios/chrome/browser/ui/ntp/metrics/BUILD.gn
+++ b/ios/chrome/browser/ui/ntp/metrics/BUILD.gn
@@ -23,7 +23,6 @@
"//components/ntp_tiles",
"//components/prefs",
"//ios/chrome/browser/discover_feed/model:constants",
- "//ios/chrome/browser/discover_feed/model:discover_feed_refresher",
"//ios/chrome/browser/metrics/model:constants",
"//ios/chrome/browser/shared/model/prefs:pref_names",
"//ios/chrome/browser/shared/public/features",
diff --git a/ios/chrome/browser/ui/ntp/metrics/feed_metrics_recorder.h b/ios/chrome/browser/ui/ntp/metrics/feed_metrics_recorder.h
index 71241a8..4a3abd9d 100644
--- a/ios/chrome/browser/ui/ntp/metrics/feed_metrics_recorder.h
+++ b/ios/chrome/browser/ui/ntp/metrics/feed_metrics_recorder.h
@@ -13,7 +13,6 @@
#import "ios/chrome/browser/ui/ntp/metrics/feed_metrics_constants.h"
#import "ios/chrome/browser/ui/ntp/metrics/feed_refresh_state_tracker.h"
-class DiscoverFeedRefresher;
@protocol FeedControlDelegate;
@protocol NewTabPageFollowDelegate;
@protocol NewTabPageMetricsDelegate;
@@ -35,9 +34,6 @@
// Whether or not the feed is currently being shown on the Start Surface.
@property(nonatomic, assign) BOOL isShownOnStartSurface;
-// Object that can refresh the feed.
-@property(nonatomic, assign) DiscoverFeedRefresher* feedRefresher;
-
// Delegate for reporting feed actions to the NTP metrics recorder.
@property(nonatomic, weak) id<NewTabPageMetricsDelegate> NTPMetricsDelegate;
diff --git a/ios/chrome/browser/ui/ntp/metrics/feed_metrics_recorder.mm b/ios/chrome/browser/ui/ntp/metrics/feed_metrics_recorder.mm
index 3534476..b136a1e 100644
--- a/ios/chrome/browser/ui/ntp/metrics/feed_metrics_recorder.mm
+++ b/ios/chrome/browser/ui/ntp/metrics/feed_metrics_recorder.mm
@@ -13,7 +13,6 @@
#import "base/metrics/user_metrics_action.h"
#import "base/time/time.h"
#import "components/prefs/pref_service.h"
-#import "ios/chrome/browser/discover_feed/model/discover_feed_refresher.h"
#import "ios/chrome/browser/metrics/model/constants.h"
#import "ios/chrome/browser/shared/public/features/features.h"
#import "ios/chrome/browser/ui/ntp/feed_control_delegate.h"
@@ -58,12 +57,6 @@
// metric.
@property(nonatomic, assign) NSDate* activityBucketLastReportedDate;
-// Tracks whether user has engaged with the latest refreshed content. The term
-// "engaged" is defined by its usage in this file. For example, it may be
-// similar to `engagedSimpleReportedDiscover`.
-@property(nonatomic, assign, getter=hasEngagedWithLatestRefreshedContent)
- BOOL engagedWithLatestRefreshedContent;
-
// Tracking property to record a scroll for Good Visits.
// TODO(crbug.com/1373650) separate the property below in two, one for each
// feed.
@@ -91,9 +84,6 @@
// `ContentSuggestions.Feed.TimeSpentInFeed`
@property(nonatomic, assign) base::TimeDelta timeSpentInFeed;
-// Timer to refresh the feed.
-@property(nonatomic, strong) NSTimer* refreshTimer;
-
// YES if the NTP is visible.
@property(nonatomic, assign) BOOL isNTPVisible;
@@ -124,11 +114,6 @@
#pragma mark - Public
-- (void)dealloc {
- [self.refreshTimer invalidate];
- self.refreshTimer = nil;
-}
-
+ (void)recordFeedRefreshTrigger:(FeedRefreshTrigger)trigger {
base::UmaHistogramEnumeration(kDiscoverFeedRefreshTrigger, trigger);
}
@@ -180,10 +165,7 @@
- (void)recordNTPDidChangeVisibility:(BOOL)visible {
self.isNTPVisible = visible;
- // Invalidate the timer when the user returns to the feed since the feed
- // should not be refreshed when the user is viewing it.
if (visible) {
- [self.refreshTimer invalidate];
[self recordDiscoverFeedUserActionHistogram:FeedUserActionType::
kOpenedFeedSurface
asInteraction:NO];
@@ -584,10 +566,6 @@
- (void)recordFeedWillRefresh {
base::RecordAction(base::UserMetricsAction(kFeedWillRefresh));
- // The feed will have new content so reset the engagement tracking variable.
- // TODO(crbug.com/1423467): We need to know whether the feed was actually
- // refreshed, and not just when it was triggered.
- self.engagedWithLatestRefreshedContent = NO;
}
- (void)recordFeedSelected:(FeedType)feedType
@@ -1040,10 +1018,6 @@
// Chrome run.
if (scrollDistance > 0 || interacted) {
[self recordEngagedSimple];
- if (GetFeedRefreshEngagementCriteriaType() ==
- FeedRefreshEngagementCriteriaType::kSimpleEngagement) {
- self.engagedWithLatestRefreshedContent = YES;
- }
}
// Report the user as engaged if they have scrolled more than the threshold or
@@ -1051,20 +1025,9 @@
// Chrome run.
if (scrollDistance > kMinScrollThreshold || interacted) {
[self recordEngaged];
- if (GetFeedRefreshEngagementCriteriaType() ==
- FeedRefreshEngagementCriteriaType::kEngagement) {
- self.engagedWithLatestRefreshedContent = YES;
- }
}
[self.sessionRecorder recordUserInteractionOrScrolling];
-
- // This must be called after setting `engagedWithLatestRefreshedContent`
- // properly after scrolling or interactions.
- if (IsFeedSessionCloseForegroundRefreshEnabled() &&
- [self hasEngagedWithLatestRefreshedContent]) {
- [self setOrExtendRefreshTimer];
- }
}
// Checks if a Good Visit should be recorded. `interacted` is YES if it was
@@ -1250,10 +1213,6 @@
UMA_HISTOGRAM_ENUMERATION(kDiscoverFeedEngagementTypeHistogram,
FeedEngagementType::kGoodVisit);
self.goodVisitReportedDiscover = YES;
- if (GetFeedRefreshEngagementCriteriaType() ==
- FeedRefreshEngagementCriteriaType::kGoodVisit) {
- self.engagedWithLatestRefreshedContent = YES;
- }
}
// Log interaction for Following feed.
@@ -1429,29 +1388,6 @@
}
}
-// Sets or extends the refresh timer.
-- (void)setOrExtendRefreshTimer {
- [self.refreshTimer invalidate];
- __weak FeedMetricsRecorder* weakSelf = self;
- self.refreshTimer = [NSTimer
- scheduledTimerWithTimeInterval:GetFeedRefreshTimerTimeoutInSeconds()
- target:weakSelf
- selector:@selector(refreshTimerEnded)
- userInfo:nil
- repeats:NO];
-}
-
-// Signals that the refresh timer ended.
-- (void)refreshTimerEnded {
- [self.refreshTimer invalidate];
- self.refreshTimer = nil;
- if (!self.isNTPVisible) {
- // The feed refresher checks feed engagement criteria.
- self.feedRefresher->RefreshFeed(
- FeedRefreshTrigger::kForegroundFeedNotVisible);
- }
-}
-
#pragma mark - Converters
// Converts a FollowingFeedSortType NSEnum into a FeedSortType enum.
diff --git a/ios/chrome/browser/ui/ntp/metrics/feed_refresh_state_tracker.h b/ios/chrome/browser/ui/ntp/metrics/feed_refresh_state_tracker.h
index 6956d5b..2a173dc2 100644
--- a/ios/chrome/browser/ui/ntp/metrics/feed_refresh_state_tracker.h
+++ b/ios/chrome/browser/ui/ntp/metrics/feed_refresh_state_tracker.h
@@ -10,10 +10,6 @@
// content.
@protocol FeedRefreshStateTracker
-// Returns YES if the user has engaged with the latest refreshed content. The
-// term "engaged" is an implementation detail of the receiver.
-- (BOOL)hasEngagedWithLatestRefreshedContent;
-
// Returns YES if the NTP is visible to the user.
- (BOOL)isNTPVisible;
