Add canonical check for reporting implicit user actions

Reporting implicit user actions will be enabled in either of these
cases:
* When Feed V1 is enabled AND ReportFeedUserActions is enabled.
* When Feed V2 is enabled.

Bug: 1044139
Change-Id: I0dfa1d8d1f05a82f2308b7a84e5f58919e81b432
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2339590Reviewed-by: Wei-Yin Chen (陳威尹) <wychen@chromium.org>
Reviewed-by: Theresa  <twellington@chromium.org>
Reviewed-by: Dan H <harringtond@chromium.org>
Commit-Queue: Carlos Knippschild <carlosk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#796184}

chrome/android/features/start_surface/internal/java/src/org/chromium/chrome/features/start_surface/ExploreSurfaceCoordinator.java

@@ -13,9 +13,9 @@ import org.chromium.chrome.browser.ChromeActivity;
 import org.chromium.chrome.browser.feed.FeedSurfaceCoordinator;
 import org.chromium.chrome.browser.feed.StreamLifecycleManager;
 import org.chromium.chrome.browser.feed.action.FeedActionHandler;
+import org.chromium.chrome.browser.feed.shared.FeedFeatures;
 import org.chromium.chrome.browser.feed.shared.FeedSurfaceDelegate;
 import org.chromium.chrome.browser.feed.shared.stream.Stream;
-import org.chromium.chrome.browser.flags.ChromeFeatureList;
 import org.chromium.chrome.browser.ntp.snippets.SectionHeaderView;
 import org.chromium.chrome.browser.profiles.Profile;
 import org.chromium.chrome.start_surface.R;
@@ -93,7 +93,7 @@ class ExploreSurfaceCoordinator implements FeedSurfaceDelegate {
         SectionHeaderView sectionHeaderView = null;
         if (hasHeader) {
             LayoutInflater inflater = LayoutInflater.from(mActivity);
-            if (ChromeFeatureList.isEnabled(ChromeFeatureList.REPORT_FEED_USER_ACTIONS)) {
+            if (FeedFeatures.isReportingUserActions()) {
                 sectionHeaderView = (SectionHeaderView) inflater.inflate(
                         R.layout.new_tab_page_snippets_expandable_header_with_menu, null, false);
             } else {

chrome/android/features/start_surface/internal/java/src/org/chromium/chrome/features/start_surface/FeedLoadingLayout.java

@@ -19,8 +19,7 @@ import android.view.ViewGroup;
 import android.widget.ImageView;
 import android.widget.LinearLayout;
 
-import org.chromium.chrome.browser.flags.CachedFeatureFlags;
-import org.chromium.chrome.browser.flags.ChromeFeatureList;
+import org.chromium.chrome.browser.feed.shared.FeedFeatures;
 import org.chromium.chrome.start_surface.R;
 import org.chromium.components.browser_ui.widget.displaystyle.HorizontalDisplayStyle;
 import org.chromium.components.browser_ui.widget.displaystyle.UiConfig;
@@ -75,11 +74,12 @@ public class FeedLoadingLayout extends LinearLayout {
     private void setHeader() {
         LayoutInflater inflater = LayoutInflater.from(mContext);
         View header;
-        // This flag is checked directly with ChromeFeatureList#isEnabled() in other places. Using
-        // CachedFeatureFlags#isEnabled here is deliberate for a pre-native check. This
+        // FeedFeatures.cachedIsReportingUserActions uses CachedFeatureFlags for checking feature
+        // states, but these same features are checked directly with ChromeFeatureList in other
+        // places. Using the cached check here is deliberate for pre-native usage. This
         // inconsistency is fine because the check here is for the Feed header blank size, the
         // mismatch is bearable and only once for every change.
-        if (CachedFeatureFlags.isEnabled(ChromeFeatureList.REPORT_FEED_USER_ACTIONS)) {
+        if (FeedFeatures.cachedIsReportingUserActions()) {
             header = inflater.inflate(
                     R.layout.new_tab_page_snippets_expandable_header_with_menu, null, false);
             header.findViewById(R.id.header_menu).setVisibility(INVISIBLE);

chrome/android/features/tab_ui/java/src/org/chromium/chrome/browser/tasks/TasksView.java

@@ -25,6 +25,7 @@ import androidx.core.view.ViewCompat;
 import com.google.android.material.appbar.AppBarLayout;
 
 import org.chromium.base.ApiCompatibilityUtils;
+import org.chromium.chrome.browser.feed.shared.FeedFeatures;
 import org.chromium.chrome.browser.flags.CachedFeatureFlags;
 import org.chromium.chrome.browser.flags.ChromeFeatureList;
 import org.chromium.chrome.browser.lifecycle.ActivityLifecycleDispatcher;
@@ -109,13 +110,13 @@ class TasksView extends CoordinatorLayoutForPointer {
 
     private void setTabCarouselTitleStyle() {
         // Match the tab carousel title style with the feed header.
-        // TODO(crbug.com/1016952): Migrate ChromeFeatureList.isEnabled to using cached flags for
-        // instant start. There are many places checking REPORT_FEED_USER_ACTIONS, like in
+        // TODO(crbug.com/1016952): Migrate feature flag checks to use cached flags for instant
+        // start. There are many places checking FeedFeatures.isReportingUserActions, like in
         // ExploreSurfaceCoordinator.
         TextView titleDescription = (TextView) findViewById(R.id.tab_switcher_title_description);
         TextView moreTabs = (TextView) findViewById(R.id.more_tabs);
         if (!CachedFeatureFlags.isEnabled(ChromeFeatureList.INSTANT_START)
-                && ChromeFeatureList.isEnabled(ChromeFeatureList.REPORT_FEED_USER_ACTIONS)) {
+                && FeedFeatures.isReportingUserActions()) {
             ApiCompatibilityUtils.setTextAppearance(
                     titleDescription, R.style.TextAppearance_TextSmall_Secondary);
             ApiCompatibilityUtils.setTextAppearance(

chrome/android/features/tab_ui/javatests/src/org/chromium/chrome/browser/tasks/TasksViewBinderTest.java

@@ -55,7 +55,7 @@ import org.chromium.ui.modelutil.PropertyModel;
 import org.chromium.ui.modelutil.PropertyModelChangeProcessor;
 import org.chromium.ui.test.util.DummyUiActivityTestCase;
 
-import java.util.Collections;
+import java.util.HashMap;
 import java.util.Map;
 import java.util.concurrent.atomic.AtomicBoolean;
 
@@ -74,8 +74,10 @@ public class TasksViewBinderTest extends DummyUiActivityTestCase {
         super.setUpTest();
         MockitoAnnotations.initMocks(this);
 
-        Map<String, Boolean> testFeatures =
-                Collections.singletonMap(ChromeFeatureList.REPORT_FEED_USER_ACTIONS, true);
+        Map<String, Boolean> testFeatures = new HashMap<>();
+        testFeatures.put(ChromeFeatureList.INTEREST_FEED_V2, false);
+        testFeatures.put(ChromeFeatureList.INTEREST_FEED_CONTENT_SUGGESTIONS, true);
+        testFeatures.put(ChromeFeatureList.REPORT_FEED_USER_ACTIONS, false);
         ChromeFeatureList.setTestFeatures(testFeatures);
 
         TestThreadUtils.runOnUiThreadBlocking(() -> {

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

@@ -19,10 +19,10 @@ import androidx.recyclerview.widget.RecyclerView;
 import org.chromium.base.MemoryPressureListener;
 import org.chromium.base.memory.MemoryPressureCallback;
 import org.chromium.chrome.R;
+import org.chromium.chrome.browser.feed.shared.FeedFeatures;
 import org.chromium.chrome.browser.feed.shared.stream.Stream;
 import org.chromium.chrome.browser.feed.shared.stream.Stream.ContentChangedListener;
 import org.chromium.chrome.browser.feed.shared.stream.Stream.ScrollListener;
-import org.chromium.chrome.browser.flags.ChromeFeatureList;
 import org.chromium.chrome.browser.native_page.ContextMenuManager;
 import org.chromium.chrome.browser.native_page.NativePageNavigationDelegate;
 import org.chromium.chrome.browser.ntp.NewTabPageLayout;
@@ -121,7 +121,7 @@ public class FeedSurfaceMediator
         mPrefChangeRegistrar = new PrefChangeRegistrar();
         mHasHeader = mCoordinator.getSectionHeaderView() != null;
         mPrefChangeRegistrar.addObserver(Pref.ENABLE_SNIPPETS, this::updateContent);
-        mHasHeaderMenu = ChromeFeatureList.isEnabled(ChromeFeatureList.REPORT_FEED_USER_ACTIONS);
+        mHasHeaderMenu = FeedFeatures.isReportingUserActions();
 
         // Check that there is a navigation delegate when using the feed header menu.
         if (mPageNavigationDelegate == null && mHasHeaderMenu) {

chrome/android/feed/core/java/src/org/chromium/chrome/browser/feed/library/basicstream/internal/actions/StreamActionApiImpl.java

@@ -24,7 +24,7 @@ import org.chromium.chrome.browser.feed.library.api.internal.actionparser.Action
 import org.chromium.chrome.browser.feed.library.basicstream.internal.pendingdismiss.ClusterPendingDismissHelper;
 import org.chromium.chrome.browser.feed.library.sharedstream.contextmenumanager.ContextMenuManager;
 import org.chromium.chrome.browser.feed.library.sharedstream.pendingdismiss.PendingDismissCallback;
-import org.chromium.chrome.browser.flags.ChromeFeatureList;
+import org.chromium.chrome.browser.feed.shared.FeedFeatures;
 import org.chromium.components.feed.core.proto.libraries.api.internal.StreamDataProto.StreamDataOperation;
 import org.chromium.components.feed.core.proto.ui.action.FeedActionProto.FeedActionMetadata.ElementType;
 import org.chromium.components.feed.core.proto.ui.action.FeedActionProto.LabelledFeedActionData;
@@ -333,21 +333,21 @@ public class StreamActionApiImpl implements StreamActionApi {
 
     @Override
     public void reportClickAction(String contentId, ActionPayload payload) {
-        if (ChromeFeatureList.isEnabled(ChromeFeatureList.REPORT_FEED_USER_ACTIONS)) {
+        if (FeedFeatures.isReportingUserActions()) {
             mActionManager.createAndUploadAction(contentId, payload);
         }
     }
 
     @Override
     public void reportViewVisible(View view, String contentId, ActionPayload payload) {
-        if (ChromeFeatureList.isEnabled(ChromeFeatureList.REPORT_FEED_USER_ACTIONS)) {
+        if (FeedFeatures.isReportingUserActions()) {
             mActionManager.onViewVisible(view, contentId, payload);
         }
     }
 
     @Override
     public void reportViewHidden(View view, String contentId) {
-        if (ChromeFeatureList.isEnabled(ChromeFeatureList.REPORT_FEED_USER_ACTIONS)) {
+        if (FeedFeatures.isReportingUserActions()) {
             mActionManager.onViewHidden(view, contentId);
         }
     }

chrome/android/feed/core/java/src/org/chromium/chrome/browser/feed/library/feedactionmanager/FeedActionManagerImpl.java

@@ -32,6 +32,7 @@ import org.chromium.chrome.browser.feed.library.common.concurrent.MainThreadRunn
 import org.chromium.chrome.browser.feed.library.common.concurrent.TaskQueue;
 import org.chromium.chrome.browser.feed.library.common.concurrent.TaskQueue.TaskType;
 import org.chromium.chrome.browser.feed.library.common.time.Clock;
+import org.chromium.chrome.browser.feed.shared.FeedFeatures;
 import org.chromium.chrome.browser.feed.shared.stream.Stream.ScrollListener;
 import org.chromium.chrome.browser.feed.shared.stream.Stream.ScrollListener.ScrollState;
 import org.chromium.chrome.browser.flags.ChromeFeatureList;
@@ -355,7 +356,7 @@ public class FeedActionManagerImpl implements ActionManager {
 
     private void reportViewActions(Runnable doneCallback) {
         Set<StreamUploadableAction> actions = new HashSet<>();
-        if (ChromeFeatureList.isEnabled(ChromeFeatureList.REPORT_FEED_USER_ACTIONS)) {
+        if (FeedFeatures.isReportingUserActions()) {
             Iterator<Map.Entry<String, ViewActionData>> entryIterator =
                     mContentData.entrySet().iterator();
 

chrome/android/feed/core/java/src/org/chromium/chrome/browser/feed/library/feedrequestmanager/FeedRequestManagerImpl.java

@@ -41,7 +41,7 @@ import org.chromium.chrome.browser.feed.library.common.protoextensions.FeedExten
 import org.chromium.chrome.browser.feed.library.common.time.TimingUtils;
 import org.chromium.chrome.browser.feed.library.common.time.TimingUtils.ElapsedTimeTracker;
 import org.chromium.chrome.browser.feed.library.feedrequestmanager.internal.Utils;
-import org.chromium.chrome.browser.flags.ChromeFeatureList;
+import org.chromium.chrome.browser.feed.shared.FeedFeatures;
 import org.chromium.chrome.browser.profiles.Profile;
 import org.chromium.chrome.browser.profiles.ProfileManager;
 import org.chromium.chrome.browser.signin.IdentityServicesProvider;
@@ -395,7 +395,7 @@ public class FeedRequestManagerImpl implements FeedRequestManager {
             addCapabilityIfConfigEnabled(feedRequestBuilder, ConfigKey.USE_SECONDARY_PAGE_REQUEST,
                     Capability.USE_SECONDARY_PAGE_REQUEST);
 
-            if (ChromeFeatureList.isEnabled(ChromeFeatureList.REPORT_FEED_USER_ACTIONS)) {
+            if (FeedFeatures.isReportingUserActions()) {
                 feedRequestBuilder.addClientCapability(Capability.CLICK_ACTION);
                 feedRequestBuilder.addClientCapability(Capability.VIEW_ACTION);
                 feedRequestBuilder.addClientCapability(

chrome/android/feed/core/java/src/org/chromium/chrome/browser/feed/shared/FeedFeatures.java

+// Copyright 2020 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.feed.shared;
+
+import org.chromium.chrome.browser.flags.CachedFeatureFlags;
+import org.chromium.chrome.browser.flags.ChromeFeatureList;
+
+/**
+ * Helper methods covering more complex Feed related feature checks and states.
+ */
+public final class FeedFeatures {
+    /**
+     * @return Whether implicit Feed user actions are being reported based on feature states. Can be
+     *         used for both Feed v1 and v2.
+     */
+    public static boolean isReportingUserActions() {
+        return ChromeFeatureList.isEnabled(ChromeFeatureList.INTEREST_FEED_V2)
+                || (ChromeFeatureList.isEnabled(ChromeFeatureList.INTEREST_FEED_CONTENT_SUGGESTIONS)
+                        && ChromeFeatureList.isEnabled(ChromeFeatureList.REPORT_FEED_USER_ACTIONS));
+    }
+
+    /**
+     * Identical to {@link isReportingUserActions} but uses {@link CachedFeatureFlags} for checking
+     * feature states.
+     */
+    public static boolean cachedIsReportingUserActions() {
+        return CachedFeatureFlags.isEnabled(ChromeFeatureList.INTEREST_FEED_V2)
+                || (CachedFeatureFlags.isEnabled(
+                            ChromeFeatureList.INTEREST_FEED_CONTENT_SUGGESTIONS)
+                        && CachedFeatureFlags.isEnabled(
+                                ChromeFeatureList.REPORT_FEED_USER_ACTIONS));
+    }
+}
\ No newline at end of file

chrome/android/feed/core/javatests/src/org/chromium/chrome/browser/feed/library/basicstream/internal/actions/StreamActionApiImplTest.java

@@ -78,6 +78,8 @@ import java.util.List;
 /** Tests for {@link StreamActionApiImpl}. */
 @RunWith(LocalRobolectricTestRunner.class)
 @Config(manifest = Config.NONE)
+@Features.DisableFeatures(ChromeFeatureList.INTEREST_FEED_V2)
+@Features.EnableFeatures(ChromeFeatureList.INTEREST_FEED_CONTENT_SUGGESTIONS)
 public class StreamActionApiImplTest {
     private static final String URL = "www.google.com";
     private static final String OPEN_LABEL = "Open";

chrome/android/feed/core/javatests/src/org/chromium/chrome/browser/feed/library/feedactionmanager/FeedActionManagerImplTest.java

@@ -65,7 +65,9 @@ import java.util.Set;
 /** Tests of the {@link FeedActionManagerImpl} class. */
 @RunWith(LocalRobolectricTestRunner.class)
 @Config(manifest = Config.NONE)
-@Features.EnableFeatures(ChromeFeatureList.REPORT_FEED_USER_ACTIONS)
+@Features.DisableFeatures(ChromeFeatureList.INTEREST_FEED_V2)
+@Features.EnableFeatures({ChromeFeatureList.INTEREST_FEED_CONTENT_SUGGESTIONS,
+        ChromeFeatureList.REPORT_FEED_USER_ACTIONS})
 public class FeedActionManagerImplTest {
     private static final String CONTENT_ID_STRING = "contentIdString";
     private static final String SESSION_ID = "session";

chrome/android/feed/core/javatests/src/org/chromium/chrome/browser/feed/library/feedrequestmanager/FeedRequestManagerImplTest.java

@@ -95,7 +95,9 @@ import java.util.Set;
 /** Test of the {@link FeedRequestManagerImpl} class. */
 @RunWith(LocalRobolectricTestRunner.class)
 @Config(manifest = Config.NONE)
-@Features.DisableFeatures(ChromeFeatureList.REPORT_FEED_USER_ACTIONS)
+@Features.EnableFeatures(ChromeFeatureList.INTEREST_FEED_CONTENT_SUGGESTIONS)
+@Features.
+DisableFeatures({ChromeFeatureList.REPORT_FEED_USER_ACTIONS, ChromeFeatureList.INTEREST_FEED_V2})
 public class FeedRequestManagerImplTest {
     private static final int NOT_FOUND = 404;
     private static final String TABLE = "table";

chrome/android/feed/core/javatests/src/org/chromium/chrome/browser/feed/library/mocknetworkclient/MockServerNetworkClientTest.java

@@ -68,7 +68,9 @@ import java.util.Collections;
 /** Tests of the {@link MockServerNetworkClient} class. */
 @RunWith(LocalRobolectricTestRunner.class)
 @Config(manifest = Config.NONE)
-@Features.DisableFeatures(ChromeFeatureList.REPORT_FEED_USER_ACTIONS)
+@Features.EnableFeatures(ChromeFeatureList.INTEREST_FEED_CONTENT_SUGGESTIONS)
+@Features.
+DisableFeatures({ChromeFeatureList.REPORT_FEED_USER_ACTIONS, ChromeFeatureList.INTEREST_FEED_V2})
 public class MockServerNetworkClientTest extends NetworkClientConformanceTest {
     private final Configuration mConfiguration = new Configuration.Builder().build();
     private final FakeClock mFakeClock = new FakeClock();

chrome/android/feed/feed_java_sources.gni

@@ -374,6 +374,7 @@ if (enable_feed_in_chrome) {
     "//chrome/android/feed/core/java/src/org/chromium/chrome/browser/feed/library/sharedstream/scroll/ScrollListenerNotifier.java",
     "//chrome/android/feed/core/java/src/org/chromium/chrome/browser/feed/library/sharedstream/scroll/ScrollLogger.java",
     "//chrome/android/feed/core/java/src/org/chromium/chrome/browser/feed/library/sharedstream/scroll/ScrollRestoreHelper.java",
+    "//chrome/android/feed/core/java/src/org/chromium/chrome/browser/feed/shared/FeedFeatures.java",
     "//chrome/android/feed/core/java/src/org/chromium/chrome/browser/feed/shared/FeedSurfaceDelegate.java",
     "//chrome/android/feed/core/java/src/org/chromium/chrome/browser/feed/shared/FeedSurfaceProvider.java",
     "//chrome/android/feed/core/java/src/org/chromium/chrome/browser/feed/shared/ScrollTracker.java",

chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java

@@ -36,10 +36,10 @@ import org.chromium.chrome.browser.feed.FeedSurfaceCoordinator;
 import org.chromium.chrome.browser.feed.NtpStreamLifecycleManager;
 import org.chromium.chrome.browser.feed.StreamLifecycleManager;
 import org.chromium.chrome.browser.feed.action.FeedActionHandler;
+import org.chromium.chrome.browser.feed.shared.FeedFeatures;
 import org.chromium.chrome.browser.feed.shared.FeedSurfaceDelegate;
 import org.chromium.chrome.browser.feed.shared.FeedSurfaceProvider;
 import org.chromium.chrome.browser.feed.shared.stream.Stream;
-import org.chromium.chrome.browser.flags.ChromeFeatureList;
 import org.chromium.chrome.browser.lifecycle.ActivityLifecycleDispatcher;
 import org.chromium.chrome.browser.lifecycle.LifecycleObserver;
 import org.chromium.chrome.browser.lifecycle.PauseResumeWithNativeObserver;
@@ -420,7 +420,7 @@ public class NewTabPage implements NativePage, InvalidationAwareThumbnailProvide
 
         // Determine the feed header to use.
         final SectionHeaderView sectionHeaderView;
-        if (ChromeFeatureList.isEnabled(ChromeFeatureList.REPORT_FEED_USER_ACTIONS)) {
+        if (FeedFeatures.isReportingUserActions()) {
             sectionHeaderView = (SectionHeaderView) inflater.inflate(
                     R.layout.new_tab_page_snippets_expandable_header_with_menu, null, false);
         } else {

chrome/browser/flags/android/java/src/org/chromium/chrome/browser/flags/CachedFeatureFlags.java

@@ -53,7 +53,7 @@ public class CachedFeatureFlags {
             put(ChromeFeatureList.HOMEPAGE_LOCATION_POLICY, false);
             put(ChromeFeatureList.SERVICE_MANAGER_FOR_DOWNLOAD, false);
             put(ChromeFeatureList.SERVICE_MANAGER_FOR_BACKGROUND_PREFETCH, false);
-            put(ChromeFeatureList.INTEREST_FEED_CONTENT_SUGGESTIONS, false);
+            put(ChromeFeatureList.INTEREST_FEED_CONTENT_SUGGESTIONS, true);
             put(ChromeFeatureList.CHROME_DUET, false);
             put(ChromeFeatureList.COMMAND_LINE_ON_NON_ROOTED, false);
             put(ChromeFeatureList.CHROME_DUET_ADAPTIVE, true);