[EoC] Add more restrictions for whether the feature is enabled

Add an EnabledStateMonitor to monitor changes to contextualsuggestions
enabled state. Currently the only monitored state is whether sync
upload to Google is active.

Also updates the FeatureUtilities check to account for the search engine
promo.

BUG=823987

Change-Id: I9b4ae44ae94e73ef5c7e43cbfe6aebf657f4ada0
Reviewed-on: https://chromium-review.googlesource.com/988343Reviewed-by: Pavel Yatsuk <pavely@chromium.org>
Reviewed-by: Filip Gorski <fgorski@chromium.org>
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Commit-Queue: Theresa <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548051}

chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java

@@ -1260,7 +1260,7 @@ public abstract class ChromeActivity extends AsyncInitializationActivity
         VrShellDelegate.onNativeLibraryAvailable();
         super.finishNativeInitialization();
 
-        if (FeatureUtilities.isContextualSuggestionsBottomSheetEnabled()) {
+        if (FeatureUtilities.isContextualSuggestionsBottomSheetEnabled(isTablet())) {
             ViewGroup coordinator = (ViewGroup) findViewById(R.id.coordinator);
             getLayoutInflater().inflate(R.layout.bottom_sheet, coordinator);
             mBottomSheet = coordinator.findViewById(R.id.bottom_sheet);
@@ -1273,13 +1273,10 @@ public abstract class ChromeActivity extends AsyncInitializationActivity
                     getCompositorViewHolder().getLayoutManager(), mFadingBackgroundView,
                     mBottomSheet);
 
-            if (ChromeFeatureList.isEnabled(
-                        ChromeFeatureList.CONTEXTUAL_SUGGESTIONS_BOTTOM_SHEET)) {
             mContextualSuggestionsCoordinator = new ContextualSuggestionsCoordinator(
                     this, mBottomSheetController, getTabModelSelector());
         }
     }
-    }
 
     /**
      * @return Whether native initialization has been completed for this activity.

chrome/android/java/src/org/chromium/chrome/browser/contextual_suggestions/ContextualSuggestionsCoordinator.java

@@ -27,10 +27,10 @@ import org.chromium.chrome.browser.widget.bottomsheet.BottomSheetController;
 public class ContextualSuggestionsCoordinator {
     private final ChromeActivity mActivity;
     private final BottomSheetController mBottomSheetController;
+    private final TabModelSelector mTabModelSelector;
     private final Profile mProfile;
     private final ContextualSuggestionsModel mModel;
     private final ContextualSuggestionsMediator mMediator;
-    private final SuggestionsUiDelegateImpl mUiDelegate;
 
     private @Nullable ToolbarCoordinator mToolbarCoordinator;
     private @Nullable ContentCoordinator mContentCoordinator;
@@ -46,19 +46,12 @@ public class ContextualSuggestionsCoordinator {
             BottomSheetController bottomSheetController, TabModelSelector tabModelSelector) {
         mActivity = activity;
         mBottomSheetController = bottomSheetController;
+        mTabModelSelector = tabModelSelector;
         mProfile = Profile.getLastUsedProfile().getOriginalProfile();
 
         mModel = new ContextualSuggestionsModel();
         mMediator = new ContextualSuggestionsMediator(mActivity, mProfile, tabModelSelector,
                 activity.getFullscreenManager(), this, mModel);
-
-        SuggestionsSource suggestionsSource = mMediator.getSuggestionsSource();
-        SuggestionsNavigationDelegate navigationDelegate = new SuggestionsNavigationDelegateImpl(
-                mActivity, mProfile, mBottomSheetController.getBottomSheet(), tabModelSelector);
-        mUiDelegate = new SuggestionsUiDelegateImpl(suggestionsSource, new DummyEventReporter(),
-                navigationDelegate, mProfile, mBottomSheetController.getBottomSheet(),
-                mActivity.getChromeApplication().getReferencePool(),
-                mActivity.getSnackbarManager());
     }
 
     /** Called when the containing activity is destroyed. */
@@ -73,15 +66,26 @@ public class ContextualSuggestionsCoordinator {
     /**
      * Preload the contextual suggestions in the {@link BottomSheet}; content won't actually be
      * shown until {@link #showSuggestions()} is called.
+     *
+     * @param suggestionsSource The {@link SuggestionsSource} used to retrieve additional things
+     *                          needed to display suggestions (e.g. favicons, thumbnails).
      */
-    void preloadSuggestionsInSheet() {
+    void preloadSuggestionsInSheet(SuggestionsSource suggestionsSource) {
+        SuggestionsNavigationDelegate navigationDelegate = new SuggestionsNavigationDelegateImpl(
+                mActivity, mProfile, mBottomSheetController.getBottomSheet(), mTabModelSelector);
+        SuggestionsUiDelegateImpl uiDelegate =
+                new SuggestionsUiDelegateImpl(suggestionsSource, new DummyEventReporter(),
+                        navigationDelegate, mProfile, mBottomSheetController.getBottomSheet(),
+                        mActivity.getChromeApplication().getReferencePool(),
+                        mActivity.getSnackbarManager());
+
         // TODO(twellington): Introduce another method that creates bottom sheet content with only
         // a toolbar view when suggestions are fist available, and use this method to construct the
         // content view when the sheet is opened.
         mToolbarCoordinator =
                 new ToolbarCoordinator(mActivity, mBottomSheetController.getBottomSheet(), mModel);
         mContentCoordinator = new ContentCoordinator(mActivity,
-                mBottomSheetController.getBottomSheet(), mProfile, mUiDelegate, mModel,
+                mBottomSheetController.getBottomSheet(), mProfile, uiDelegate, mModel,
                 mActivity.getWindowAndroid(), mActivity::closeContextMenu);
         mBottomSheetContent = new ContextualSuggestionsBottomSheetContent(
                 mContentCoordinator, mToolbarCoordinator);

chrome/android/java/src/org/chromium/chrome/browser/contextual_suggestions/ContextualSuggestionsMediator.java

@@ -14,7 +14,6 @@ import org.chromium.chrome.browser.fullscreen.ChromeFullscreenManager;
 import org.chromium.chrome.browser.fullscreen.ChromeFullscreenManager.FullscreenListener;
 import org.chromium.chrome.browser.ntp.snippets.SnippetArticle;
 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.tabmodel.TabModelSelector;
 import org.chromium.chrome.browser.util.MathUtils;
@@ -29,19 +28,24 @@ import java.util.List;
  * the contextual suggestions C++ components (via a bridge), updating the model, and communicating
  * with the component coordinator(s).
  */
-class ContextualSuggestionsMediator implements FetchHelper.Delegate {
+class ContextualSuggestionsMediator implements EnabledStateMonitor.Observer, FetchHelper.Delegate {
     private final Context mContext;
+    private final Profile mProfile;
+    private final TabModelSelector mTabModelSelector;
     private final ContextualSuggestionsCoordinator mCoordinator;
     private final ContextualSuggestionsModel mModel;
-    private final SnippetsBridge mBridge;
-    private final FetchHelper mFetchHelper;
+    private final EnabledStateMonitor mEnabledStateMonitor;
     private final ChromeFullscreenManager mFullscreenManager;
 
-    private boolean mDidSuggestionsShowForTab;
-
+    @Nullable
+    private SnippetsBridge mBridge;
+    @Nullable
+    private FetchHelper mFetchHelper;
     @Nullable
     private String mCurrentRequestUrl;
 
+    private boolean mDidSuggestionsShowForTab;
+
     /**
      * Construct a new {@link ContextualSuggestionsMediator}.
      * @param context The {@link Context} used to retrieve resources.
@@ -56,12 +60,15 @@ class ContextualSuggestionsMediator implements FetchHelper.Delegate {
             TabModelSelector tabModelSelector, ChromeFullscreenManager fullscreenManager,
             ContextualSuggestionsCoordinator coordinator, ContextualSuggestionsModel model) {
         mContext = context;
+        mProfile = profile;
+        mTabModelSelector = tabModelSelector;
         mCoordinator = coordinator;
         mModel = model;
         mFullscreenManager = fullscreenManager;
 
-        mBridge = new SnippetsBridge(Profile.getLastUsedProfile());
-        mFetchHelper = new FetchHelper(this, tabModelSelector);
+        // Create a state monitor that will alert this mediator if the enabled state for contextual
+        // suggestions changes.
+        mEnabledStateMonitor = new EnabledStateMonitor(this);
 
         fullscreenManager.addListener(new FullscreenListener() {
             @Override
@@ -74,7 +81,7 @@ class ContextualSuggestionsMediator implements FetchHelper.Delegate {
                 // remain hidden since their offset from the bottom of the screen is determined by
                 // the top controls.
                 if (!mDidSuggestionsShowForTab && mModel.hasSuggestions()
-                        && areBrowserControlsHidden()) {
+                        && areBrowserControlsHidden() && mBridge != null) {
                     mDidSuggestionsShowForTab = true;
                     mCoordinator.showSuggestions();
                 }
@@ -88,19 +95,10 @@ class ContextualSuggestionsMediator implements FetchHelper.Delegate {
         });
     }
 
-    /**
-     * @return The {@link SuggestionsSource} used to fetch suggestions.
-     *
-     * TODO(twellington): This method is needed to construct {@link SuggestionsUiDelegateImpl} in
-     * the coordinator. Try to remove this dependency.
-     */
-    SuggestionsSource getSuggestionsSource() {
-        return mBridge;
-    }
-
     /** Destroys the mediator. */
     void destroy() {
-        mFetchHelper.destroy();
+        if (mFetchHelper != null) mFetchHelper.destroy();
+        mEnabledStateMonitor.destroy();
     }
 
     /**
@@ -111,6 +109,22 @@ class ContextualSuggestionsMediator implements FetchHelper.Delegate {
                 mFullscreenManager.getTopControlsHeight());
     }
 
+    @Override
+    public void onEnabledStateChanged(boolean enabled) {
+        if (enabled) {
+            mBridge = new SnippetsBridge(mProfile);
+            mFetchHelper = new FetchHelper(this, mTabModelSelector);
+        } else {
+            clearSuggestions();
+
+            mFetchHelper.destroy();
+            mFetchHelper = null;
+
+            mBridge.destroy();
+            mBridge = null;
+        }
+    }
+
     @Override
     public void requestSuggestions(String url) {
         mCurrentRequestUrl = url;
@@ -136,6 +150,11 @@ class ContextualSuggestionsMediator implements FetchHelper.Delegate {
         clearSuggestions();
     }
 
+    /**
+     * Called when suggestions are cleared either due to the user explicitly dismissing
+     * suggestions via the close button or due to the FetchHelper signaling state should
+     * be cleared.
+     */
     private void clearSuggestions() {
         // TODO(twellington): Does this signal need to go back to FetchHelper?
         mDidSuggestionsShowForTab = false;
@@ -147,10 +166,12 @@ class ContextualSuggestionsMediator implements FetchHelper.Delegate {
     }
 
     private void preloadSuggestions(ClusterList clusters, String title) {
+        if (mBridge == null) return;
+
         mModel.setClusterList(clusters);
         mModel.setCloseButtonOnClickListener(view -> { clearSuggestions(); });
         mModel.setTitle(mContext.getString(R.string.contextual_suggestions_toolbar_title, title));
-        mCoordinator.preloadSuggestionsInSheet();
+        mCoordinator.preloadSuggestionsInSheet(mBridge);
     }
 
     // TODO(twellington): Remove after clusters are returned from the backend.

chrome/android/java/src/org/chromium/chrome/browser/contextual_suggestions/EnabledStateMonitor.java

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.chrome.browser.contextual_suggestions;
+
+import org.chromium.chrome.browser.sync.ProfileSyncService;
+import org.chromium.chrome.browser.sync.ProfileSyncService.SyncStateChangedListener;
+import org.chromium.components.sync.ModelType;
+import org.chromium.components.sync.UploadState;
+
+/**
+ * A monitor that is responsible for detecting changes to conditions required for contextual
+ * suggestions to be enabled. Alerts its {@link Observer} when state changes.
+ */
+public class EnabledStateMonitor implements SyncStateChangedListener {
+    /** An observer to be notified of enabled state changes. **/
+    interface Observer {
+        void onEnabledStateChanged(boolean enabled);
+    }
+
+    private final Observer mObserver;
+    private boolean mEnabled;
+
+    /**
+     * Construct a new {@link EnabledStateMonitor}.
+     * @param observer The {@link Observer} to be notified of changes to enabled state.
+     */
+    EnabledStateMonitor(Observer observer) {
+        mObserver = observer;
+        ProfileSyncService.get().addSyncStateChangedListener(this);
+        updateEnabledState();
+    }
+
+    void destroy() {
+        ProfileSyncService.get().removeSyncStateChangedListener(this);
+    }
+
+    @Override
+    public void syncStateChanged() {
+        updateEnabledState();
+    }
+
+    /**
+     * Updates whether contextual suggestions are enabled. Notifies the observer if the
+     * enabled state has changed.
+     */
+    private void updateEnabledState() {
+        boolean previousState = mEnabled;
+
+        ProfileSyncService service = ProfileSyncService.get();
+        mEnabled = service.getUploadToGoogleState(ModelType.HISTORY_DELETE_DIRECTIVES)
+                == UploadState.ACTIVE;
+
+        // TODO(twellington): Add other run-time checks, e.g. enterprise policy, opt-out state.
+
+        if (mEnabled != previousState) mObserver.onEnabledStateChanged(mEnabled);
+    }
+}

chrome/android/java/src/org/chromium/chrome/browser/sync/ProfileSyncService.java

@@ -12,6 +12,7 @@ import org.chromium.base.VisibleForTesting;
 import org.chromium.base.annotations.CalledByNative;
 import org.chromium.components.sync.ModelType;
 import org.chromium.components.sync.PassphraseType;
+import org.chromium.components.sync.UploadState;
 
 import java.util.HashSet;
 import java.util.List;
@@ -407,6 +408,15 @@ public class ProfileSyncService {
         return nativeHasUnrecoverableError(mNativeProfileSyncServiceAndroid);
     }
 
+    /**
+     * @return Whether {@code modelType} is being uploaded to Google. This is useful for features
+     *         that depend on user consent for uploading data (e.g. history) to Google.
+     */
+    @UploadState
+    public int getUploadToGoogleState(@ModelType int modelType) {
+        return nativeGetUploadToGoogleState(mNativeProfileSyncServiceAndroid, modelType);
+    }
+
     /**
      * Called when the state of the native sync engine has changed, so various
      * UI elements can update themselves.
@@ -574,6 +584,8 @@ public class ProfileSyncService {
     private native boolean nativeIsSyncActive(long nativeProfileSyncServiceAndroid);
     private native boolean nativeHasKeepEverythingSynced(long nativeProfileSyncServiceAndroid);
     private native boolean nativeHasUnrecoverableError(long nativeProfileSyncServiceAndroid);
+    private native int nativeGetUploadToGoogleState(
+            long nativeProfileSyncServiceAndroid, int modelType);
     private native boolean nativeIsPassphrasePrompted(long nativeProfileSyncServiceAndroid);
     private native void nativeSetPassphrasePrompted(long nativeProfileSyncServiceAndroid,
                                                     boolean prompted);

chrome/android/java/src/org/chromium/chrome/browser/util/FeatureUtilities.java

@@ -24,6 +24,7 @@ import org.chromium.base.library_loader.LibraryLoader;
 import org.chromium.chrome.browser.ChromeFeatureList;
 import org.chromium.chrome.browser.ChromeSwitches;
 import org.chromium.chrome.browser.firstrun.FirstRunUtils;
+import org.chromium.chrome.browser.locale.LocaleManager;
 import org.chromium.chrome.browser.preferences.ChromePreferenceManager;
 import org.chromium.chrome.browser.tabmodel.DocumentModeAssassin;
 import org.chromium.components.signin.AccountManagerFacade;
@@ -323,11 +324,16 @@ public class FeatureUtilities {
         return sIsChromeModernDesignEnabled;
     }
 
-    /** @return Whether the contextual suggestions bottom sheet is enabled. */
-    public static boolean isContextualSuggestionsBottomSheetEnabled() {
-        // TODO(twellington): Also check for enterprise users and users with history sync enabled
-        // without encryption.
-        return !DeviceFormFactor.isTablet() && isChromeModernDesignEnabled()
+    /**
+     * @param isTablet Whether the containing Activity is being displayed on a tablet-sized screen.
+     * @return Whether the contextual suggestions bottom sheet is enabled.
+     */
+    public static boolean isContextualSuggestionsBottomSheetEnabled(boolean isTablet) {
+        boolean hasSeenSearchEnginePromo =
+                LocaleManager.getInstance().hasCompletedSearchEnginePromo()
+                || LocaleManager.getInstance().hasShownSearchEnginePromoThisSession();
+
+        return !isTablet && !hasSeenSearchEnginePromo && isChromeModernDesignEnabled()
                 && ChromeFeatureList.isEnabled(
                            ChromeFeatureList.CONTEXTUAL_SUGGESTIONS_BOTTOM_SHEET);
     }

chrome/android/java_sources.gni

@@ -302,6 +302,7 @@ chrome_java_sources = [
   "java/src/org/chromium/chrome/browser/contextual_suggestions/ContextualSuggestionsMediator.java",
   "java/src/org/chromium/chrome/browser/contextual_suggestions/ContextualSuggestionsModel.java",
   "java/src/org/chromium/chrome/browser/contextual_suggestions/DummyEventReporter.java",
+  "java/src/org/chromium/chrome/browser/contextual_suggestions/EnabledStateMonitor.java",
   "java/src/org/chromium/chrome/browser/contextual_suggestions/FetchHelper.java",
   "java/src/org/chromium/chrome/browser/contextual_suggestions/PropertyKey.java",
   "java/src/org/chromium/chrome/browser/contextual_suggestions/ToolbarCoordinator.java",

chrome/browser/sync/profile_sync_service_android.cc

@@ -29,8 +29,10 @@
 #include "components/prefs/pref_service.h"
 #include "components/signin/core/browser/signin_manager.h"
 #include "components/strings/grit/components_strings.h"
+#include "components/sync/base/model_type.h"
 #include "components/sync/base/pref_names.h"
 #include "components/sync/driver/about_sync_util.h"
+#include "components/sync/driver/sync_service_utils.h"
 #include "components/sync/engine/net/network_resources.h"
 #include "components/sync/syncable/read_transaction.h"
 #include "content/public/browser/browser_thread.h"
@@ -365,6 +367,15 @@ jboolean ProfileSyncServiceAndroid::HasUnrecoverableError(
   return sync_service_->HasUnrecoverableError();
 }
 
+jint ProfileSyncServiceAndroid::GetUploadToGoogleState(
+    JNIEnv* env,
+    const JavaParamRef<jobject>&,
+    jint model_type) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  return static_cast<int>(syncer::GetUploadToGoogleState(
+      sync_service_, static_cast<syncer::ModelType>(model_type)));
+}
+
 jint ProfileSyncServiceAndroid::GetProtocolErrorClientAction(
     JNIEnv* env,
     const JavaParamRef<jobject>&) {

chrome/browser/sync/profile_sync_service_android.h

@@ -125,6 +125,9 @@ class ProfileSyncServiceAndroid : public syncer::SyncServiceObserver {
   jboolean HasUnrecoverableError(
       JNIEnv* env,
       const base::android::JavaParamRef<jobject>& obj);
+  jint GetUploadToGoogleState(JNIEnv* env,
+                              const base::android::JavaParamRef<jobject>& obj,
+                              jint model_type);
 
   // Gets SyncProtocolError.ClientAction.
   jint GetProtocolErrorClientAction(

components/sync/android/BUILD.gn

@@ -58,6 +58,7 @@ java_cpp_enum("java_enums") {
   sources = [
     "//components/sync/base/model_type.h",
     "//components/sync/base/stop_source.h",
+    "//components/sync/driver/sync_service_utils.h",
     "//components/sync/protocol/sync_protocol_error.h",
   ]
 }

components/sync/driver/sync_service_utils.h

@@ -20,6 +20,8 @@ bool IsTabSyncEnabledAndUnencrypted(SyncService* sync_service,
 
 // Indicates whether uploading of data to Google is enabled, i.e. the user has
 // given consent to upload this data.
+// A Java counterpart will be generated for this enum.
+// GENERATED_JAVA_ENUM_PACKAGE: org.chromium.components.sync
 enum class UploadState {
   // Syncing is enabled in principle, but the sync service is still
   // initializing, so e.g. we don't know about any auth errors yet.