[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/567182Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Commit-Queue: Nicolas Dossou-Gbété <dgn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486034}

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) {
@@ -62,7 +67,7 @@ public class SnippetsBridge implements SuggestionsSource {
         assert mNativeSnippetsBridge != 0;
         nativeDestroy(mNativeSnippetsBridge);
         mNativeSnippetsBridge = 0;
-        mObserver = null;
+        mObserverList.clear();
     }
 
     /**
@@ -83,8 +88,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() {
@@ -174,9 +180,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
@@ -229,22 +240,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);
@@ -253,7 +268,7 @@ public class SnippetsBridge implements SuggestionsSource {
     private static native void nativeRemoteSuggestionsSchedulerOnFetchDue();
     private static native void nativeRemoteSuggestionsSchedulerRescheduleFetching();
     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

@@ -42,6 +42,11 @@ public interface SuggestionsSource extends DestructionObserver {
      */
     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.
@@ -120,5 +125,26 @@ public interface SuggestionsSource extends DestructionObserver {
     /**
      * 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.onDestroy();
+
         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

@@ -163,18 +163,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,
@@ -288,6 +276,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