[go: up one dir, main page]

[Feed Position] Read result from segmentation platform for enabling Feed Position.

1. Use segmentation result to decide whether to enable Feed Position
   changes if target Feed/non-Feed is enabled.
2. Add shouldShowLogo() checks in getGridMvtTopMargin() and
   getGridMvtBottomMargin() in NewTabPageLayout.

(cherry picked from commit 1891d9e2d5bcea55ff4560ccbb5e79a3101ffc24)

Bug: 1329288
Change-Id: Icef1bf9f644d4c5e2e852bdd4b0257941291026f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3724947
Reviewed-by: Sky Malice <skym@chromium.org>
Reviewed-by: Siddhartha S <ssid@chromium.org>
Commit-Queue: Hao Dong <spdonghao@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Mark Pearson <mpearson@chromium.org>
Reviewed-by: Xi Han <hanxi@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1021998}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3749762
Cr-Commit-Position: refs/branch-heads/5112@{#817}
Cr-Branched-From: b13d3fe7b3c47a56354ef54b221008afa754412e-refs/heads/main@{#1012729}
diff --git a/chrome/android/BUILD.gn b/chrome/android/BUILD.gn
index 44c7cdd..4b5c239 100644
--- a/chrome/android/BUILD.gn
+++ b/chrome/android/BUILD.gn
@@ -1040,6 +1040,7 @@
     "//chrome/browser/safety_check/android:java",
     "//chrome/browser/safety_check/android:junit",
     "//chrome/browser/search_engines/android:java",
+    "//chrome/browser/segmentation_platform:factory_java",
     "//chrome/browser/share:java",
     "//chrome/browser/signin/services/android:java",
     "//chrome/browser/signin/services/android:junit",
@@ -1146,6 +1147,7 @@
     "//components/search_engines/android:java",
     "//components/security_state/content/android:java",
     "//components/security_state/core:security_state_enums_java",
+    "//components/segmentation_platform/public:public_java",
     "//components/signin/core/browser:signin_enums_java",
     "//components/signin/public/android:java",
     "//components/signin/public/android:signin_java_test_support",
diff --git a/chrome/android/chrome_junit_test_java_sources.gni b/chrome/android/chrome_junit_test_java_sources.gni
index 1ee2f1fe..547ff55 100644
--- a/chrome/android/chrome_junit_test_java_sources.gni
+++ b/chrome/android/chrome_junit_test_java_sources.gni
@@ -161,6 +161,7 @@
   "junit/src/org/chromium/chrome/browser/notifications/NotificationTriggerBackgroundTaskTest.java",
   "junit/src/org/chromium/chrome/browser/notifications/NotificationTriggerSchedulerTest.java",
   "junit/src/org/chromium/chrome/browser/notifications/WebPlatformNotificationMetricsTest.java",
+  "junit/src/org/chromium/chrome/browser/ntp/FeedPositionUtilUnitTest.java",
   "junit/src/org/chromium/chrome/browser/offlinepages/BackgroundSchedulerTest.java",
   "junit/src/org/chromium/chrome/browser/offlinepages/CctOfflinePageModelObserverTest.java",
   "junit/src/org/chromium/chrome/browser/offlinepages/ClientIdTest.java",
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/ntp/FeedPositionUtils.java b/chrome/android/java/src/org/chromium/chrome/browser/ntp/FeedPositionUtils.java
index d96db100..e14b467 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/ntp/FeedPositionUtils.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/ntp/FeedPositionUtils.java
@@ -4,28 +4,127 @@
 
 package org.chromium.chrome.browser.ntp;
 
+import androidx.annotation.IntDef;
+import androidx.annotation.VisibleForTesting;
+
+import org.chromium.base.metrics.RecordHistogram;
 import org.chromium.chrome.browser.flags.ChromeFeatureList;
+import org.chromium.chrome.browser.profiles.Profile;
+import org.chromium.chrome.browser.segmentation_platform.SegmentationPlatformServiceFactory;
+import org.chromium.components.segmentation_platform.SegmentSelectionResult;
+import org.chromium.components.segmentation_platform.SegmentationPlatformService;
+import org.chromium.components.segmentation_platform.proto.SegmentationProto.SegmentId;
+
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
 
 /**
  * A class to handle the state of flags for Feed position experiment.
  */
 public class FeedPositionUtils {
+    // The key is used to decide whether the user likes to use Feed. Should be consistent with
+    // |kFeedUserSegmentationKey| in config.h in components/segmentation_platform/.
+    public static final String FEED_USER_SEGMENT_KEY = "feed_user_segment";
+    public static final String FEED_ACTIVE_TARGETING = "feed_active_targeting";
     public static final String PUSH_DOWN_FEED_SMALL = "push_down_feed_small";
     public static final String PUSH_DOWN_FEED_LARGE = "push_down_feed_large";
     public static final String PULL_UP_FEED = "pull_up_feed";
 
+    // Constants with FeedPositionSegmentationResult in enums.xml.
+    // These values are persisted to logs. Entries should not be renumbered and
+    // numeric values should never be reused.
+    @IntDef({FeedPositionSegmentationResult.UNINITIALIZED,
+            FeedPositionSegmentationResult.IS_FEED_ACTIVE_USER,
+            FeedPositionSegmentationResult.IS_NON_FEED_ACTIVE_USER,
+            FeedPositionSegmentationResult.NUM_ENTRIES})
+    @Retention(RetentionPolicy.SOURCE)
+    public @interface FeedPositionSegmentationResult {
+        int UNINITIALIZED = 0;
+        int IS_FEED_ACTIVE_USER = 1;
+        int IS_NON_FEED_ACTIVE_USER = 2;
+        int NUM_ENTRIES = 3;
+    }
+
+    /**
+     * Returns whether the pushing down (small) Feed experiment is enabled.
+     */
     public static boolean isFeedPushDownSmallEnabled() {
         return ChromeFeatureList.getFieldTrialParamByFeatureAsBoolean(
-                ChromeFeatureList.FEED_POSITION_ANDROID, PUSH_DOWN_FEED_SMALL, false);
+                       ChromeFeatureList.FEED_POSITION_ANDROID, PUSH_DOWN_FEED_SMALL, false)
+                && getBehaviourResultFromSegmentation();
     }
 
+    /**
+     * Returns whether the pushing down (large) Feed experiment is enabled.
+     */
     public static boolean isFeedPushDownLargeEnabled() {
         return ChromeFeatureList.getFieldTrialParamByFeatureAsBoolean(
-                ChromeFeatureList.FEED_POSITION_ANDROID, PUSH_DOWN_FEED_LARGE, false);
+                       ChromeFeatureList.FEED_POSITION_ANDROID, PUSH_DOWN_FEED_LARGE, false)
+                && getBehaviourResultFromSegmentation();
     }
 
+    /**
+     * Returns whether the pulling up Feed experiment is enabled.
+     */
     public static boolean isFeedPullUpEnabled() {
         return ChromeFeatureList.getFieldTrialParamByFeatureAsBoolean(
-                ChromeFeatureList.FEED_POSITION_ANDROID, PULL_UP_FEED, false);
+                       ChromeFeatureList.FEED_POSITION_ANDROID, PULL_UP_FEED, false)
+                && getBehaviourResultFromSegmentation();
+    }
+
+    /**
+     * Returns the string for whether we should target to Feed active users or non-Feed users:
+     * 1. "active" means targeting to Feed active users.
+     * 2. "non-active" means targeting to Non-Feed active users.
+     * 3. Other string (including empty string) is for no targeting.
+     */
+    @VisibleForTesting
+    static String getTargetFeedOrNonFeedUsersParam() {
+        return ChromeFeatureList.getFieldTrialParamByFeature(
+                ChromeFeatureList.FEED_POSITION_ANDROID, FEED_ACTIVE_TARGETING);
+    }
+
+    /**
+     * Called to check whether feed position should be enabled based on segmentation model result.
+     * Check whether we should target Feed active users, if not, return true; if so, check whether
+     * the user is a Feed/non-Feed active users accordingly.
+     */
+    private static boolean getBehaviourResultFromSegmentation() {
+        String targetFeedOrNonFeedUsersParam = getTargetFeedOrNonFeedUsersParam();
+        if (targetFeedOrNonFeedUsersParam == null) return true;
+
+        @FeedPositionSegmentationResult
+        int resultEnum = FeedPositionUtils.getSegmentationResult();
+        RecordHistogram.recordEnumeratedHistogram("NewTabPage.FeedPositionSegmentationResult",
+                resultEnum, FeedPositionSegmentationResult.NUM_ENTRIES);
+        switch (targetFeedOrNonFeedUsersParam) {
+            case "active":
+                return resultEnum == FeedPositionSegmentationResult.IS_FEED_ACTIVE_USER;
+            case "non-active":
+                return resultEnum == FeedPositionSegmentationResult.IS_NON_FEED_ACTIVE_USER;
+        }
+        return true;
+    }
+
+    /**
+     * Called to get segment selection result from segmentation platform service.
+     * @return The segmentation result.
+     */
+    private static @FeedPositionSegmentationResult int getSegmentationResult() {
+        @FeedPositionSegmentationResult
+        int resultEnum;
+        SegmentationPlatformService segmentationPlatformService =
+                SegmentationPlatformServiceFactory.getForProfile(
+                        Profile.getLastUsedRegularProfile());
+        SegmentSelectionResult result =
+                segmentationPlatformService.getCachedSegmentResult(FEED_USER_SEGMENT_KEY);
+        if (!result.isReady) {
+            resultEnum = FeedPositionSegmentationResult.UNINITIALIZED;
+        } else if (result.selectedSegment == SegmentId.OPTIMIZATION_TARGET_SEGMENTATION_FEED_USER) {
+            resultEnum = FeedPositionSegmentationResult.IS_FEED_ACTIVE_USER;
+        } else {
+            resultEnum = FeedPositionSegmentationResult.IS_NON_FEED_ACTIVE_USER;
+        }
+        return resultEnum;
     }
 }
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageLayout.java b/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageLayout.java
index f455f5d..a00ab4a 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageLayout.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageLayout.java
@@ -943,8 +943,12 @@
 
     // TODO(crbug.com/1329288): Remove this method when the Feed position experiment is cleaned up.
     private int getGridMvtTopMargin() {
-        int resourcesId = !shouldShowLogo() ? R.dimen.tile_grid_layout_no_logo_top_margin
-                                            : R.dimen.tile_grid_layout_top_margin;
+        if (!shouldShowLogo()) {
+            return getResources().getDimensionPixelOffset(
+                    R.dimen.tile_grid_layout_no_logo_top_margin);
+        }
+
+        int resourcesId = R.dimen.tile_grid_layout_top_margin;
 
         if (FeedPositionUtils.isFeedPushDownLargeEnabled()) {
             resourcesId = R.dimen.tile_grid_layout_top_margin_push_down_large;
@@ -961,6 +965,8 @@
     private int getGridMvtBottomMargin() {
         int resourcesId = R.dimen.tile_grid_layout_bottom_margin;
 
+        if (!shouldShowLogo()) return getResources().getDimensionPixelOffset(resourcesId);
+
         if (FeedPositionUtils.isFeedPushDownLargeEnabled()) {
             resourcesId = R.dimen.tile_grid_layout_bottom_margin_push_down_large;
         } else if (FeedPositionUtils.isFeedPushDownSmallEnabled()) {
diff --git a/chrome/android/junit/src/org/chromium/chrome/browser/ntp/DEPS b/chrome/android/junit/src/org/chromium/chrome/browser/ntp/DEPS
new file mode 100644
index 0000000..e87bac6
--- /dev/null
+++ b/chrome/android/junit/src/org/chromium/chrome/browser/ntp/DEPS
@@ -0,0 +1,4 @@
+include_rules = [
+  "+components/segmentation_platform/public/android/java/src/org/chromium/components/segmentation_platform/SegmentSelectionResult.java",
+  "+components/segmentation_platform/public/android/java/src/org/chromium/components/segmentation_platform/SegmentationPlatformService.java",
+]
diff --git a/chrome/android/junit/src/org/chromium/chrome/browser/ntp/FeedPositionUtilUnitTest.java b/chrome/android/junit/src/org/chromium/chrome/browser/ntp/FeedPositionUtilUnitTest.java
new file mode 100644
index 0000000..1bbd0a461
--- /dev/null
+++ b/chrome/android/junit/src/org/chromium/chrome/browser/ntp/FeedPositionUtilUnitTest.java
@@ -0,0 +1,124 @@
+// Copyright 2022 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.ntp;
+
+import static org.mockito.Mockito.when;
+
+import static org.chromium.chrome.browser.ntp.FeedPositionUtils.FEED_USER_SEGMENT_KEY;
+
+import androidx.test.filters.SmallTest;
+
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TestRule;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
+import org.robolectric.annotation.Config;
+
+import org.chromium.base.FeatureList;
+import org.chromium.base.FeatureList.TestValues;
+import org.chromium.base.test.BaseRobolectricTestRunner;
+import org.chromium.chrome.browser.flags.ChromeFeatureList;
+import org.chromium.chrome.browser.profiles.Profile;
+import org.chromium.chrome.browser.segmentation_platform.SegmentationPlatformServiceFactory;
+import org.chromium.chrome.test.util.browser.Features;
+import org.chromium.components.segmentation_platform.SegmentSelectionResult;
+import org.chromium.components.segmentation_platform.SegmentationPlatformService;
+import org.chromium.components.segmentation_platform.proto.SegmentationProto.SegmentId;
+
+/** Unit tests for {@link FeedPositionUtils} class. */
+@RunWith(BaseRobolectricTestRunner.class)
+@Config(manifest = Config.NONE)
+public class FeedPositionUtilUnitTest {
+    @Rule
+    public TestRule mProcessor = new Features.JUnitProcessor();
+
+    private final TestValues mTestValues = new TestValues();
+
+    @Mock
+    Profile mProfile;
+    @Mock
+    SegmentationPlatformService mSegmentationPlatformService;
+
+    @Before
+    public void setUp() {
+        MockitoAnnotations.initMocks(this);
+        Profile.setLastUsedProfileForTesting(mProfile);
+        SegmentationPlatformServiceFactory.setForTests(mSegmentationPlatformService);
+        when(mSegmentationPlatformService.getCachedSegmentResult(FEED_USER_SEGMENT_KEY))
+                .thenReturn(new SegmentSelectionResult(false, null));
+    }
+
+    @After
+    public void tearDown() {
+        FeatureList.setTestValues(null);
+    }
+
+    @Test
+    @SmallTest
+    public void testIsFeedPushDownSmallEnabled() {
+        setFeedPositionFlags(FeedPositionUtils.PUSH_DOWN_FEED_SMALL, "");
+        Assert.assertTrue(FeedPositionUtils.isFeedPushDownSmallEnabled());
+
+        setFeedPositionFlags(FeedPositionUtils.PUSH_DOWN_FEED_SMALL, "active");
+        Assert.assertFalse(FeedPositionUtils.isFeedPushDownSmallEnabled());
+
+        when(mSegmentationPlatformService.getCachedSegmentResult(FEED_USER_SEGMENT_KEY))
+                .thenReturn(new SegmentSelectionResult(
+                        true, SegmentId.OPTIMIZATION_TARGET_SEGMENTATION_FEED_USER));
+        Assert.assertTrue(FeedPositionUtils.isFeedPushDownSmallEnabled());
+    }
+
+    @Test
+    @SmallTest
+    public void testIsFeedPushDownLargeEnabled() {
+        setFeedPositionFlags(FeedPositionUtils.PUSH_DOWN_FEED_LARGE, "");
+        Assert.assertTrue(FeedPositionUtils.isFeedPushDownLargeEnabled());
+
+        setFeedPositionFlags(FeedPositionUtils.PUSH_DOWN_FEED_SMALL, "active");
+        Assert.assertFalse(FeedPositionUtils.isFeedPushDownLargeEnabled());
+
+        when(mSegmentationPlatformService.getCachedSegmentResult(FEED_USER_SEGMENT_KEY))
+                .thenReturn(new SegmentSelectionResult(
+                        true, SegmentId.OPTIMIZATION_TARGET_SEGMENTATION_FEED_USER));
+        Assert.assertTrue(FeedPositionUtils.isFeedPushDownLargeEnabled());
+    }
+
+    @Test
+    @SmallTest
+    public void testIsFeedPullUpEnabled() {
+        setFeedPositionFlags(FeedPositionUtils.PULL_UP_FEED, "");
+        Assert.assertTrue(FeedPositionUtils.isFeedPullUpEnabled());
+
+        setFeedPositionFlags(FeedPositionUtils.PULL_UP_FEED, "non-active");
+        Assert.assertFalse(FeedPositionUtils.isFeedPullUpEnabled());
+
+        when(mSegmentationPlatformService.getCachedSegmentResult(FEED_USER_SEGMENT_KEY))
+                .thenReturn(new SegmentSelectionResult(
+                        true, SegmentId.OPTIMIZATION_TARGET_SEGMENTATION_FEED_USER));
+        Assert.assertFalse(FeedPositionUtils.isFeedPullUpEnabled());
+
+        when(mSegmentationPlatformService.getCachedSegmentResult(FEED_USER_SEGMENT_KEY))
+                .thenReturn(
+                        new SegmentSelectionResult(true, SegmentId.OPTIMIZATION_TARGET_UNKNOWN));
+        Assert.assertTrue(FeedPositionUtils.isFeedPullUpEnabled());
+    }
+
+    private void setFeedPositionFlags(
+            String feedPositionVariation, String targetFeedOrNonFeedUsersValue) {
+        mTestValues.addFieldTrialParamOverride(
+                ChromeFeatureList.FEED_POSITION_ANDROID, feedPositionVariation, "true");
+        mTestValues.addFieldTrialParamOverride(ChromeFeatureList.FEED_POSITION_ANDROID,
+                FeedPositionUtils.FEED_ACTIVE_TARGETING, targetFeedOrNonFeedUsersValue);
+        FeatureList.setTestValues(mTestValues);
+
+        Assert.assertEquals(targetFeedOrNonFeedUsersValue,
+                FeedPositionUtils.getTargetFeedOrNonFeedUsersParam());
+    }
+}
diff --git a/chrome/browser/about_flags.cc b/chrome/browser/about_flags.cc
index a247464..334c4e7 100644
--- a/chrome/browser/about_flags.cc
+++ b/chrome/browser/about_flags.cc
@@ -2155,6 +2155,26 @@
 const FeatureEntry::FeatureParam kFeedPositionAndroid_pull_up_feed[] = {
     {"pull_up_feed", "true"}};
 
+const FeatureEntry::FeatureParam
+    kFeedPositionAndroid_push_down_feed_large_target_feed_active[] = {
+        {"push_down_feed_large", "true"},
+        {"feed_active_targeting", "active"}};
+
+const FeatureEntry::FeatureParam
+    kFeedPositionAndroid_push_down_feed_large_target_non_feed_active[] = {
+        {"push_down_feed_large", "true"},
+        {"feed_active_targeting", "non-active"}};
+
+const FeatureEntry::FeatureParam
+    kFeedPositionAndroid_pull_up_feed_target_feed_active[] = {
+        {"pull_up_feed", "true"},
+        {"feed_active_targeting", "active"}};
+
+const FeatureEntry::FeatureParam
+    kFeedPositionAndroid_pull_up_feed_target_non_feed_active[] = {
+        {"pull_up_feed", "true"},
+        {"feed_active_targeting", "non-active"}};
+
 const FeatureEntry::FeatureVariation kFeedPositionAndroidVariations[] = {
     {"Push down Feed (small)", kFeedPositionAndroid_push_down_feed_small,
      std::size(kFeedPositionAndroid_push_down_feed_small), nullptr},
@@ -2162,6 +2182,23 @@
      std::size(kFeedPositionAndroid_push_down_feed_large), nullptr},
     {"Pull up Feed", kFeedPositionAndroid_pull_up_feed,
      std::size(kFeedPositionAndroid_pull_up_feed), nullptr},
+    {"Push down Feed (large) with targeting Feed active users",
+     kFeedPositionAndroid_push_down_feed_large_target_feed_active,
+     std::size(kFeedPositionAndroid_push_down_feed_large_target_feed_active),
+     nullptr},
+    {"Push down Feed (large) with targeting non-Feed active users",
+     kFeedPositionAndroid_push_down_feed_large_target_non_feed_active,
+     std::size(
+         kFeedPositionAndroid_push_down_feed_large_target_non_feed_active),
+     nullptr},
+    {"Pull up Feed with targeting Feed active users",
+     kFeedPositionAndroid_pull_up_feed_target_feed_active,
+     std::size(kFeedPositionAndroid_pull_up_feed_target_feed_active), nullptr},
+    {"Pull up Feed with targeting non-Feed active users",
+     kFeedPositionAndroid_pull_up_feed_target_non_feed_active,
+     std::size(kFeedPositionAndroid_pull_up_feed_target_non_feed_active),
+     nullptr},
+
 };
 
 const FeatureEntry::FeatureParam kFeatureNotificationGuide_low_engaged[] = {
diff --git a/tools/metrics/histograms/enums.xml b/tools/metrics/histograms/enums.xml
index 1d6c0d4..693f12f 100644
--- a/tools/metrics/histograms/enums.xml
+++ b/tools/metrics/histograms/enums.xml
@@ -40701,6 +40701,12 @@
              X button to close the notice."/>
 </enum>
 
+<enum name="FeedPositionSegmentationResult">
+  <int value="0" label="Uninitialized."/>
+  <int value="1" label="Is a Feed active user."/>
+  <int value="2" label="Is a non-Feed user."/>
+</enum>
+
 <enum name="FeedRequestReason">
   <int value="0" label="UNKNOWN"/>
   <int value="1" label="ZERO_STATE"/>
diff --git a/tools/metrics/histograms/metadata/new_tab_page/histograms.xml b/tools/metrics/histograms/metadata/new_tab_page/histograms.xml
index f185ce6..8e3c421 100644
--- a/tools/metrics/histograms/metadata/new_tab_page/histograms.xml
+++ b/tools/metrics/histograms/metadata/new_tab_page/histograms.xml
@@ -855,6 +855,18 @@
   </summary>
 </histogram>
 
+<histogram name="NewTabPage.FeedPositionSegmentationResult"
+    enum="FeedPositionSegmentationResult" expires_after="2022-09-24">
+  <owner>hanxi@chromium.org</owner>
+  <owner>ssid@chromium.org</owner>
+  <owner>spdonghao@chromium.org</owner>
+  <summary>
+    Logs the result from segmentation platform that determines whether the user
+    is a Feed active user or a non-Feed user. Recorded three times when a new
+    tab page is shown and Feed position (target) experiment is enabled.
+  </summary>
+</histogram>
+
 <histogram name="NewTabPage.HasCredentials" enum="BooleanYesNo"
     expires_after="2022-11-20">
   <owner>danpeng@google.com</owner>