Fix the notice card uma metric

Bug: 1141737
Change-Id: I01750af13a9fd4b58863957a98a0dabaf9a68230
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2493708Reviewed-by: Dan H <harringtond@chromium.org>
Commit-Queue: Vincent Boisselle <vincb@google.com>
Cr-Commit-Position: refs/heads/master@{#820270}

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

@@ -302,9 +302,9 @@ public class FeedRequestManagerImpl implements FeedRequestManager {
     private void logServerCapabilities(Response response, boolean isRefreshRequest) {
         FeedResponse feedResponse = response.getExtension(FeedResponse.feedResponse);
         List<Capability> capabilities = feedResponse.getServerCapabilitiesList();
-        RecordHistogram.recordBooleanHistogram("ContentSuggestions.Feed.NoticeCardFulfilled",
-                capabilities.contains(Capability.REPORT_FEED_USER_ACTIONS_NOTICE_CARD));
         if (isRefreshRequest) {
+            RecordHistogram.recordBooleanHistogram("ContentSuggestions.Feed.NoticeCardFulfilled",
+                    capabilities.contains(Capability.REPORT_FEED_USER_ACTIONS_NOTICE_CARD));
             mMainThreadRunner.execute("Update notice card pref",
                     ()
                             -> updateNoticeCardPref(capabilities.contains(

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

@@ -33,7 +33,9 @@ import org.robolectric.Robolectric;
 import org.robolectric.annotation.Config;
 import org.robolectric.util.ReflectionHelpers;
 
+import org.chromium.base.metrics.test.ShadowRecordHistogram;
 import org.chromium.base.test.util.JniMocker;
+import org.chromium.base.test.util.MetricsUtils;
 import org.chromium.chrome.browser.feed.library.api.host.config.ApplicationInfo;
 import org.chromium.chrome.browser.feed.library.api.host.config.Configuration;
 import org.chromium.chrome.browser.feed.library.api.host.config.Configuration.ConfigKey;
@@ -106,7 +108,7 @@ import java.util.Set;
 
 /** Test of the {@link FeedRequestManagerImpl} class. */
 @RunWith(LocalRobolectricTestRunner.class)
-@Config(manifest = Config.NONE)
+@Config(manifest = Config.NONE, shadows = {ShadowRecordHistogram.class})
 @Features.EnableFeatures(ChromeFeatureList.INTEREST_FEED_CONTENT_SUGGESTIONS)
 @Features.
 DisableFeatures({ChromeFeatureList.REPORT_FEED_USER_ACTIONS, ChromeFeatureList.INTEREST_FEED_V2})
@@ -243,7 +245,12 @@ public class FeedRequestManagerImplTest {
     }
 
     @Test
-    public void testTriggerRefresh_setNoticeCardPref() throws Exception {
+    public void testTriggerRefresh_setNoticeCardPrefAndRecordHistogram() throws Exception {
+        MetricsUtils.HistogramDelta noticeCardNotFulfilledDelta = new MetricsUtils.HistogramDelta(
+                "ContentSuggestions.Feed.NoticeCardFulfilled", 0 /*false*/);
+        MetricsUtils.HistogramDelta noticeCardFulfilledDelta = new MetricsUtils.HistogramDelta(
+                "ContentSuggestions.Feed.NoticeCardFulfilled", 1 /*true*/);
+
         // Skip the read of the int that determines the length of the encoded proto. This is to
         // avoid having to encode the length which is a feature we don't want to test here.
         Configuration configuration =
@@ -257,6 +264,7 @@ public class FeedRequestManagerImplTest {
                 mApplicationInfo, mFakeMainThreadRunner, mFakeBasicLoggingApi,
                 mFakeTooltipSupportedApi);
 
+        // Trigger a refresh that has a notice card.
         Response response =
                 Response.newBuilder()
                         .setExtension(FeedResponse.feedResponse,
@@ -267,8 +275,16 @@ public class FeedRequestManagerImplTest {
                         .build();
         mFakeNetworkClient.addResponse(new HttpResponse(200, response.toByteArray(), false));
         mRequestManager.triggerRefresh(RequestReason.HOST_REQUESTED, input -> {});
-
         verify(mPrefService, times(1)).setBoolean(Pref.LAST_FETCH_HAD_NOTICE_CARD, true);
+        assertThat(noticeCardNotFulfilledDelta.getDelta()).isEqualTo(0);
+        assertThat(noticeCardFulfilledDelta.getDelta()).isEqualTo(1);
+
+        // Trigger a refresh that doesn't have a notice card.
+        mFakeNetworkClient.addResponse(
+                new HttpResponse(200, Response.getDefaultInstance().toByteArray(), false));
+        mRequestManager.triggerRefresh(RequestReason.HOST_REQUESTED, input -> {});
+        assertThat(noticeCardNotFulfilledDelta.getDelta()).isEqualTo(1);
+        assertThat(noticeCardFulfilledDelta.getDelta()).isEqualTo(1);
     }
 
     @Test
@@ -294,7 +310,12 @@ public class FeedRequestManagerImplTest {
     }
 
     @Test
-    public void testLoadMore_dontSetNoticeCardPref() throws Exception {
+    public void testLoadMore_dontSetNoticeCardPrefAndDontRecordHistogram() throws Exception {
+        MetricsUtils.HistogramDelta noticeCardNotFulfilledDelta = new MetricsUtils.HistogramDelta(
+                "ContentSuggestions.Feed.NoticeCardFulfilled", 0 /*false*/);
+        MetricsUtils.HistogramDelta noticeCardFulfilledDelta = new MetricsUtils.HistogramDelta(
+                "ContentSuggestions.Feed.NoticeCardFulfilled", 1 /*true*/);
+
         // Skip the read of the int that determines the length of the encoded proto. This is to
         // avoid having to encode the length which is a feature we don't want to test here.
         Configuration configuration =
@@ -318,6 +339,8 @@ public class FeedRequestManagerImplTest {
         mRequestManager.loadMore(token, ConsistencyToken.getDefaultInstance(), input -> {});
 
         verify(mPrefService, never()).setBoolean(Pref.LAST_FETCH_HAD_NOTICE_CARD, true);
+        assertThat(noticeCardNotFulfilledDelta.getDelta()).isEqualTo(0);
+        assertThat(noticeCardFulfilledDelta.getDelta()).isEqualTo(0);
     }
 
     @Test