feedv2: Unload the model when it's not needed

After all surfaces are detached, the model is eventually unloaded.
The timeout used is configurable via finch parameter.

Bug: 1044139
Change-Id: I015945215c1a0cc25fff92ccf20a5d72f52c7309
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2216037
Commit-Queue: Dan H <harringtond@chromium.org>
Reviewed-by: Carlos Knippschild <carlosk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#772956}

components/feed/core/v2/config.cc

@@ -52,6 +52,11 @@ void OverrideWithFinch(Config* config) {
   config->max_action_upload_bytes = base::GetFieldTrialParamByFeatureAsInt(
       kInterestFeedV2, "max_action_upload_bytes",
       config->max_action_upload_bytes);
+
+  config->model_unload_timeout =
+      base::TimeDelta::FromSecondsD(base::GetFieldTrialParamByFeatureAsDouble(
+          kInterestFeedV2, "model_unload_timeout_seconds",
+          config->model_unload_timeout.InSecondsF()));
 }
 
 }  // namespace

components/feed/core/v2/config.h

@@ -28,6 +28,9 @@ struct Config {
   base::TimeDelta max_action_age = base::TimeDelta::FromHours(24);
   // Maximum payload size for one action upload batch.
   size_t max_action_upload_bytes = 20000;
+  // If no surfaces are attached, the stream model is unloaded after this
+  // timeout.
+  base::TimeDelta model_unload_timeout = base::TimeDelta::FromSeconds(1);
 };
 
 // Gets the current configuration.

components/feed/core/v2/feed_stream.cc

@@ -10,6 +10,7 @@
 
 #include "base/bind.h"
 #include "base/metrics/histogram_macros.h"
+#include "base/threading/thread_task_runner_handle.h"
 #include "base/time/clock.h"
 #include "base/time/tick_clock.h"
 #include "components/feed/core/common/pref_names.h"
@@ -17,6 +18,7 @@
 #include "components/feed/core/proto/v2/ui.pb.h"
 #include "components/feed/core/proto/v2/wire/there_and_back_again_data.pb.h"
 #include "components/feed/core/shared_prefs/pref_names.h"
+#include "components/feed/core/v2/config.h"
 #include "components/feed/core/v2/enums.h"
 #include "components/feed/core/v2/feed_network.h"
 #include "components/feed/core/v2/feed_store.h"
@@ -157,11 +159,36 @@ void FeedStream::AttachSurface(SurfaceInterface* surface) {
   metrics_reporter_->SurfaceOpened(surface->GetSurfaceId());
   TriggerStreamLoad();
   surface_updater_->SurfaceAdded(surface);
+  // Cancel any scheduled model unload task.
+  ++unload_on_detach_sequence_number_;
 }
 
 void FeedStream::DetachSurface(SurfaceInterface* surface) {
   metrics_reporter_->SurfaceClosed(surface->GetSurfaceId());
   surface_updater_->SurfaceRemoved(surface);
+  if (!surface_updater_->HasSurfaceAttached()) {
+    base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
+        FROM_HERE,
+        base::BindOnce(&FeedStream::AddUnloadModelIfNoSurfacesAttachedTask,
+                       GetWeakPtr(), unload_on_detach_sequence_number_),
+        GetFeedConfig().model_unload_timeout);
+  }
+}
+
+void FeedStream::AddUnloadModelIfNoSurfacesAttachedTask(int sequence_number) {
+  // Don't continue if unload_on_detach_sequence_number_ has changed.
+  if (unload_on_detach_sequence_number_ != sequence_number)
+    return;
+
+  task_queue_.AddTask(std::make_unique<offline_pages::ClosureTask>(
+      base::BindOnce(&FeedStream::UnloadModelIfNoSurfacesAttachedTask,
+                     base::Unretained(this))));
+}
+
+void FeedStream::UnloadModelIfNoSurfacesAttachedTask() {
+  if (surface_updater_->HasSurfaceAttached())
+    return;
+  UnloadModel();
 }
 
 void FeedStream::SetArticlesListVisible(bool is_visible) {

components/feed/core/v2/feed_stream.h

@@ -233,10 +233,18 @@ class FeedStream : public FeedStreamApi,
 
  private:
   class ModelStoreChangeMonitor;
+
+  base::WeakPtr<FeedStream> GetWeakPtr() {
+    return weak_ptr_factory_.GetWeakPtr();
+  }
+
   // A single function task to delete stored feed data and force a refresh.
   // To only be called from within a |Task|.
   void ForceRefreshForDebuggingTask();
 
+  void AddUnloadModelIfNoSurfacesAttachedTask(int sequence_number);
+  void UnloadModelIfNoSurfacesAttachedTask();
+
   void InitialStreamLoadComplete(LoadStreamTask::Result result);
   void LoadMoreComplete(LoadMoreTask::Result result);
   void BackgroundRefreshComplete(LoadStreamTask::Result result);
@@ -274,9 +282,12 @@ class FeedStream : public FeedStreamApi,
   base::TimeTicks suppress_refreshes_until_;
   std::vector<base::OnceCallback<void(bool)>> load_more_complete_callbacks_;
   Metadata metadata_;
+  int unload_on_detach_sequence_number_ = 0;
 
   // To allow tests to wait on task queue idle.
   base::RepeatingClosure idle_callback_;
+
+  base::WeakPtrFactory<FeedStream> weak_ptr_factory_{this};
 };
 
 }  // namespace feed

components/feed/core/v2/feed_stream_unittest.cc

@@ -401,6 +401,9 @@ class TestMetricsReporter : public MetricsReporter {
 class FeedStreamTest : public testing::Test, public FeedStream::Delegate {
  public:
   void SetUp() override {
+    // Reset to default config, since tests can change it.
+    SetFeedConfigForTesting(Config());
+
     feed::prefs::RegisterFeedSharedProfilePrefs(profile_prefs_.registry());
     feed::RegisterProfilePrefs(profile_prefs_.registry());
     metrics_reporter_ = std::make_unique<TestMetricsReporter>(
@@ -1430,5 +1433,75 @@ TEST_F(FeedStreamTest, MetadataLoadedWhenDatabaseInitialized) {
   EXPECT_EQ(1, stream_->GetMetadata()->GetNextActionId().GetUnsafeValue());
 }
 
+TEST_F(FeedStreamTest, ModelUnloadsAfterTimeout) {
+  Config config;
+  config.model_unload_timeout = base::TimeDelta::FromSeconds(1);
+  SetFeedConfigForTesting(config);
+
+  response_translator_.InjectResponse(MakeTypicalInitialModelState());
+  TestSurface surface(stream_.get());
+  WaitForIdleTaskQueue();
+
+  surface.Detach();
+
+  task_environment_.FastForwardBy(base::TimeDelta::FromMilliseconds(999));
+  WaitForIdleTaskQueue();
+  EXPECT_TRUE(stream_->GetModel());
+
+  task_environment_.FastForwardBy(base::TimeDelta::FromMilliseconds(2));
+  WaitForIdleTaskQueue();
+  EXPECT_FALSE(stream_->GetModel());
+}
+
+TEST_F(FeedStreamTest, ModelDoesNotUnloadIfSurfaceIsAttached) {
+  Config config;
+  config.model_unload_timeout = base::TimeDelta::FromSeconds(1);
+  SetFeedConfigForTesting(config);
+
+  response_translator_.InjectResponse(MakeTypicalInitialModelState());
+  TestSurface surface(stream_.get());
+  WaitForIdleTaskQueue();
+
+  surface.Detach();
+
+  task_environment_.FastForwardBy(base::TimeDelta::FromMilliseconds(999));
+  WaitForIdleTaskQueue();
+  EXPECT_TRUE(stream_->GetModel());
+
+  surface.Attach(stream_.get());
+
+  task_environment_.FastForwardBy(base::TimeDelta::FromMilliseconds(2));
+  WaitForIdleTaskQueue();
+  EXPECT_TRUE(stream_->GetModel());
+}
+
+TEST_F(FeedStreamTest, ModelUnloadsAfterSecondTimeout) {
+  Config config;
+  config.model_unload_timeout = base::TimeDelta::FromSeconds(1);
+  SetFeedConfigForTesting(config);
+
+  response_translator_.InjectResponse(MakeTypicalInitialModelState());
+  TestSurface surface(stream_.get());
+  WaitForIdleTaskQueue();
+
+  surface.Detach();
+
+  task_environment_.FastForwardBy(base::TimeDelta::FromMilliseconds(999));
+  WaitForIdleTaskQueue();
+  EXPECT_TRUE(stream_->GetModel());
+
+  // Attaching another surface will prolong the unload time for another second.
+  surface.Attach(stream_.get());
+  surface.Detach();
+
+  task_environment_.FastForwardBy(base::TimeDelta::FromMilliseconds(999));
+  WaitForIdleTaskQueue();
+  EXPECT_TRUE(stream_->GetModel());
+
+  task_environment_.FastForwardBy(base::TimeDelta::FromMilliseconds(2));
+  WaitForIdleTaskQueue();
+  EXPECT_FALSE(stream_->GetModel());
+}
+
 }  // namespace
 }  // namespace feed