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

This reverts commit 139a79ec.

Reason for revert: This is instacrashing one of my test devices when I
try to load a webpage. See crbug.com/742056

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}

TBR=bauerb@chromium.org,dgn@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 738872
Change-Id: I9c3c7724215e59f6988c84e95fc7422c40872d72
Reviewed-on: https://chromium-review.googlesource.com/570806Reviewed-by: Ryan Landay <rlanday@chromium.org>
Commit-Queue: Ryan Landay <rlanday@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486597}

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

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

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

@@ -40,10 +40,6 @@ 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,10 +37,6 @@ 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,14 +15,9 @@ 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;
@@ -108,9 +103,6 @@ public class NewTabPageAdapter extends Adapter<NewTabPageViewHolder> implements
             mRoot.addChild(mBottomSpacer);
         }
 
-        RemoteSuggestionsStatusObserver suggestionsObserver = new RemoteSuggestionsStatusObserver();
-        mUiDelegate.addDestructionObserver(suggestionsObserver);
-
         updateAllDismissedVisibility();
         mRoot.setParent(this);
     }
@@ -230,8 +222,7 @@ public class NewTabPageAdapter extends Adapter<NewTabPageViewHolder> implements
     }
 
     private void updateAllDismissedVisibility() {
-        boolean showAllDismissed = hasAllBeenDismissed()
-                && mUiDelegate.getSuggestionsSource().areRemoteSuggestionsEnabled();
+        boolean showAllDismissed = hasAllBeenDismissed();
         mAllDismissed.setVisible(showAllDismissed);
         mFooter.setVisible(!showAllDismissed);
     }
@@ -318,24 +309,4 @@ 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
-    protected void setVisibilityInternal(boolean visible) {
+    public void setVisible(boolean visible) {
         if (mVisible == visible) return;
         mVisible = visible;
 

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

@@ -28,8 +28,4 @@ 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().addObserver(this);
+        mUiDelegate.getSuggestionsSource().setObserver(this);
         mOfflinePageBridge = offlinePageBridge;
 
         mUiDelegate.addDestructionObserver(new DestructionObserver() {

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

@@ -15,10 +15,6 @@ 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;
@@ -43,37 +39,23 @@ 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 mSigninObserver;
+    private final SigninObserver mObserver;
 
     public SignInPromo(SuggestionsUiDelegate uiDelegate) {
         mDismissed = ChromePreferenceManager.getInstance().getNewTabPageSigninPromoDismissed();
 
-        SuggestionsSource suggestionsSource = uiDelegate.getSuggestionsSource();
         SigninManager signinManager = SigninManager.get(ContextUtils.getApplicationContext());
         if (mDismissed) {
-            mSigninObserver = null;
+            mObserver = null;
         } else {
-            mSigninObserver = new SigninObserver(signinManager, suggestionsSource);
-            uiDelegate.addDestructionObserver(mSigninObserver);
+            mObserver = new SigninObserver(signinManager);
+            uiDelegate.addDestructionObserver(mObserver);
         }
 
-        mCanSignIn = signinManager.isSignInAllowed() && !signinManager.isSignedInOnNative();
-        mCanShowPersonalizedSuggestions = suggestionsSource.areRemoteSuggestionsEnabled();
-        updateVisibility();
+        setVisible(signinManager.isSignInAllowed() && !signinManager.isSignedInOnNative());
     }
 
     @Override
@@ -88,7 +70,7 @@ public class SignInPromo extends OptionalLeaf
      */
     @Nullable
     public DestructionObserver getObserver() {
-        return mSigninObserver;
+        return mObserver;
     }
 
     @Override
@@ -132,8 +114,9 @@ public class SignInPromo extends OptionalLeaf
         mImpressionTracker.reset(null);
     }
 
-    private void updateVisibility() {
-        setVisibilityInternal(!mDismissed && mCanSignIn && mCanShowPersonalizedSuggestions);
+    @Override
+    public void setVisible(boolean visible) {
+        super.setVisible(!mDismissed && visible);
     }
 
     @Override
@@ -144,31 +127,26 @@ 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;
-        updateVisibility();
+        setVisible(false);
 
         ChromePreferenceManager.getInstance().setNewTabPageSigninPromoDismissed(true);
-        mSigninObserver.unregister();
+        mObserver.unregister();
         itemRemovedCallback.onResult(ContextUtils.getApplicationContext().getString(getHeader()));
     }
 
     @VisibleForTesting
-    class SigninObserver extends SuggestionsSource.EmptyObserver
+    class SigninObserver
             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, SuggestionsSource suggestionsSource) {
+        private SigninObserver(SigninManager signinManager) {
             mSigninManager = signinManager;
             mSigninManager.addSignInAllowedObserver(this);
             mSigninManager.addSignInStateObserver(this);
-
-            mSuggestionsSource = suggestionsSource;
-            mSuggestionsSource.addObserver(this);
         }
 
         private void unregister() {
@@ -177,8 +155,6 @@ public class SignInPromo extends OptionalLeaf
 
             mSigninManager.removeSignInAllowedObserver(this);
             mSigninManager.removeSignInStateObserver(this);
-
-            mSuggestionsSource.removeObserver(this);
         }
 
         @Override
@@ -191,31 +167,17 @@ 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.
-            mCanSignIn = mSigninManager.isSignInAllowed();
-            updateVisibility();
+            setVisible(mSigninManager.isSignInAllowed());
         }
 
         @Override
         public void onSignedIn() {
-            mCanSignIn = false;
-            updateVisibility();
+            setVisible(false);
         }
 
         @Override
         public void onSignedOut() {
-            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();
+            setVisible(true);
         }
     }
 

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

@@ -63,8 +63,4 @@ 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;
-        setVisibilityInternal(true);
+        setVisible(true);
     }
 
     @Override
@@ -41,8 +41,4 @@ 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,7 +7,6 @@ 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;
@@ -23,17 +22,13 @@ public class SnippetsBridge implements SuggestionsSource {
     private static final String TAG = "SnippetsBridge";
 
     private long mNativeSnippetsBridge;
-    private final ObserverList<Observer> mObserverList = new ObserverList<>();
+    private SuggestionsSource.Observer mObserver;
 
     public static boolean isCategoryStatusAvailable(@CategoryStatus int status) {
-        // Note: This code is duplicated in category_status.cc.
+        // Note: This code is duplicated in content_suggestions_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) {
@@ -67,7 +62,7 @@ public class SnippetsBridge implements SuggestionsSource {
         assert mNativeSnippetsBridge != 0;
         nativeDestroy(mNativeSnippetsBridge);
         mNativeSnippetsBridge = 0;
-        mObserverList.clear();
+        mObserver = null;
     }
 
     /**
@@ -88,9 +83,8 @@ public class SnippetsBridge implements SuggestionsSource {
         nativeSetRemoteSuggestionsEnabled(enabled);
     }
 
-    @Override
-    public boolean areRemoteSuggestionsEnabled() {
-        return nativeAreRemoteSuggestionsEnabled(mNativeSnippetsBridge);
+    public static boolean areRemoteSuggestionsEnabled() {
+        return nativeAreRemoteSuggestionsEnabled();
     }
 
     public static boolean areRemoteSuggestionsManaged() {
@@ -180,14 +174,9 @@ public class SnippetsBridge implements SuggestionsSource {
     }
 
     @Override
-    public void addObserver(Observer observer) {
+    public void setObserver(Observer observer) {
         assert observer != null;
-        mObserverList.addObserver(observer);
-    }
-
-    @Override
-    public void removeObserver(Observer observer) {
-        mObserverList.removeObserver(observer);
+        mObserver = observer;
     }
 
     @Override
@@ -240,26 +229,22 @@ public class SnippetsBridge implements SuggestionsSource {
 
     @CalledByNative
     private void onNewSuggestions(@CategoryInt int category) {
-        for (Observer observer : mObserverList) observer.onNewSuggestions(category);
+        if (mObserver != null) mObserver.onNewSuggestions(category);
     }
 
     @CalledByNative
     private void onCategoryStatusChanged(@CategoryInt int category, @CategoryStatus int newStatus) {
-        for (Observer observer : mObserverList) {
-            observer.onCategoryStatusChanged(category, newStatus);
-        }
+        if (mObserver != null) mObserver.onCategoryStatusChanged(category, newStatus);
     }
 
     @CalledByNative
     private void onSuggestionInvalidated(@CategoryInt int category, String idWithinCategory) {
-        for (Observer observer : mObserverList) {
-            observer.onSuggestionInvalidated(category, idWithinCategory);
-        }
+        if (mObserver != null) mObserver.onSuggestionInvalidated(category, idWithinCategory);
     }
 
     @CalledByNative
     private void onFullRefreshRequired() {
-        for (Observer observer : mObserverList) observer.onFullRefreshRequired();
+        if (mObserver != null) mObserver.onFullRefreshRequired();
     }
 
     private native long nativeInit(Profile profile);
@@ -268,7 +253,7 @@ public class SnippetsBridge implements SuggestionsSource {
     private static native void nativeRemoteSuggestionsSchedulerOnPersistentSchedulerWakeUp();
     private static native void nativeRemoteSuggestionsSchedulerOnBrowserUpgraded();
     private static native void nativeSetRemoteSuggestionsEnabled(boolean enabled);
-    private native boolean nativeAreRemoteSuggestionsEnabled(long nativeNTPSnippetsBridge);
+    private static native boolean nativeAreRemoteSuggestionsEnabled();
     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,11 +42,6 @@ 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.
@@ -125,26 +120,5 @@ public interface SuggestionsSource extends DestructionObserver {
     /**
      * Sets the recipient for update events from the source.
      */
-    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() {}
-    }
+    void setObserver(Observer observer);
 }

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

@@ -23,9 +23,7 @@ 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;
@@ -76,12 +74,7 @@ public class ContentSuggestionsPreferences extends PreferenceFragment {
         setHasOptionsMenu(true);
         finishSwitchInitialisation();
 
-        SuggestionsSource suggestionsSource =
-                SuggestionsDependencyFactory.getInstance().createSuggestionSource(
-                        Profile.getLastUsedProfile());
-        boolean isEnabled = suggestionsSource.areRemoteSuggestionsEnabled();
-        suggestionsSource.onDestroy();
-
+        boolean isEnabled = SnippetsBridge.areRemoteSuggestionsEnabled();
         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() {
-        setVisibilityInternal(mTileGroup.getTiles().length != 0);
+        setVisible(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,6 +11,7 @@ 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;
@@ -62,7 +63,6 @@ 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,7 +80,6 @@ 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;
@@ -255,7 +254,10 @@ public class NewTabPageAdapterTest {
         mSource.setInfoForCategory(
                 TEST_CATEGORY, new CategoryInfoBuilder(TEST_CATEGORY).showIfEmpty().build());
 
-        resetUiDelegate();
+        when(mUiDelegate.getSuggestionsSource()).thenReturn(mSource);
+        when(mUiDelegate.getEventReporter()).thenReturn(mock(SuggestionsEventReporter.class));
+        when(mUiDelegate.getSuggestionsRanker()).thenReturn(mock(SuggestionsRanker.class));
+
         reloadNtp();
     }
 
@@ -890,26 +892,28 @@ 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);
-        mSource.setRemoteSuggestionsEnabled(true);
-        resetUiDelegate();
-        reloadNtp();
+        ArgumentCaptor<DestructionObserver> observers =
+                ArgumentCaptor.forClass(DestructionObserver.class);
 
+        doNothing().when(mUiDelegate).addDestructionObserver(observers.capture());
+
+        reloadNtp();
         assertTrue(isSignInPromoVisible());
 
-        // Note: As currently implemented, these variables should point to the same object, a
+        // Note: As currently implemented, these two variables should point to the same object, a
         // SignInPromo.SigninObserver
-        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 signInStateObserver = null;
+        SignInAllowedObserver signInAllowedObserver = null;
+        for (DestructionObserver observer : observers.getAllValues()) {
+            if (observer instanceof SignInStateObserver) {
+                signInStateObserver = (SignInStateObserver) observer;
+            }
+            if (observer instanceof SignInAllowedObserver) {
+                signInAllowedObserver = (SignInAllowedObserver) observer;
+            }
+        }
 
         signInStateObserver.onSignedIn();
         assertFalse(isSignInPromoVisible());
@@ -924,15 +928,6 @@ 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
@@ -965,8 +960,17 @@ public class NewTabPageAdapterTest {
     @Test
     @Feature({"Ntp"})
     public void testAllDismissedVisibility() {
-        SigninObserver signinObserver =
-                findFirstInstanceOf(getDestructionObserver(mUiDelegate), SigninObserver.class);
+        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;
+            }
+        }
 
         @SuppressWarnings("unchecked")
         Callback<String> itemDismissedCallback = mock(Callback.class);
@@ -1041,28 +1045,8 @@ 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());
@@ -1153,13 +1137,6 @@ 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 =
@@ -1174,25 +1151,4 @@ 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,6 +160,18 @@ 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,
@@ -273,12 +285,6 @@ 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,10 +53,6 @@ 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,7 +9,6 @@ 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;
@@ -29,7 +28,7 @@ import java.util.Map;
  * A fake Suggestions source for use in unit and instrumentation tests.
  */
 public class FakeSuggestionsSource implements SuggestionsSource {
-    private ObserverList<Observer> mObserverList = new ObserverList<>();
+    private SuggestionsSource.Observer mObserver;
     private final List<Integer> mCategories = new ArrayList<>();
     private final Map<Integer, List<SnippetArticle>> mSuggestions = new HashMap<>();
     private final Map<Integer, Integer> mCategoryStatus = new LinkedHashMap<>();
@@ -46,9 +45,6 @@ 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.
      */
@@ -59,7 +55,7 @@ public class FakeSuggestionsSource implements SuggestionsSource {
         } else if (!mCategories.contains(category)) {
             mCategories.add(category);
         }
-        for (Observer observer : mObserverList) observer.onCategoryStatusChanged(category, status);
+        if (mObserver != null) mObserver.onCategoryStatusChanged(category, status);
     }
 
     /**
@@ -69,7 +65,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));
-        for (Observer observer : mObserverList) observer.onNewSuggestions(category);
+        if (mObserver != null) mObserver.onNewSuggestions(category);
     }
 
     /**
@@ -140,16 +136,14 @@ public class FakeSuggestionsSource implements SuggestionsSource {
                 break;
             }
         }
-        for (Observer observer : mObserverList) {
-            observer.onSuggestionInvalidated(category, idWithinCategory);
-        }
+        mObserver.onSuggestionInvalidated(category, idWithinCategory);
     }
 
     /**
      * Notifies the observer that a full refresh is required.
      */
     public void fireFullRefreshRequired() {
-        for (Observer observer : mObserverList) observer.onFullRefreshRequired();
+        mObserver.onFullRefreshRequired();
     }
 
     /**
@@ -165,15 +159,6 @@ 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()) {
@@ -244,13 +229,8 @@ public class FakeSuggestionsSource implements SuggestionsSource {
     }
 
     @Override
-    public void addObserver(Observer observer) {
-        mObserverList.addObserver(observer);
-    }
-
-    @Override
-    public void removeObserver(Observer observer) {
-        mObserverList.removeObserver(observer);
+    public void setObserver(Observer observer) {
+        mObserver = observer;
     }
 
     @Override