[Feed] StreamLifecycleManager respect prefs::kArticlesListVisible

Bug: 907282
Change-Id: I239dfb4ed667e6584bd598f7221003271ee7185b
Reviewed-on: https://chromium-review.googlesource.com/c/1347270Reviewed-by: Theresa <twellington@chromium.org>
Reviewed-by: Sky Malice <skym@chromium.org>
Commit-Queue: Becky Zhou <huayinz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612277}

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

@@ -282,6 +282,13 @@ public class FeedNewTabPage extends NewTabPage {
         mMediator.onThumbnailCaptured();
     }
 
+    /**
+     * @return The {@link StreamLifecycleManager} that manages the lifecycle of the {@link Stream}.
+     */
+    StreamLifecycleManager getStreamLifecycleManager() {
+        return mStreamLifecycleManager;
+    }
+
     /** @return The {@link Stream} that this class holds. */
     Stream getStream() {
         return mStream;
@@ -362,6 +369,9 @@ public class FeedNewTabPage extends NewTabPage {
     void createScrollViewForPolicy() {
         if (mStream != null) {
             mRootView.removeView(mStream.getView());
+            assert mStreamLifecycleManager
+                    != null
+                : "StreamLifecycleManager should not be null when the Stream is not null.";
             mStreamLifecycleManager.destroy();
             mStreamLifecycleManager = null;
             // Do not call mStream.onDestroy(), the mStreamLifecycleManager has done that for us.

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

@@ -152,14 +152,20 @@ class FeedNewTabPageMediator
         if (stream == null) return;
 
         stream.removeScrollListener(mStreamScrollListener);
-        stream.removeOnContentChangedListener(mStreamContentChangedListener);
-        MemoryPressureListener.removeCallback(mMemoryPressureCallback);
-        if (mSignInPromo != null) mSignInPromo.destroy();
-        mPrefChangeRegistrar.removeObserver(Pref.NTP_ARTICLES_LIST_VISIBLE);
         mStreamScrollListener = null;
+
+        stream.removeOnContentChangedListener(mStreamContentChangedListener);
         mStreamContentChangedListener = null;
+
+        MemoryPressureListener.removeCallback(mMemoryPressureCallback);
         mMemoryPressureCallback = null;
-        mSignInPromo = null;
+
+        if (mSignInPromo != null) {
+            mSignInPromo.destroy();
+            mSignInPromo = null;
+        }
+
+        mPrefChangeRegistrar.removeObserver(Pref.NTP_ARTICLES_LIST_VISIBLE);
     }
 
     /**
@@ -178,6 +184,7 @@ class FeedNewTabPageMediator
         if (mSignInPromo != null) {
             mSignInPromo.setCanShowPersonalizedSuggestions(suggestionsVisible);
         }
+        if (suggestionsVisible) mCoordinator.getStreamLifecycleManager().activate();
         mStreamContentChanged = true;
     }
 

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

@@ -29,9 +29,20 @@ import java.util.concurrent.Executors;
 public class FeedProcessScopeFactory {
     private static final String TAG = "FeedProcessScopeFtry";
 
-    /** Flag that tracks whether we've ever been disabled via enterprise policy. Should only be
-     * accessed through isFeedProcessScopeEnabled(). */
+    /**
+     * Flag that tracks whether we've ever been disabled via enterprise policy. Should only be
+     * accessed through isFeedProcessScopeEnabled().
+     */
     private static boolean sEverDisabledForPolicy;
+
+    /**
+     * Tracks whether the article suggestions should be visible to the user during the current
+     * session. If user opts in to the suggestions during the current session, the suggestions
+     * services will be immediately warmed up. If user opts out during the current session,
+     * the suggestions services will not shut down until the next session.
+     */
+    private static boolean sArticlesVisibleDuringSession;
+
     private static PrefChangeRegistrar sPrefChangeRegistrar;
     private static FeedAppLifecycle sFeedAppLifecycle;
     private static FeedProcessScope sFeedProcessScope;
@@ -109,6 +120,8 @@ public class FeedProcessScopeFactory {
                 && sFeedAppLifecycle == null && sFeedLoggingBridge == null;
         if (!isFeedProcessEnabled()) return;
 
+        sArticlesVisibleDuringSession =
+                PrefServiceBridge.getInstance().getBoolean(Pref.NTP_ARTICLES_LIST_VISIBLE);
         sPrefChangeRegistrar = new PrefChangeRegistrar();
         sPrefChangeRegistrar.addObserver(Pref.NTP_ARTICLES_SECTION_ENABLED,
                 FeedProcessScopeFactory::articlesEnabledPrefChange);
@@ -193,6 +206,21 @@ public class FeedProcessScopeFactory {
         destroy();
     }
 
+    /**
+     * @return Whether article suggestions are prepared to be shown based on user preference. If
+     *         article suggestions are set hidden within a session, this will still return true
+     *         until the next restart.
+     */
+    static boolean areArticlesVisibleDuringSession() {
+        // Skip the native call if sArticlesVisibleDuringSession is already true to reduce overhead.
+        if (!sArticlesVisibleDuringSession
+                && PrefServiceBridge.getInstance().getBoolean(Pref.NTP_ARTICLES_LIST_VISIBLE)) {
+            sArticlesVisibleDuringSession = true;
+        }
+
+        return sArticlesVisibleDuringSession;
+    }
+
     private static void articlesEnabledPrefChange() {
         // Should only be subscribed while it was enabled. A change should mean articles are now
         // disabled.

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

@@ -136,8 +136,11 @@ class StreamLifecycleManager implements ApplicationStatus.ActivityStateListener
     /** @return Whether the {@link Stream} can be shown. */
     private boolean canShow() {
         final int state = ApplicationStatus.getStateForActivity(mActivity);
+        // We don't call Stream#onShow to prevent feed services from being warmed up if the user
+        // has opted out from article suggestions during the previous session.
         return (mStreamState == CREATED || mStreamState == HIDDEN) && !mTab.isHidden()
-                && (state == ActivityState.STARTED || state == ActivityState.RESUMED);
+                && (state == ActivityState.STARTED || state == ActivityState.RESUMED)
+                && FeedProcessScopeFactory.areArticlesVisibleDuringSession();
     }
 
     /** Calls {@link Stream#onShow()}. */
@@ -150,15 +153,17 @@ class StreamLifecycleManager implements ApplicationStatus.ActivityStateListener
 
     /** @return Whether the {@link Stream} can be activated. */
     private boolean canActivate() {
-        return mStreamState != ACTIVE && mStreamState != DESTROYED && mTab.isUserInteractable()
-                && ApplicationStatus.getStateForActivity(mActivity) == ActivityState.RESUMED;
+        return (mStreamState == SHOWN || mStreamState == INACTIVE) && mTab.isUserInteractable()
+                && ApplicationStatus.getStateForActivity(mActivity) == ActivityState.RESUMED
+                && FeedProcessScopeFactory.areArticlesVisibleDuringSession();
     }
 
     /** Calls {@link Stream#onActive()}. */
-    private void activate() {
+    void activate() {
+        // Make sure the Stream can be shown and is set shown before setting it to active state.
+        show();
         if (!canActivate()) return;
 
-        show();
         mStreamState = ACTIVE;
         mStream.onActive();
     }
@@ -175,6 +180,7 @@ class StreamLifecycleManager implements ApplicationStatus.ActivityStateListener
     private void hide() {
         if (mStreamState == HIDDEN || mStreamState == CREATED || mStreamState == DESTROYED) return;
 
+        // Make sure the Stream is inactive before setting it to hidden state.
         deactivate();
         mStreamState = HIDDEN;
         // Save instance state as the Stream begins to hide. This matches the activity lifecycle
@@ -190,6 +196,7 @@ class StreamLifecycleManager implements ApplicationStatus.ActivityStateListener
     void destroy() {
         if (mStreamState == DESTROYED) return;
 
+        // Make sure the Stream is hidden before setting it to destroyed state.
         hide();
         mStreamState = DESTROYED;
         mTab.removeObserver(mTabObserver);

chrome/android/junit/src/org/chromium/chrome/browser/feed/StreamLifecycleManagerTest.java

@@ -6,7 +6,10 @@ package org.chromium.chrome.browser.feed;
 
 import static org.mockito.AdditionalMatchers.or;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyBoolean;
+import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.ArgumentMatchers.isNull;
+import static org.mockito.Mockito.doNothing;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
@@ -20,6 +23,7 @@ import android.support.test.filters.SmallTest;
 
 import com.google.android.libraries.feed.api.stream.Stream;
 
+import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -32,6 +36,8 @@ import org.robolectric.annotation.Config;
 import org.chromium.base.ActivityState;
 import org.chromium.base.ApplicationStatus;
 import org.chromium.base.test.BaseRobolectricTestRunner;
+import org.chromium.chrome.browser.preferences.Pref;
+import org.chromium.chrome.browser.preferences.PrefServiceBridge;
 import org.chromium.chrome.browser.tab.Tab;
 import org.chromium.chrome.browser.tab.Tab.TabHidingType;
 
@@ -47,17 +53,30 @@ public class StreamLifecycleManagerTest {
     private Tab mTab;
     @Mock
     private Stream mStream;
+    @Mock
+    private PrefServiceBridge mPrefServiceBridge;
 
     private StreamLifecycleManager mStreamLifecycleManager;
 
     @Before
     public void setUp() throws Exception {
         MockitoAnnotations.initMocks(this);
+
+        // Initialize a test instance for PrefServiceBridge.
+        when(mPrefServiceBridge.getBoolean(anyInt())).thenReturn(true);
+        doNothing().when(mPrefServiceBridge).setBoolean(anyInt(), anyBoolean());
+        PrefServiceBridge.setInstanceForTesting(mPrefServiceBridge);
+
         ApplicationStatus.onStateChangeForTesting(mActivity, ActivityState.CREATED);
         mStreamLifecycleManager = new StreamLifecycleManager(mStream, mActivity, mTab);
         verify(mStream, times(1)).onCreate(or(any(String.class), isNull()));
     }
 
+    @After
+    public void tearDown() {
+        PrefServiceBridge.setInstanceForTesting(null);
+    }
+
     @Test
     @SmallTest
     public void testShow() {
@@ -82,6 +101,33 @@ public class StreamLifecycleManagerTest {
         verify(mStream, times(1)).onShow();
     }
 
+    @Test
+    @SmallTest
+    public void testShow_ArticlesNotVisible() {
+        // Verify that onShow is not called when articles are set hidden by the user.
+        when(mPrefServiceBridge.getBoolean(Pref.NTP_ARTICLES_LIST_VISIBLE)).thenReturn(false);
+        ApplicationStatus.onStateChangeForTesting(mActivity, ActivityState.STARTED);
+        when(mTab.isHidden()).thenReturn(false);
+        when(mTab.isUserInteractable()).thenReturn(true);
+        mStreamLifecycleManager.getTabObserverForTesting().onShown(mTab, FROM_NEW);
+        verify(mStream, times(0)).onShow();
+
+        // Verify that onShow is called when articles are set shown by the user.
+        when(mPrefServiceBridge.getBoolean(Pref.NTP_ARTICLES_LIST_VISIBLE)).thenReturn(true);
+        mStreamLifecycleManager.getTabObserverForTesting().onShown(mTab, FROM_NEW);
+        verify(mStream, times(1)).onShow();
+
+        // Verify that onHide is called after tab is hidden.
+        mStreamLifecycleManager.getTabObserverForTesting().onHidden(mTab, CHANGED_TABS);
+        verify(mStream, times(1)).onHide();
+
+        // Verify that onShow is called when articles are set hidden by the user within the same
+        // session.
+        when(mPrefServiceBridge.getBoolean(Pref.NTP_ARTICLES_LIST_VISIBLE)).thenReturn(false);
+        mStreamLifecycleManager.getTabObserverForTesting().onShown(mTab, FROM_NEW);
+        verify(mStream, times(2)).onShow();
+    }
+
     @Test
     @SmallTest
     public void testActivate() {