Reland "[Suggestions] Remove CTAs when feature is disabled"

This is a reland of 139a79ec
Original change's description:
> [Suggestions] Remove CTAs when feature is disabled
>
> Stops showing the Sign in Promo and the All Dismissed item when remote
> suggestions are disabled and performing the advertised actions would not
> provide the user with new suggestions.
>
> Part of this CL involves making the SuggestionsSources notifies a list
> of observer rather than a single one.
>
> Bug: 738872
> Change-Id: I16c300fb14a1d5780578387d48d848570339befe
> Reviewed-on: https://chromium-review.googlesource.com/567182
> Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
> Commit-Queue: Nicolas Dossou-Gbété <dgn@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#486034}

Bug: 738872
Change-Id: Ifc5f6643b4402657b9a62e08596d9e3267ce79a8
Reviewed-on: https://chromium-review.googlesource.com/571722Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Commit-Queue: Nicolas Dossou-Gbété <dgn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486723}

chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/ActionItem.java

@@ -32,7 +32,8 @@ public class ActionItem extends OptionalLeaf {
         mCategoryInfo = section.getCategoryInfo();
         mParentSection = section;
         mSuggestionsRanker = ranker;
-        setVisible(mCategoryInfo.getAdditionalAction() != ContentSuggestionsAdditionalAction.NONE);
+        setVisibilityInternal(
+                mCategoryInfo.getAdditionalAction() != ContentSuggestionsAdditionalAction.NONE);
     }
 
     @Override

chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/AllDismissedItem.java

@@ -40,6 +40,10 @@ public class AllDismissedItem extends OptionalLeaf {
         visitor.visitAllDismissedItem();
     }
 
+    public void setVisible(boolean visible) {
+        setVisibilityInternal(visible);
+    }
+
     /**
      * ViewHolder for an item of type {@link ItemViewType#ALL_DISMISSED}.
      */

chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/Footer.java

@@ -37,6 +37,10 @@ public class Footer extends OptionalLeaf {
         visitor.visitFooter();
     }
 
+    public void setVisible(boolean visible) {
+        setVisibilityInternal(visible);
+    }
+
     /**
      * The {@code ViewHolder} for the {@link Footer}.
      */

chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapter.java

@@ -15,9 +15,14 @@ import org.chromium.base.VisibleForTesting;
 import org.chromium.chrome.browser.ChromeFeatureList;
 import org.chromium.chrome.browser.ntp.ContextMenuManager;
 import org.chromium.chrome.browser.ntp.cards.NewTabPageViewHolder.PartialBindCallback;
+import org.chromium.chrome.browser.ntp.snippets.CategoryInt;
+import org.chromium.chrome.browser.ntp.snippets.CategoryStatus;
 import org.chromium.chrome.browser.ntp.snippets.SectionHeaderViewHolder;
 import org.chromium.chrome.browser.ntp.snippets.SnippetArticleViewHolder;
+import org.chromium.chrome.browser.ntp.snippets.SnippetsBridge;
+import org.chromium.chrome.browser.ntp.snippets.SuggestionsSource;
 import org.chromium.chrome.browser.offlinepages.OfflinePageBridge;
+import org.chromium.chrome.browser.suggestions.DestructionObserver;
 import org.chromium.chrome.browser.suggestions.SuggestionsRecyclerView;
 import org.chromium.chrome.browser.suggestions.SuggestionsUiDelegate;
 import org.chromium.chrome.browser.suggestions.TileGrid;
@@ -103,6 +108,9 @@ public class NewTabPageAdapter extends Adapter<NewTabPageViewHolder> implements
             mRoot.addChild(mBottomSpacer);
         }
 
+        RemoteSuggestionsStatusObserver suggestionsObserver = new RemoteSuggestionsStatusObserver();
+        mUiDelegate.addDestructionObserver(suggestionsObserver);
+
         updateAllDismissedVisibility();
         mRoot.setParent(this);
     }
@@ -222,7 +230,8 @@ public class NewTabPageAdapter extends Adapter<NewTabPageViewHolder> implements
     }
 
     private void updateAllDismissedVisibility() {
-        boolean showAllDismissed = hasAllBeenDismissed();
+        boolean showAllDismissed = hasAllBeenDismissed()
+                && mUiDelegate.getSuggestionsSource().areRemoteSuggestionsEnabled();
         mAllDismissed.setVisible(showAllDismissed);
         mFooter.setVisible(!showAllDismissed);
     }
@@ -309,4 +318,24 @@ public class NewTabPageAdapter extends Adapter<NewTabPageViewHolder> implements
     InnerNode getRootForTesting() {
         return mRoot;
     }
+
+    private class RemoteSuggestionsStatusObserver
+            extends SuggestionsSource.EmptyObserver implements DestructionObserver {
+        public RemoteSuggestionsStatusObserver() {
+            mUiDelegate.getSuggestionsSource().addObserver(this);
+        }
+
+        @Override
+        public void onCategoryStatusChanged(
+                @CategoryInt int category, @CategoryStatus int newStatus) {
+            if (!SnippetsBridge.isCategoryRemote(category)) return;
+
+            updateAllDismissedVisibility();
+        }
+
+        @Override
+        public void onDestroy() {
+            mUiDelegate.getSuggestionsSource().removeObserver(this);
+        }
+    }
 }

chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/OptionalLeaf.java

@@ -67,7 +67,7 @@ public abstract class OptionalLeaf extends ChildNode {
      * initially considered hidden.
      */
     @CallSuper
-    public void setVisible(boolean visible) {
+    protected void setVisibilityInternal(boolean visible) {
         if (mVisible == visible) return;
         mVisible = visible;
 

chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/ProgressItem.java

@@ -28,4 +28,8 @@ class ProgressItem extends OptionalLeaf {
     protected void visitOptionalItem(NodeVisitor visitor) {
         visitor.visitProgressItem();
     }
+
+    public void setVisible(boolean visible) {
+        setVisibilityInternal(visible);
+    }
 }

chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SectionList.java

@@ -43,7 +43,7 @@ public class SectionList
 
     public SectionList(SuggestionsUiDelegate uiDelegate, OfflinePageBridge offlinePageBridge) {
         mUiDelegate = uiDelegate;
-        mUiDelegate.getSuggestionsSource().setObserver(this);
+        mUiDelegate.getSuggestionsSource().addObserver(this);
         mOfflinePageBridge = offlinePageBridge;
 
         mUiDelegate.addDestructionObserver(new DestructionObserver() {

chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SignInPromo.java

@@ -15,6 +15,10 @@ import org.chromium.base.VisibleForTesting;
 import org.chromium.base.metrics.RecordUserAction;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.ntp.ContextMenuManager;
+import org.chromium.chrome.browser.ntp.snippets.CategoryInt;
+import org.chromium.chrome.browser.ntp.snippets.CategoryStatus;
+import org.chromium.chrome.browser.ntp.snippets.SnippetsBridge;
+import org.chromium.chrome.browser.ntp.snippets.SuggestionsSource;
 import org.chromium.chrome.browser.preferences.ChromePreferenceManager;
 import org.chromium.chrome.browser.signin.AccountSigninActivity;
 import org.chromium.chrome.browser.signin.SigninAccessPoint;
@@ -39,23 +43,37 @@ public class SignInPromo extends OptionalLeaf
      */
     private boolean mDismissed;
 
+    /**
+     * Whether the signin status means that the user has the possibility to sign in.
+     */
+    private boolean mCanSignIn;
+
+    /**
+     * Whether personalized suggestions can be shown. If it's not the case, we have no reason to
+     * offer the user to sign in.
+     */
+    private boolean mCanShowPersonalizedSuggestions;
+
     private final ImpressionTracker mImpressionTracker = new ImpressionTracker(null, this);
 
     @Nullable
-    private final SigninObserver mObserver;
+    private final SigninObserver mSigninObserver;
 
     public SignInPromo(SuggestionsUiDelegate uiDelegate) {
         mDismissed = ChromePreferenceManager.getInstance().getNewTabPageSigninPromoDismissed();
 
+        SuggestionsSource suggestionsSource = uiDelegate.getSuggestionsSource();
         SigninManager signinManager = SigninManager.get(ContextUtils.getApplicationContext());
         if (mDismissed) {
-            mObserver = null;
+            mSigninObserver = null;
         } else {
-            mObserver = new SigninObserver(signinManager);
-            uiDelegate.addDestructionObserver(mObserver);
+            mSigninObserver = new SigninObserver(signinManager, suggestionsSource);
+            uiDelegate.addDestructionObserver(mSigninObserver);
         }
 
-        setVisible(signinManager.isSignInAllowed() && !signinManager.isSignedInOnNative());
+        mCanSignIn = signinManager.isSignInAllowed() && !signinManager.isSignedInOnNative();
+        mCanShowPersonalizedSuggestions = suggestionsSource.areRemoteSuggestionsEnabled();
+        updateVisibility();
     }
 
     @Override
@@ -70,7 +88,7 @@ public class SignInPromo extends OptionalLeaf
      */
     @Nullable
     public DestructionObserver getObserver() {
-        return mObserver;
+        return mSigninObserver;
     }
 
     @Override
@@ -114,9 +132,8 @@ public class SignInPromo extends OptionalLeaf
         mImpressionTracker.reset(null);
     }
 
-    @Override
-    public void setVisible(boolean visible) {
-        super.setVisible(!mDismissed && visible);
+    private void updateVisibility() {
+        setVisibilityInternal(!mDismissed && mCanSignIn && mCanShowPersonalizedSuggestions);
     }
 
     @Override
@@ -127,26 +144,31 @@ public class SignInPromo extends OptionalLeaf
     /** Hides the sign in promo and sets a preference to make sure it is not shown again. */
     @Override
     public void dismiss(Callback<String> itemRemovedCallback) {
+        assert mSigninObserver != null;
         mDismissed = true;
-        setVisible(false);
+        updateVisibility();
 
         ChromePreferenceManager.getInstance().setNewTabPageSigninPromoDismissed(true);
-        mObserver.unregister();
+        mSigninObserver.unregister();
         itemRemovedCallback.onResult(ContextUtils.getApplicationContext().getString(getHeader()));
     }
 
     @VisibleForTesting
-    class SigninObserver
+    class SigninObserver extends SuggestionsSource.EmptyObserver
             implements SignInStateObserver, SignInAllowedObserver, DestructionObserver {
         private final SigninManager mSigninManager;
+        private final SuggestionsSource mSuggestionsSource;
 
         /** Guards {@link #unregister()}, which can be called multiple times. */
         private boolean mUnregistered;
 
-        private SigninObserver(SigninManager signinManager) {
+        private SigninObserver(SigninManager signinManager, SuggestionsSource suggestionsSource) {
             mSigninManager = signinManager;
             mSigninManager.addSignInAllowedObserver(this);
             mSigninManager.addSignInStateObserver(this);
+
+            mSuggestionsSource = suggestionsSource;
+            mSuggestionsSource.addObserver(this);
         }
 
         private void unregister() {
@@ -155,6 +177,8 @@ public class SignInPromo extends OptionalLeaf
 
             mSigninManager.removeSignInAllowedObserver(this);
             mSigninManager.removeSignInStateObserver(this);
+
+            mSuggestionsSource.removeObserver(this);
         }
 
         @Override
@@ -167,17 +191,31 @@ public class SignInPromo extends OptionalLeaf
             // Listening to onSignInAllowedChanged is important for the FRE. Sign in is not allowed
             // until it is completed, but the NTP is initialised before the FRE is even shown. By
             // implementing this we can show the promo if the user did not sign in during the FRE.
-            setVisible(mSigninManager.isSignInAllowed());
+            mCanSignIn = mSigninManager.isSignInAllowed();
+            updateVisibility();
         }
 
         @Override
         public void onSignedIn() {
-            setVisible(false);
+            mCanSignIn = false;
+            updateVisibility();
         }
 
         @Override
         public void onSignedOut() {
-            setVisible(true);
+            mCanSignIn = mSigninManager.isSignInAllowed();
+            updateVisibility();
+        }
+
+        @Override
+        public void onCategoryStatusChanged(
+                @CategoryInt int category, @CategoryStatus int newStatus) {
+            if (!SnippetsBridge.isCategoryRemote(category)) return;
+
+            // Checks whether the category is enabled first to avoid unnecessary calls across JNI.
+            mCanShowPersonalizedSuggestions = SnippetsBridge.isCategoryEnabled(category)
+                    || mSuggestionsSource.areRemoteSuggestionsEnabled();
+            updateVisibility();
         }
     }
 

chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/StatusItem.java

@@ -63,4 +63,8 @@ public abstract class StatusItem extends OptionalLeaf implements StatusCardViewH
         assert holder instanceof StatusCardViewHolder;
         ((StatusCardViewHolder) holder).onBindViewHolder(this);
     }
+
+    public void setVisible(boolean visible) {
+        setVisibilityInternal(visible);
+    }
 }

chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SectionHeader.java

@@ -18,7 +18,7 @@ public class SectionHeader extends OptionalLeaf {
 
     public SectionHeader(String headerText) {
         this.mHeaderText = headerText;
-        setVisible(true);
+        setVisibilityInternal(true);
     }
 
     @Override
@@ -41,4 +41,8 @@ public class SectionHeader extends OptionalLeaf {
     public void visitOptionalItem(NodeVisitor visitor) {
         visitor.visitHeader();
     }
+
+    public void setVisible(boolean visible) {
+        setVisibilityInternal(visible);
+    }
 }

chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetsBridge.java

@@ -7,6 +7,7 @@ package org.chromium.chrome.browser.ntp.snippets;
 import android.graphics.Bitmap;
 
 import org.chromium.base.Callback;
+import org.chromium.base.ObserverList;
 import org.chromium.base.annotations.CalledByNative;
 import org.chromium.chrome.browser.ntp.cards.SuggestionsCategoryInfo;
 import org.chromium.chrome.browser.profiles.Profile;
@@ -22,13 +23,17 @@ public class SnippetsBridge implements SuggestionsSource {
     private static final String TAG = "SnippetsBridge";
 
     private long mNativeSnippetsBridge;
-    private SuggestionsSource.Observer mObserver;
+    private final ObserverList<Observer> mObserverList = new ObserverList<>();
 
     public static boolean isCategoryStatusAvailable(@CategoryStatus int status) {
-        // Note: This code is duplicated in content_suggestions_category_status.cc.
+        // Note: This code is duplicated in category_status.cc.
         return status == CategoryStatus.AVAILABLE_LOADING || status == CategoryStatus.AVAILABLE;
     }
 
+    public static boolean isCategoryRemote(@CategoryInt int category) {
+        return category > KnownCategories.REMOTE_CATEGORIES_OFFSET;
+    }
+
     /** Returns whether the category is considered "enabled", and can show content suggestions. */
     public static boolean isCategoryEnabled(@CategoryStatus int status) {
         switch (status) {
@@ -58,7 +63,7 @@ public class SnippetsBridge implements SuggestionsSource {
         assert mNativeSnippetsBridge != 0;
         nativeDestroy(mNativeSnippetsBridge);
         mNativeSnippetsBridge = 0;
-        mObserver = null;
+        mObserverList.clear();
     }
 
     /**
@@ -79,8 +84,9 @@ public class SnippetsBridge implements SuggestionsSource {
         nativeSetRemoteSuggestionsEnabled(enabled);
     }
 
-    public static boolean areRemoteSuggestionsEnabled() {
-        return nativeAreRemoteSuggestionsEnabled();
+    @Override
+    public boolean areRemoteSuggestionsEnabled() {
+        return nativeAreRemoteSuggestionsEnabled(mNativeSnippetsBridge);
     }
 
     public static boolean areRemoteSuggestionsManaged() {
@@ -171,9 +177,14 @@ public class SnippetsBridge implements SuggestionsSource {
     }
 
     @Override
-    public void setObserver(Observer observer) {
+    public void addObserver(Observer observer) {
         assert observer != null;
-        mObserver = observer;
+        mObserverList.addObserver(observer);
+    }
+
+    @Override
+    public void removeObserver(Observer observer) {
+        mObserverList.removeObserver(observer);
     }
 
     @Override
@@ -227,22 +238,26 @@ public class SnippetsBridge implements SuggestionsSource {
 
     @CalledByNative
     private void onNewSuggestions(@CategoryInt int category) {
-        if (mObserver != null) mObserver.onNewSuggestions(category);
+        for (Observer observer : mObserverList) observer.onNewSuggestions(category);
     }
 
     @CalledByNative
     private void onCategoryStatusChanged(@CategoryInt int category, @CategoryStatus int newStatus) {
-        if (mObserver != null) mObserver.onCategoryStatusChanged(category, newStatus);
+        for (Observer observer : mObserverList) {
+            observer.onCategoryStatusChanged(category, newStatus);
+        }
     }
 
     @CalledByNative
     private void onSuggestionInvalidated(@CategoryInt int category, String idWithinCategory) {
-        if (mObserver != null) mObserver.onSuggestionInvalidated(category, idWithinCategory);
+        for (Observer observer : mObserverList) {
+            observer.onSuggestionInvalidated(category, idWithinCategory);
+        }
     }
 
     @CalledByNative
     private void onFullRefreshRequired() {
-        if (mObserver != null) mObserver.onFullRefreshRequired();
+        for (Observer observer : mObserverList) observer.onFullRefreshRequired();
     }
 
     private native long nativeInit(Profile profile);
@@ -251,7 +266,7 @@ public class SnippetsBridge implements SuggestionsSource {
     private static native void nativeRemoteSuggestionsSchedulerOnPersistentSchedulerWakeUp();
     private static native void nativeRemoteSuggestionsSchedulerOnBrowserUpgraded();
     private static native void nativeSetRemoteSuggestionsEnabled(boolean enabled);
-    private static native boolean nativeAreRemoteSuggestionsEnabled();
+    private native boolean nativeAreRemoteSuggestionsEnabled(long nativeNTPSnippetsBridge);
     private static native boolean nativeAreRemoteSuggestionsManaged();
     private static native boolean nativeAreRemoteSuggestionsManagedByCustodian();
     private static native void nativeSetContentSuggestionsNotificationsEnabled(boolean enabled);

chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SuggestionsSource.java

@@ -47,6 +47,11 @@ public interface SuggestionsSource {
      */
     void fetchRemoteSuggestions();
 
+    /**
+     * @return Whether remote suggestions are enabled.
+     */
+    boolean areRemoteSuggestionsEnabled();
+
     /**
      * Gets the categories in the order in which they should be displayed.
      * @return The categories.
@@ -125,5 +130,26 @@ public interface SuggestionsSource {
     /**
      * Sets the recipient for update events from the source.
      */
-    void setObserver(Observer observer);
+    void addObserver(Observer observer);
+
+    /**
+     * Removes an observer. Is no-op if the observer was not already registered.
+     */
+    void removeObserver(Observer observer);
+
+    /** No-op implementation of {@link SuggestionsSource.Observer}. */
+    class EmptyObserver implements Observer {
+        @Override
+        public void onNewSuggestions(@CategoryInt int category) {}
+
+        @Override
+        public void onCategoryStatusChanged(
+                @CategoryInt int category, @CategoryStatus int newStatus) {}
+
+        @Override
+        public void onSuggestionInvalidated(@CategoryInt int category, String idWithinCategory) {}
+
+        @Override
+        public void onFullRefreshRequired() {}
+    }
 }

chrome/android/java/src/org/chromium/chrome/browser/preferences/ContentSuggestionsPreferences.java

@@ -23,7 +23,9 @@ import org.chromium.chrome.browser.ntp.ContentSuggestionsNotificationHelper;
 import org.chromium.chrome.browser.ntp.snippets.ContentSuggestionsNotificationAction;
 import org.chromium.chrome.browser.ntp.snippets.ContentSuggestionsNotificationOptOut;
 import org.chromium.chrome.browser.ntp.snippets.SnippetsBridge;
+import org.chromium.chrome.browser.ntp.snippets.SuggestionsSource;
 import org.chromium.chrome.browser.profiles.Profile;
+import org.chromium.chrome.browser.suggestions.SuggestionsDependencyFactory;
 
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
@@ -74,7 +76,12 @@ public class ContentSuggestionsPreferences extends PreferenceFragment {
         setHasOptionsMenu(true);
         finishSwitchInitialisation();
 
-        boolean isEnabled = SnippetsBridge.areRemoteSuggestionsEnabled();
+        SuggestionsSource suggestionsSource =
+                SuggestionsDependencyFactory.getInstance().createSuggestionSource(
+                        Profile.getLastUsedProfile());
+        boolean isEnabled = suggestionsSource.areRemoteSuggestionsEnabled();
+        suggestionsSource.destroy();
+
         mIsEnabled = !isEnabled; // Opposite so that we trigger side effects below.
         updatePreferences(isEnabled);
 

chrome/android/java/src/org/chromium/chrome/browser/suggestions/TileGrid.java

@@ -66,7 +66,7 @@ public class TileGrid extends OptionalLeaf implements TileGroup.Observer {
 
     @Override
     public void onTileDataChanged() {
-        setVisible(mTileGroup.getTiles().length != 0);
+        setVisibilityInternal(mTileGroup.getTiles().length != 0);
         if (isVisible()) notifyItemChanged(0, new ViewHolder.UpdateTilesCallback(mTileGroup));
     }
 

chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapterTest.java

@@ -11,7 +11,6 @@ import static org.junit.Assert.assertTrue;
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.atLeastOnce;
-import static org.mockito.Mockito.doNothing;
 import static org.mockito.Mockito.inOrder;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.reset;
@@ -63,6 +62,7 @@ import org.chromium.chrome.browser.ntp.snippets.CategoryInt;
 import org.chromium.chrome.browser.ntp.snippets.CategoryStatus;
 import org.chromium.chrome.browser.ntp.snippets.KnownCategories;
 import org.chromium.chrome.browser.ntp.snippets.SnippetArticle;
+import org.chromium.chrome.browser.ntp.snippets.SuggestionsSource;
 import org.chromium.chrome.browser.offlinepages.OfflinePageBridge;
 import org.chromium.chrome.browser.preferences.ChromePreferenceManager;
 import org.chromium.chrome.browser.signin.SigninManager;
@@ -80,6 +80,7 @@ import org.chromium.chrome.test.util.browser.suggestions.FakeSuggestionsSource;
 import org.chromium.testing.local.LocalRobolectricTestRunner;
 
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
@@ -254,10 +255,7 @@ public class NewTabPageAdapterTest {
         mSource.setInfoForCategory(
                 TEST_CATEGORY, new CategoryInfoBuilder(TEST_CATEGORY).showIfEmpty().build());
 
-        when(mUiDelegate.getSuggestionsSource()).thenReturn(mSource);
-        when(mUiDelegate.getEventReporter()).thenReturn(mock(SuggestionsEventReporter.class));
-        when(mUiDelegate.getSuggestionsRanker()).thenReturn(mock(SuggestionsRanker.class));
-
+        resetUiDelegate();
         reloadNtp();
     }
 
@@ -892,28 +890,26 @@ public class NewTabPageAdapterTest {
     @Test
     @Feature({"Ntp"})
     public void testSigninPromo() {
+        @CategoryInt
+        final int remoteCategory = KnownCategories.REMOTE_CATEGORIES_OFFSET + TEST_CATEGORY;
+
         when(mMockSigninManager.isSignInAllowed()).thenReturn(true);
         when(mMockSigninManager.isSignedInOnNative()).thenReturn(false);
-        ArgumentCaptor<DestructionObserver> observers =
-                ArgumentCaptor.forClass(DestructionObserver.class);
-
-        doNothing().when(mUiDelegate).addDestructionObserver(observers.capture());
-
+        mSource.setRemoteSuggestionsEnabled(true);
+        resetUiDelegate();
         reloadNtp();
+
         assertTrue(isSignInPromoVisible());
 
-        // Note: As currently implemented, these two variables should point to the same object, a
+        // Note: As currently implemented, these variables should point to the same object, a
         // SignInPromo.SigninObserver
-        SignInStateObserver signInStateObserver = null;
-        SignInAllowedObserver signInAllowedObserver = null;
-        for (DestructionObserver observer : observers.getAllValues()) {
-            if (observer instanceof SignInStateObserver) {
-                signInStateObserver = (SignInStateObserver) observer;
-            }
-            if (observer instanceof SignInAllowedObserver) {
-                signInAllowedObserver = (SignInAllowedObserver) observer;
-            }
-        }
+        List<DestructionObserver> observers = getDestructionObserver(mUiDelegate);
+        SignInStateObserver signInStateObserver =
+                findFirstInstanceOf(observers, SignInStateObserver.class);
+        SignInAllowedObserver signInAllowedObserver =
+                findFirstInstanceOf(observers, SignInAllowedObserver.class);
+        SuggestionsSource.Observer suggestionsObserver =
+                findFirstInstanceOf(observers, SuggestionsSource.Observer.class);
 
         signInStateObserver.onSignedIn();
         assertFalse(isSignInPromoVisible());
@@ -928,6 +924,15 @@ public class NewTabPageAdapterTest {
         when(mMockSigninManager.isSignInAllowed()).thenReturn(true);
         signInAllowedObserver.onSignInAllowedChanged();
         assertTrue(isSignInPromoVisible());
+
+        mSource.setRemoteSuggestionsEnabled(false);
+        suggestionsObserver.onCategoryStatusChanged(
+                remoteCategory, CategoryStatus.CATEGORY_EXPLICITLY_DISABLED);
+        assertFalse(isSignInPromoVisible());
+
+        mSource.setRemoteSuggestionsEnabled(true);
+        suggestionsObserver.onCategoryStatusChanged(remoteCategory, CategoryStatus.AVAILABLE);
+        assertTrue(isSignInPromoVisible());
     }
 
     @Test
@@ -960,17 +965,8 @@ public class NewTabPageAdapterTest {
     @Test
     @Feature({"Ntp"})
     public void testAllDismissedVisibility() {
-        ArgumentCaptor<DestructionObserver> observers =
-                ArgumentCaptor.forClass(DestructionObserver.class);
-
-        verify(mUiDelegate, atLeastOnce()).addDestructionObserver(observers.capture());
-
-        SigninObserver signinObserver = null;
-        for (DestructionObserver observer : observers.getAllValues()) {
-            if (observer instanceof SigninObserver) {
-                signinObserver = (SigninObserver) observer;
-            }
-        }
+        SigninObserver signinObserver =
+                findFirstInstanceOf(getDestructionObserver(mUiDelegate), SigninObserver.class);
 
         @SuppressWarnings("unchecked")
         Callback<String> itemDismissedCallback = mock(Callback.class);
@@ -1045,8 +1041,28 @@ public class NewTabPageAdapterTest {
         assertEquals(RecyclerView.NO_POSITION,
                 mAdapter.getFirstPositionForType(ItemViewType.ALL_DISMISSED));
 
+        // Disabling remote suggestions should remove both the promo and the AllDismissed item
+        mSource.setRemoteSuggestionsEnabled(false);
+        signinObserver.onCategoryStatusChanged(
+                KnownCategories.REMOTE_CATEGORIES_OFFSET + TEST_CATEGORY,
+                CategoryStatus.CATEGORY_EXPLICITLY_DISABLED);
+        // Adapter content:
+        // Idx | Item
+        // ----|--------------------
+        // 0   | Above-the-fold
+        // 1   | Footer
+        // 2   | Spacer
+        assertEquals(ItemViewType.FOOTER, mAdapter.getItemViewType(1));
+        assertEquals(RecyclerView.NO_POSITION,
+                mAdapter.getFirstPositionForType(ItemViewType.ALL_DISMISSED));
+        assertEquals(
+                RecyclerView.NO_POSITION, mAdapter.getFirstPositionForType(ItemViewType.PROMO));
+
         // Prepare some suggestions. They should not load because the category is dismissed on
         // the current NTP.
+        mSource.setRemoteSuggestionsEnabled(true);
+        signinObserver.onCategoryStatusChanged(
+                KnownCategories.REMOTE_CATEGORIES_OFFSET + TEST_CATEGORY, CategoryStatus.AVAILABLE);
         mSource.setStatusForCategory(TEST_CATEGORY, CategoryStatus.AVAILABLE);
         mSource.setSuggestionsForCategory(TEST_CATEGORY, createDummySuggestions(1, TEST_CATEGORY));
         mSource.setInfoForCategory(TEST_CATEGORY, new CategoryInfoBuilder(TEST_CATEGORY).build());
@@ -1137,6 +1153,13 @@ public class NewTabPageAdapterTest {
         return new SectionDescriptor(Collections.<SnippetArticle>emptyList());
     }
 
+    private void resetUiDelegate() {
+        reset(mUiDelegate);
+        when(mUiDelegate.getSuggestionsSource()).thenReturn(mSource);
+        when(mUiDelegate.getEventReporter()).thenReturn(mock(SuggestionsEventReporter.class));
+        when(mUiDelegate.getSuggestionsRanker()).thenReturn(mock(SuggestionsRanker.class));
+    }
+
     private void reloadNtp() {
         mAdapter = new NewTabPageAdapter(mUiDelegate, mock(View.class), makeUiConfig(),
                 mOfflinePageBridge, mock(ContextMenuManager.class), /* tileGroupDelegate =
@@ -1151,4 +1174,25 @@ public class NewTabPageAdapterTest {
     private int getCategory(TreeNode item) {
         return ((SuggestionsSection) item).getCategory();
     }
+
+    /**
+     * Note: Currently the observers need to be re-registered to be returned again if this method
+     * has been called, as it relies on argument captors that don't repeatedly capture individual
+     * calls.
+     * @return The currently registered destruction observers.
+     */
+    private List<DestructionObserver> getDestructionObserver(SuggestionsUiDelegate delegate) {
+        ArgumentCaptor<DestructionObserver> observers =
+                ArgumentCaptor.forClass(DestructionObserver.class);
+        verify(delegate, atLeastOnce()).addDestructionObserver(observers.capture());
+        return observers.getAllValues();
+    }
+
+    @SuppressWarnings("unchecked")
+    private static <T> T findFirstInstanceOf(Collection<?> collection, Class<T> clazz) {
+        for (Object item : collection) {
+            if (clazz.isAssignableFrom(item.getClass())) return (T) item;
+        }
+        return null;
+    }
 }

chrome/browser/android/ntp/ntp_snippets_bridge.cc

@@ -160,18 +160,6 @@ static void SetRemoteSuggestionsEnabled(JNIEnv* env,
   content_suggestions_service->SetRemoteSuggestionsEnabled(enabled);
 }
 
-static jboolean AreRemoteSuggestionsEnabled(
-    JNIEnv* env,
-    const JavaParamRef<jclass>& caller) {
-  ntp_snippets::ContentSuggestionsService* content_suggestions_service =
-      ContentSuggestionsServiceFactory::GetForProfile(
-          ProfileManager::GetLastUsedProfile());
-  if (!content_suggestions_service)
-    return false;
-
-  return content_suggestions_service->AreRemoteSuggestionsEnabled();
-}
-
 // Returns true if the remote provider is managed by an adminstrator's policy.
 static jboolean AreRemoteSuggestionsManaged(
     JNIEnv* env,
@@ -285,6 +273,12 @@ ScopedJavaLocalRef<jobject> NTPSnippetsBridge::GetSuggestionsForCategory(
       content_suggestions_service_->GetSuggestionsForCategory(category));
 }
 
+jboolean NTPSnippetsBridge::AreRemoteSuggestionsEnabled(
+    JNIEnv* env,
+    const JavaParamRef<jobject>& obj) {
+  return content_suggestions_service_->AreRemoteSuggestionsEnabled();
+}
+
 void NTPSnippetsBridge::FetchSuggestionImage(
     JNIEnv* env,
     const JavaParamRef<jobject>& obj,

chrome/browser/android/ntp/ntp_snippets_bridge.h

@@ -53,6 +53,10 @@ class NTPSnippetsBridge
       const base::android::JavaParamRef<jobject>& obj,
       jint j_category_id);
 
+  jboolean AreRemoteSuggestionsEnabled(
+      JNIEnv* env,
+      const base::android::JavaParamRef<jobject>& obj);
+
   void FetchSuggestionImage(
       JNIEnv* env,
       const base::android::JavaParamRef<jobject>& obj,

chrome/test/android/javatests/src/org/chromium/chrome/test/util/browser/suggestions/FakeSuggestionsSource.java

@@ -9,6 +9,7 @@ import static org.chromium.chrome.test.util.browser.suggestions.ContentSuggestio
 import android.graphics.Bitmap;
 
 import org.chromium.base.Callback;
+import org.chromium.base.ObserverList;
 import org.chromium.base.ThreadUtils;
 import org.chromium.chrome.browser.ntp.cards.SuggestionsCategoryInfo;
 import org.chromium.chrome.browser.ntp.snippets.CategoryInt;
@@ -28,7 +29,7 @@ import java.util.Map;
  * A fake Suggestions source for use in unit and instrumentation tests.
  */
 public class FakeSuggestionsSource implements SuggestionsSource {
-    private SuggestionsSource.Observer mObserver;
+    private ObserverList<Observer> mObserverList = new ObserverList<>();
     private final List<Integer> mCategories = new ArrayList<>();
     private final Map<Integer, List<SnippetArticle>> mSuggestions = new HashMap<>();
     private final Map<Integer, Integer> mCategoryStatus = new LinkedHashMap<>();
@@ -45,6 +46,9 @@ public class FakeSuggestionsSource implements SuggestionsSource {
             new HashMap<>();
     private final Map<Integer, Integer> mDismissedCategoryStatus = new LinkedHashMap<>();
     private final Map<Integer, SuggestionsCategoryInfo> mDismissedCategoryInfo = new HashMap<>();
+
+    private boolean mRemoteSuggestionsEnabled = true;
+
     /**
      * Sets the status to be returned for a given category.
      */
@@ -55,7 +59,7 @@ public class FakeSuggestionsSource implements SuggestionsSource {
         } else if (!mCategories.contains(category)) {
             mCategories.add(category);
         }
-        if (mObserver != null) mObserver.onCategoryStatusChanged(category, status);
+        for (Observer observer : mObserverList) observer.onCategoryStatusChanged(category, status);
     }
 
     /**
@@ -65,7 +69,7 @@ public class FakeSuggestionsSource implements SuggestionsSource {
             @CategoryInt int category, List<SnippetArticle> suggestions) {
         // Copy the suggestions list so that it can't be modified anymore.
         mSuggestions.put(category, new ArrayList<>(suggestions));
-        if (mObserver != null) mObserver.onNewSuggestions(category);
+        for (Observer observer : mObserverList) observer.onNewSuggestions(category);
     }
 
     /**
@@ -136,14 +140,16 @@ public class FakeSuggestionsSource implements SuggestionsSource {
                 break;
             }
         }
-        mObserver.onSuggestionInvalidated(category, idWithinCategory);
+        for (Observer observer : mObserverList) {
+            observer.onSuggestionInvalidated(category, idWithinCategory);
+        }
     }
 
     /**
      * Notifies the observer that a full refresh is required.
      */
     public void fireFullRefreshRequired() {
-        mObserver.onFullRefreshRequired();
+        for (Observer observer : mObserverList) observer.onFullRefreshRequired();
     }
 
     /**
@@ -159,6 +165,15 @@ public class FakeSuggestionsSource implements SuggestionsSource {
     @Override
     public void fetchRemoteSuggestions() {}
 
+    @Override
+    public boolean areRemoteSuggestionsEnabled() {
+        return mRemoteSuggestionsEnabled;
+    }
+
+    public void setRemoteSuggestionsEnabled(boolean enabled) {
+        mRemoteSuggestionsEnabled = enabled;
+    }
+
     @Override
     public void dismissSuggestion(SnippetArticle suggestion) {
         for (List<SnippetArticle> suggestions : mSuggestions.values()) {
@@ -229,8 +244,13 @@ public class FakeSuggestionsSource implements SuggestionsSource {
     }
 
     @Override
-    public void setObserver(Observer observer) {
-        mObserver = observer;
+    public void addObserver(Observer observer) {
+        mObserverList.addObserver(observer);
+    }
+
+    @Override
+    public void removeObserver(Observer observer) {
+        mObserverList.removeObserver(observer);
     }
 
     @Override