[Feed] Add missing user actions

User Actions:
Suggestions.ScrolledAfterOpen
Suggestions.Card.SwipedAway

UMA:
ContentSuggestions.FetchPendingSpinner.VisibleDuration

Also add Guard against calls from Feed after destroy(), like
https://chromium-review.googlesource.com/c/chromium/src/+/1345422

Bug: 908706
Change-Id: Ifdd9dcab594d74eb79d0161d1ee66027a7adcea0
Reviewed-on: https://chromium-review.googlesource.com/c/1351932
Commit-Queue: Gang Wu <gangwu@chromium.org>
Reviewed-by: Sky Malice <skym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612306}

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

@@ -4,6 +4,9 @@
 
 package org.chromium.chrome.browser.feed;
 
+import android.support.annotation.NonNull;
+
+import com.google.android.libraries.feed.api.stream.ScrollListener;
 import com.google.android.libraries.feed.host.logging.ActionType;
 import com.google.android.libraries.feed.host.logging.BasicLoggingApi;
 import com.google.android.libraries.feed.host.logging.ContentLoggingData;
@@ -36,14 +39,20 @@ public class FeedLoggingBridge implements BasicLoggingApi {
 
     /** Cleans up native half of this bridge. */
     public void destroy() {
-        assert mNativeFeedLoggingBridge != 0;
+        // Bridge could have been destroyed for policy when this is called.
+        // See https://crbug.com/901414.
+        if (mNativeFeedLoggingBridge == 0) return;
+
         nativeDestroy(mNativeFeedLoggingBridge);
         mNativeFeedLoggingBridge = 0;
     }
 
     @Override
     public void onContentViewed(ContentLoggingData data) {
-        assert mNativeFeedLoggingBridge != 0;
+        // Bridge could have been destroyed for policy when this is called.
+        // See https://crbug.com/901414.
+        if (mNativeFeedLoggingBridge == 0) return;
+
         nativeOnContentViewed(mNativeFeedLoggingBridge, data.getPositionInStream(),
                 TimeUnit.SECONDS.toMillis(data.getPublishedTimeSeconds()),
                 TimeUnit.SECONDS.toMillis(data.getTimeContentBecameAvailable()), data.getScore());
@@ -51,24 +60,39 @@ public class FeedLoggingBridge implements BasicLoggingApi {
 
     @Override
     public void onContentDismissed(ContentLoggingData data) {
-        assert mNativeFeedLoggingBridge != 0;
+        // Bridge could have been destroyed for policy when this is called.
+        // See https://crbug.com/901414.
+        if (mNativeFeedLoggingBridge == 0) return;
+
         nativeOnContentDismissed(
                 mNativeFeedLoggingBridge, data.getPositionInStream(), data.getRepresentationUri());
     }
 
     @Override
-    public void onContentSwiped(ContentLoggingData data) {}
+    public void onContentSwiped(ContentLoggingData data) {
+        // Bridge could have been destroyed for policy when this is called.
+        // See https://crbug.com/901414.
+        if (mNativeFeedLoggingBridge == 0) return;
+
+        nativeOnContentSwiped(mNativeFeedLoggingBridge);
+    }
 
     @Override
     public void onContentClicked(ContentLoggingData data) {
-        assert mNativeFeedLoggingBridge != 0;
+        // Bridge could have been destroyed for policy when this is called.
+        // See https://crbug.com/901414.
+        if (mNativeFeedLoggingBridge == 0) return;
+
         nativeOnContentClicked(mNativeFeedLoggingBridge, data.getPositionInStream(),
                 TimeUnit.SECONDS.toMillis(data.getPublishedTimeSeconds()), data.getScore());
     }
 
     @Override
     public void onClientAction(ContentLoggingData data, @ActionType int actionType) {
-        assert mNativeFeedLoggingBridge != 0;
+        // Bridge could have been destroyed for policy when this is called.
+        // See https://crbug.com/901414.
+        if (mNativeFeedLoggingBridge == 0) return;
+
         recordUserAction(actionType);
         nativeOnClientAction(
                 mNativeFeedLoggingBridge, feedActionToWindowOpenDisposition(actionType));
@@ -76,43 +100,67 @@ public class FeedLoggingBridge implements BasicLoggingApi {
 
     @Override
     public void onContentContextMenuOpened(ContentLoggingData data) {
-        assert mNativeFeedLoggingBridge != 0;
+        // Bridge could have been destroyed for policy when this is called.
+        // See https://crbug.com/901414.
+        if (mNativeFeedLoggingBridge == 0) return;
+
         nativeOnContentContextMenuOpened(mNativeFeedLoggingBridge, data.getPositionInStream(),
                 TimeUnit.SECONDS.toMillis(data.getPublishedTimeSeconds()), data.getScore());
     }
 
     @Override
     public void onMoreButtonViewed(int position) {
-        assert mNativeFeedLoggingBridge != 0;
+        // Bridge could have been destroyed for policy when this is called.
+        // See https://crbug.com/901414.
+        if (mNativeFeedLoggingBridge == 0) return;
+
         nativeOnMoreButtonViewed(mNativeFeedLoggingBridge, position);
     }
 
     @Override
     public void onMoreButtonClicked(int position) {
-        assert mNativeFeedLoggingBridge != 0;
+        // Bridge could have been destroyed for policy when this is called.
+        // See https://crbug.com/901414.
+        if (mNativeFeedLoggingBridge == 0) return;
+
         nativeOnMoreButtonClicked(mNativeFeedLoggingBridge, position);
     }
 
     @Override
     public void onOpenedWithContent(int timeToPopulateMs, int contentCount) {
-        assert mNativeFeedLoggingBridge != 0;
+        // Bridge could have been destroyed for policy when this is called.
+        // See https://crbug.com/901414.
+        if (mNativeFeedLoggingBridge == 0) return;
+
         nativeOnOpenedWithContent(mNativeFeedLoggingBridge, timeToPopulateMs, contentCount);
     }
 
     @Override
     public void onOpenedWithNoImmediateContent() {
-        assert mNativeFeedLoggingBridge != 0;
+        // Bridge could have been destroyed for policy when this is called.
+        // See https://crbug.com/901414.
+        if (mNativeFeedLoggingBridge == 0) return;
+
         nativeOnOpenedWithNoImmediateContent(mNativeFeedLoggingBridge);
     }
 
     @Override
     public void onOpenedWithNoContent() {
-        assert mNativeFeedLoggingBridge != 0;
+        // Bridge could have been destroyed for policy when this is called.
+        // See https://crbug.com/901414.
+        if (mNativeFeedLoggingBridge == 0) return;
+
         nativeOnOpenedWithNoContent(mNativeFeedLoggingBridge);
     }
 
     @Override
-    public void onSpinnerShown(int timeShownMs, @SpinnerType int spinnerType) {}
+    public void onSpinnerShown(int timeShownMs, @SpinnerType int spinnerType) {
+        // Bridge could have been destroyed for policy when this is called.
+        // See https://crbug.com/901414.
+        if (mNativeFeedLoggingBridge == 0) return;
+
+        nativeOnSpinnerShown(mNativeFeedLoggingBridge, timeShownMs);
+    }
 
     /**
      * Reports how long a user spends on the page.
@@ -168,12 +216,46 @@ public class FeedLoggingBridge implements BasicLoggingApi {
         }
     }
 
+    private void reportScrolledAfterOpen() {
+        // Bridge could have been destroyed for policy when this is called.
+        // See https://crbug.com/901414.
+        if (mNativeFeedLoggingBridge == 0) return;
+
+        nativeReportScrolledAfterOpen(mNativeFeedLoggingBridge);
+    }
+
+    /**
+     * One-shot reporter that records the first time the user scrolls in the {@link Stream}.
+     */
+    public static class ScrollEventReporter implements ScrollListener {
+        private final FeedLoggingBridge mLoggingBridge;
+        private boolean mFired;
+
+        public ScrollEventReporter(@NonNull FeedLoggingBridge loggingBridge) {
+            super();
+            mLoggingBridge = loggingBridge;
+        }
+
+        @Override
+        public void onScrollStateChanged(@ScrollState int state) {
+            if (mFired) return;
+            if (state != ScrollState.DRAGGING) return;
+
+            mLoggingBridge.reportScrolledAfterOpen();
+            mFired = true;
+        }
+
+        @Override
+        public void onScrolled(int dx, int dy) {}
+    }
+
     private native long nativeInit(Profile profile);
     private native void nativeDestroy(long nativeFeedLoggingBridge);
     private native void nativeOnContentViewed(long nativeFeedLoggingBridge, int position,
             long publishedTimeMs, long timeContentBecameAvailableMs, float score);
     private native void nativeOnContentDismissed(
             long nativeFeedLoggingBridge, int position, String uri);
+    private native void nativeOnContentSwiped(long nativeFeedLoggingBridge);
     private native void nativeOnContentClicked(
             long nativeFeedLoggingBridge, int position, long publishedTimeMs, float score);
     private native void nativeOnClientAction(
@@ -186,6 +268,8 @@ public class FeedLoggingBridge implements BasicLoggingApi {
             long nativeFeedLoggingBridge, int timeToPopulateMs, int contentCount);
     private native void nativeOnOpenedWithNoImmediateContent(long nativeFeedLoggingBridge);
     private native void nativeOnOpenedWithNoContent(long nativeFeedLoggingBridge);
+    private native void nativeOnSpinnerShown(long nativeFeedLoggingBridge, long spinnerShownTimeMs);
     private native void nativeOnContentTargetVisited(
             long nativeFeedLoggingBridge, long visitTimeMs, boolean isOffline, boolean returnToNtp);
+    private native void nativeReportScrolledAfterOpen(long nativeFeedLoggingBridge);
 }

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

@@ -350,6 +350,7 @@ public class FeedNewTabPage extends NewTabPage {
         if (mSigninPromoView != null) UiUtils.removeViewFromParent(mSigninPromoView);
         mStream.setHeaderViews(Arrays.asList(new NonDismissibleHeader(mNewTabPageLayout),
                 new NonDismissibleHeader(mSectionHeaderView)));
+        mStream.addScrollListener(new FeedLoggingBridge.ScrollEventReporter(loggingBridge));
         // Explicitly request focus on the scroll container to avoid UrlBar being focused after
         // the scroll container for policy is removed.
         view.requestFocus();

chrome/browser/android/feed/feed_logging_bridge.cc

@@ -65,6 +65,12 @@ void FeedLoggingBridge::OnContentDismissed(
       j_position, GURL(ConvertJavaStringToUTF8(j_env, j_url)));
 }
 
+void FeedLoggingBridge::OnContentSwiped(
+    JNIEnv* j_env,
+    const base::android::JavaRef<jobject>& j_this) {
+  feed_logging_metrics_->OnSuggestionSwiped();
+}
+
 void FeedLoggingBridge::OnContentClicked(
     JNIEnv* j_env,
     const base::android::JavaRef<jobject>& j_this,
@@ -120,6 +126,14 @@ void FeedLoggingBridge::OnOpenedWithNoContent(
     JNIEnv* j_env,
     const base::android::JavaRef<jobject>& j_this) {}
 
+void FeedLoggingBridge::OnSpinnerShown(
+    JNIEnv* j_env,
+    const base::android::JavaRef<jobject>& j_this,
+    const jlong j_shownTimeMs) {
+  feed_logging_metrics_->OnSpinnerShown(
+      base::TimeDelta::FromMilliseconds(j_shownTimeMs));
+}
+
 void FeedLoggingBridge::OnContentTargetVisited(
     JNIEnv* j_env,
     const base::android::JavaRef<jobject>& j_this,
@@ -135,4 +149,10 @@ void FeedLoggingBridge::OnContentTargetVisited(
   }
 }
 
+void FeedLoggingBridge::ReportScrolledAfterOpen(
+    JNIEnv* j_env,
+    const base::android::JavaRef<jobject>& j_this) {
+  feed_logging_metrics_->ReportScrolledAfterOpen();
+}
+
 }  // namespace feed

chrome/browser/android/feed/feed_logging_bridge.h

@@ -35,6 +35,9 @@ class FeedLoggingBridge {
                           const jint j_position,
                           const base::android::JavaRef<jstring>& j_url);
 
+  void OnContentSwiped(JNIEnv* j_env,
+                       const base::android::JavaRef<jobject>& j_this);
+
   void OnContentClicked(JNIEnv* j_env,
                         const base::android::JavaRef<jobject>& j_this,
                         const jint j_position,
@@ -71,12 +74,19 @@ class FeedLoggingBridge {
   void OnOpenedWithNoContent(JNIEnv* j_env,
                              const base::android::JavaRef<jobject>& j_this);
 
+  void OnSpinnerShown(JNIEnv* j_env,
+                      const base::android::JavaRef<jobject>& j_this,
+                      const jlong j_shownTimeMs);
+
   void OnContentTargetVisited(JNIEnv* j_env,
                               const base::android::JavaRef<jobject>& j_this,
                               const jlong visit_time_ms,
                               const jboolean is_offline,
                               const jboolean return_to_ntp);
 
+  void ReportScrolledAfterOpen(JNIEnv* j_env,
+                               const base::android::JavaRef<jobject>& j_this);
+
  private:
   FeedLoggingMetrics* feed_logging_metrics_;
 

components/feed/core/feed_logging_metrics.cc

@@ -181,6 +181,10 @@ void FeedLoggingMetrics::OnSuggestionDismissed(int position, const GURL& url) {
   base::RecordAction(base::UserMetricsAction("Suggestions.Content.Dismissed"));
 }
 
+void FeedLoggingMetrics::OnSuggestionSwiped() {
+  base::RecordAction(base::UserMetricsAction("Suggestions.Card.SwipedAway"));
+}
+
 void FeedLoggingMetrics::OnSuggestionArticleVisited(base::TimeDelta visit_time,
                                                     bool return_to_ntp) {
   base::UmaHistogramLongTimes(
@@ -212,6 +216,15 @@ void FeedLoggingMetrics::OnMoreButtonClicked(int position) {
       kMaxSuggestionsForArticle + 1);
 }
 
+void FeedLoggingMetrics::OnSpinnerShown(base::TimeDelta shown_time) {
+  base::UmaHistogramLongTimes(
+      "ContentSuggestions.FetchPendingSpinner.VisibleDuration", shown_time);
+}
+
+void FeedLoggingMetrics::ReportScrolledAfterOpen() {
+  base::RecordAction(base::UserMetricsAction("Suggestions.ScrolledAfterOpen"));
+}
+
 void FeedLoggingMetrics::CheckURLVisitedDone(int position, bool visited) {
   if (visited) {
     UMA_HISTOGRAM_EXACT_LINEAR("NewTabPage.ContentSuggestions.DismissedVisited",

components/feed/core/feed_logging_metrics.h

@@ -58,6 +58,8 @@ class FeedLoggingMetrics {
 
   void OnSuggestionDismissed(int position, const GURL& url);
 
+  void OnSuggestionSwiped();
+
   void OnSuggestionArticleVisited(base::TimeDelta visit_time,
                                   bool return_to_ntp);
 
@@ -69,6 +71,10 @@ class FeedLoggingMetrics {
 
   void OnMoreButtonClicked(int position);
 
+  void OnSpinnerShown(base::TimeDelta shown_time);
+
+  void ReportScrolledAfterOpen();
+
  private:
   void CheckURLVisitedDone(int position, bool visited);