[Feed] Trigger refresh after sign in and out.

Previously the scheduler was being directly called by the
ChromeBrowsingDataRemoverDelegate when history was deleted. When this
happened, we cleared internal state and suppressed refreshes. This
sometimes coincided with when the FeedAppLifecycle cleared all stored
articles, but not very well. Particularly, when:
* Signing in
* Signing out
* Clear the cache

So instead, the scheduler is notified by the FeedAppLifecycle exactly
when all articles are cleared. A boolean is also passed along to inform
the scheduler which of the two states are we in. Did something get cleared
and we should suppress refreshing? Or do we now have no data and need to
refresh immediately.

Bug: 905959
Change-Id: I25c57022a0da52cc42746d265beae1c55f84d3ec
Reviewed-on: https://chromium-review.googlesource.com/c/1355994
Commit-Queue: Sky Malice <skym@chromium.org>
Reviewed-by: Filip Gorski <fgorski@chromium.org>
Reviewed-by: Martin Šrámek <msramek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612848}

chrome/android/feed/core/java/src/org/chromium/chrome/browser/feed/FeedAppLifecycle.java

@@ -102,7 +102,7 @@ public class FeedAppLifecycle
      * We call onClearAll to avoid presenting personalized suggestions based on deleted history.
      */
     public void onHistoryDeleted() {
-        onClearAll();
+        onClearAll(/*suppressRefreshes*/ true);
     }
 
     /**
@@ -110,7 +110,7 @@ public class FeedAppLifecycle
      * Feed deletes its cached browsing data.
      */
     public void onCachedDataCleared() {
-        onClearAll();
+        onClearAll(/*suppressRefreshes*/ false);
     }
 
     /**
@@ -152,12 +152,12 @@ public class FeedAppLifecycle
 
     @Override
     public void onSignedIn() {
-        onClearAll();
+        onClearAll(/*suppressRefreshes*/ false);
     }
 
     @Override
     public void onSignedOut() {
-        onClearAll();
+        onClearAll(/*suppressRefreshes*/ false);
     }
 
     private void onEnterForeground() {
@@ -170,9 +170,14 @@ public class FeedAppLifecycle
         mAppLifecycleListener.onEnterBackground();
     }
 
-    private void onClearAll() {
+    private void onClearAll(boolean suppressRefreshes) {
         reportEvent(AppLifecycleEvent.CLEAR_ALL);
+        // It is important that #onClearAll() is called before notifying the scheduler, otherwise
+        // the clear all could wipe out the new results. These are both async operations that are
+        // kicked off here, but the Feed is responsible for tracking them and making sure they're
+        // correctly respected.
         mAppLifecycleListener.onClearAll();
+        mFeedScheduler.onArticlesCleared(suppressRefreshes);
     }
 
     private void initialize() {

chrome/android/feed/core/java/src/org/chromium/chrome/browser/feed/FeedScheduler.java

@@ -17,9 +17,19 @@ public interface FeedScheduler extends SchedulerApi {
     /** To be called whenever the browser is foregrounded. */
     void onForegrounded();
 
-    /** To be called when a background scheduling task wakes up the browser. */
+    /**
+     * To be called when a background scheduling task wakes up the browser.
+     * @param onCompletion to be run when the fixed timer logic is complete.
+     */
     void onFixedTimer(Runnable onCompletion);
 
     /** To be called when an article is consumed, influencing future scheduling. */
     void onSuggestionConsumed();
+
+    /**
+     * To be called when articles are cleared.
+     * @param suppressRefreshes whether the scheduler should temporarily avoid kicking off
+     * refreshes. This is used, for example, when history data is deleted.
+     */
+    void onArticlesCleared(boolean suppressRefreshes);
 }

chrome/android/feed/core/java/src/org/chromium/chrome/browser/feed/FeedSchedulerBridge.java

@@ -124,6 +124,12 @@ public class FeedSchedulerBridge implements FeedScheduler {
         nativeOnSuggestionConsumed(mNativeBridge);
     }
 
+    @Override
+    public void onArticlesCleared(boolean suppressRefreshes) {
+        assert mNativeBridge != 0;
+        nativeOnArticlesCleared(mNativeBridge, suppressRefreshes);
+    }
+
     @CalledByNative
     private boolean triggerRefresh() {
         if (mRequestManager != null && mSessionManager != null) {
@@ -156,4 +162,6 @@ public class FeedSchedulerBridge implements FeedScheduler {
     private native void nativeOnFixedTimer(
             long nativeFeedSchedulerBridge, Callback<Void> onCompletion);
     private native void nativeOnSuggestionConsumed(long nativeFeedSchedulerBridge);
+    private native void nativeOnArticlesCleared(
+            long nativeFeedSchedulerBridge, boolean suppressRefreshes);
 }

chrome/android/javatests/src/org/chromium/chrome/browser/feed/FeedAppLifecycleTest.java

@@ -161,6 +161,7 @@ public class FeedAppLifecycleTest {
         assertEquals(1,
                 RecordHistogram.getHistogramValueCountForTesting(
                         mHistogramAppLifecycleEvents, AppLifecycleEvent.CLEAR_ALL));
+        verify(mFeedScheduler, times(1)).onArticlesCleared(true);
     }
 
     @Test
@@ -173,6 +174,7 @@ public class FeedAppLifecycleTest {
         assertEquals(1,
                 RecordHistogram.getHistogramValueCountForTesting(
                         mHistogramAppLifecycleEvents, AppLifecycleEvent.CLEAR_ALL));
+        verify(mFeedScheduler, times(1)).onArticlesCleared(false);
     }
 
     @Test
@@ -185,6 +187,7 @@ public class FeedAppLifecycleTest {
         assertEquals(1,
                 RecordHistogram.getHistogramValueCountForTesting(
                         mHistogramAppLifecycleEvents, AppLifecycleEvent.CLEAR_ALL));
+        verify(mFeedScheduler, times(1)).onArticlesCleared(false);
     }
 
     @Test
@@ -197,6 +200,7 @@ public class FeedAppLifecycleTest {
         assertEquals(1,
                 RecordHistogram.getHistogramValueCountForTesting(
                         mHistogramAppLifecycleEvents, AppLifecycleEvent.CLEAR_ALL));
+        verify(mFeedScheduler, times(1)).onArticlesCleared(false);
     }
 
     @Test

chrome/browser/android/feed/feed_scheduler_bridge.cc

@@ -100,6 +100,13 @@ void FeedSchedulerBridge::OnSuggestionConsumed(
   scheduler_host_->OnSuggestionConsumed();
 }
 
+void FeedSchedulerBridge::OnArticlesCleared(
+    JNIEnv* env,
+    const base::android::JavaRef<jobject>& j_this,
+    jboolean j_suppress_refreshes) {
+  scheduler_host_->OnArticlesCleared(j_suppress_refreshes);
+}
+
 void FeedSchedulerBridge::TriggerRefresh() {
   JNIEnv* env = base::android::AttachCurrentThread();
   Java_FeedSchedulerBridge_triggerRefresh(env, j_this_);

chrome/browser/android/feed/feed_scheduler_bridge.h

@@ -56,6 +56,10 @@ class FeedSchedulerBridge {
   void OnSuggestionConsumed(JNIEnv* env,
                             const base::android::JavaRef<jobject>& j_this);
 
+  void OnArticlesCleared(JNIEnv* env,
+                         const base::android::JavaRef<jobject>& j_this,
+                         jboolean j_suppress_refreshes);
+
  private:
   // Callable by native code to invoke Java code. Sends a request to the Feed
   // library to make the refresh call.

chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.cc

@@ -72,7 +72,6 @@
 #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_settings.h"
 #include "components/device_event_log/device_event_log.h"
 #include "components/feed/content/feed_host_service.h"
-#include "components/feed/core/feed_scheduler_host.h"
 #include "components/history/core/browser/history_service.h"
 #include "components/language/core/browser/url_language_histogram.h"
 #include "components/nacl/browser/nacl_browser.h"
@@ -418,16 +417,6 @@ void ChromeBrowsingDataRemoverDelegate::RemoveEmbedderData(
                                                   filter, bookmark_model);
     }
 
-#if defined(OS_ANDROID)
-#if BUILDFLAG(ENABLE_FEED_IN_CHROME)
-    feed::FeedHostService* feed_host_service =
-        feed::FeedHostServiceFactory::GetForBrowserContext(profile_);
-    if (feed_host_service) {
-      feed_host_service->GetSchedulerHost()->OnHistoryCleared();
-    }
-#endif  // BUILDFLAG(ENABLE_FEED_IN_CHROME)
-#endif  // defined(OS_ANDROID)
-
     language::UrlLanguageHistogram* language_histogram =
         UrlLanguageHistogramFactory::GetForBrowserContext(profile_);
     if (language_histogram) {

chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc

@@ -73,7 +73,6 @@
 #include "components/domain_reliability/clear_mode.h"
 #include "components/domain_reliability/monitor.h"
 #include "components/favicon/core/favicon_service.h"
-#include "components/feed/core/pref_names.h"
 #include "components/history/core/browser/history_service.h"
 #include "components/language/core/browser/url_language_histogram.h"
 #include "components/ntp_snippets/bookmarks/bookmark_last_visit_utils.h"
@@ -2903,17 +2902,4 @@ TEST_F(ChromeBrowsingDataRemoverDelegateTest, WipeOriginVerifierData) {
   EXPECT_EQ(before + 1,
       customtabs::OriginVerifier::GetClearBrowsingDataCallCountForTesting());
 }
-
-#if BUILDFLAG(ENABLE_FEED_IN_CHROME)
-TEST_F(ChromeBrowsingDataRemoverDelegateTest, FeedClearsLastFetchAttempt) {
-  PrefService* prefs = GetProfile()->GetPrefs();
-  prefs->SetTime(feed::prefs::kLastFetchAttemptTime, base::Time::Now());
-
-  BlockUntilBrowsingDataRemoved(
-      base::Time(), base::Time::Max(),
-      ChromeBrowsingDataRemoverDelegate::DATA_TYPE_HISTORY, false);
-
-  EXPECT_EQ(base::Time(), prefs->GetTime(feed::prefs::kLastFetchAttemptTime));
-}
-#endif  // BUILDFLAG(ENABLE_FEED_IN_CHROME)
 #endif  // defined(OS_ANDROID)

components/feed/core/feed_scheduler_host.cc

@@ -316,15 +316,20 @@ void FeedSchedulerHost::OnSuggestionsShown() {
   user_classifier_.OnEvent(UserClassifier::Event::kSuggestionsViewed);
 }
 
-void FeedSchedulerHost::OnHistoryCleared() {
-  // Due to privacy, we should not fetch for a while (unless the user explicitly
-  // asks for new suggestions) to give sync the time to propagate the changes in
-  // history to the server.
+void FeedSchedulerHost::OnArticlesCleared(bool suppress_refreshes) {
+  // Since there are no stored articles, a refresh will be needed soon.
+  profile_prefs_->ClearPref(prefs::kLastFetchAttemptTime);
+
+  if (suppress_refreshes) {
+    // Due to privacy, we should not fetch for a while (unless the user
+    // explicitly asks for new suggestions) to give sync the time to propagate
+    // the changes in history to the server.
     suppress_refreshes_until_ =
         clock_->Now() +
         base::TimeDelta::FromMinutes(kSuppressRefreshDurationMinutes.Get());
-  // After that time elapses, we should fetch as soon as possible.
-  profile_prefs_->ClearPref(prefs::kLastFetchAttemptTime);
+  } else if (ShouldRefresh(TriggerType::kNtpShown)) {
+    refresh_callback_.Run();
+  }
 }
 
 void FeedSchedulerHost::OnEulaAccepted() {

components/feed/core/feed_scheduler_host.h

@@ -109,9 +109,10 @@ class FeedSchedulerHost : web_resource::EulaAcceptedNotifier::Observer {
   // users to track the kind of user, and optimize refresh frequency.
   void OnSuggestionsShown();
 
-  // When the user clears history, the scheduler will clear out some stored data
-  // and stop requesting refreshes for a period of time.
-  void OnHistoryCleared();
+  // Should be called when something happens to clear stored articles. The
+  // scheduler updates its internal state and treats this event as a kNtpShown
+  // trigger.
+  void OnArticlesCleared(bool suppress_refreshes);
 
  private:
   FRIEND_TEST_ALL_PREFIXES(FeedSchedulerHostTest, GetTriggerThreshold);

components/feed/core/feed_scheduler_host_unittest.cc

@@ -786,13 +786,13 @@ TEST_F(FeedSchedulerHostTest, DisableBogusTriggers) {
                 /*has_outstanding_request*/ false));
 }
 
-TEST_F(FeedSchedulerHostTest, OnHistoryCleared) {
+TEST_F(FeedSchedulerHostTest, OnArticlesCleared) {
   // OnForegrounded() does nothing because content is fresher than threshold.
   scheduler()->OnReceiveNewContent(test_clock()->Now());
   scheduler()->OnForegrounded();
   EXPECT_EQ(0, refresh_call_count());
 
-  scheduler()->OnHistoryCleared();
+  scheduler()->OnArticlesCleared(/*suppress_refreshes*/ true);
 
   scheduler()->OnForegrounded();
   EXPECT_EQ(0, refresh_call_count());
@@ -823,7 +823,7 @@ TEST_F(FeedSchedulerHostTest, SuppressRefreshDuration) {
       kInterestFeedContentSuggestions.name,
       {{kSuppressRefreshDurationMinutes.name, "100"}},
       {kInterestFeedContentSuggestions.name});
-  scheduler()->OnHistoryCleared();
+  scheduler()->OnArticlesCleared(/*suppress_refreshes*/ true);
 
   test_clock()->Advance(TimeDelta::FromMinutes(99));
   scheduler()->OnForegrounded();
@@ -834,6 +834,24 @@ TEST_F(FeedSchedulerHostTest, SuppressRefreshDuration) {
   EXPECT_EQ(1, refresh_call_count());
 }
 
+TEST_F(FeedSchedulerHostTest, OnArticlesClearedNoSupress) {
+  EXPECT_EQ(0, refresh_call_count());
+
+  scheduler()->OnArticlesCleared(/*suppress_refreshes*/ false);
+  EXPECT_EQ(1, refresh_call_count());
+  scheduler()->OnReceiveNewContent(test_clock()->Now());
+
+  scheduler()->OnArticlesCleared(/*suppress_refreshes*/ false);
+  EXPECT_EQ(2, refresh_call_count());
+  scheduler()->OnReceiveNewContent(test_clock()->Now());
+
+  scheduler()->OnArticlesCleared(/*suppress_refreshes*/ true);
+  EXPECT_EQ(2, refresh_call_count());
+
+  scheduler()->OnArticlesCleared(/*suppress_refreshes*/ false);
+  EXPECT_EQ(2, refresh_call_count());
+}
+
 TEST_F(FeedSchedulerHostTest, OustandingRequest) {
   scheduler()->OnForegrounded();
   EXPECT_EQ(1, refresh_call_count());