[Feed] fix missing UMA counts for suggestion open

counts open suggestions from menu to NewTabPage.ContentSuggestions.Opened. Bug: 911835 Change-Id: I6035cbcab72c1e7acba1ff92da06804e56b21e1f Reviewed-on: https://chromium-review.googlesource.com/c/1362210 Reviewed-by: Sky Malice <[email protected]> Commit-Queue: Gang Wu <[email protected]> Cr-Original-Commit-Position: refs/heads/master@{#613890}(cherry picked from commit 99a5cb6) Reviewed-on: https://chromium-review.googlesource.com/c/1365838 Reviewed-by: Gang Wu <[email protected]> Cr-Commit-Position: refs/branch-heads/3626@{#111} Cr-Branched-From: d897fb1-refs/heads/master@{#612437}
nwjs · Dec 6, 2018 · db4d990 · db4d990
1 parent 806042f
commit db4d990
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 35 deletions.
diff --git a/chrome/android/feed/core/java/src/org/chromium/chrome/browser/feed/FeedLoggingBridge.java b/chrome/android/feed/core/java/src/org/chromium/chrome/browser/feed/FeedLoggingBridge.java
@@ -79,12 +79,10 @@ public void onContentSwiped(ContentLoggingData data) {
 
     @Override
     public void onContentClicked(ContentLoggingData data) {
-        // 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());
+        // Records content's clicks in onClientAction. When a user clicks on content, Feed libraries
+        // will call both onClientAction and onContentClicked, and onClientAction will receive
+        // ActionType.OPEN_URL in this case. so to avoid double counts, we records content's clicks
+        // in onClientAction.
     }
 
     @Override
@@ -94,8 +92,9 @@ public void onClientAction(ContentLoggingData data, @ActionType int actionType)
         if (mNativeFeedLoggingBridge == 0) return;
 
         recordUserAction(actionType);
-        nativeOnClientAction(
-                mNativeFeedLoggingBridge, feedActionToWindowOpenDisposition(actionType));
+        nativeOnClientAction(mNativeFeedLoggingBridge,
+                feedActionToWindowOpenDisposition(actionType), data.getPositionInStream(),
+                TimeUnit.SECONDS.toMillis(data.getPublishedTimeSeconds()), data.getScore());
     }
 
     @Override
@@ -184,7 +183,7 @@ private int feedActionToWindowOpenDisposition(@ActionType int actionType) {
             case ActionType.OPEN_URL:
                 return WindowOpenDisposition.CURRENT_TAB;
             case ActionType.OPEN_URL_INCOGNITO:
-                return WindowOpenDisposition.IGNORE_ACTION;
+                return WindowOpenDisposition.OFF_THE_RECORD;
             case ActionType.OPEN_URL_NEW_TAB:
                 return WindowOpenDisposition.NEW_BACKGROUND_TAB;
             case ActionType.OPEN_URL_NEW_WINDOW:
@@ -256,10 +255,8 @@ private native void nativeOnContentViewed(long nativeFeedLoggingBridge, int posi
     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(
-            long nativeFeedLoggingBridge, int windowOpenDisposition);
+    private native void nativeOnClientAction(long nativeFeedLoggingBridge,
+            int windowOpenDisposition, int position, long publishedTimeMs, float score);
     private native void nativeOnContentContextMenuOpened(
             long nativeFeedLoggingBridge, int position, long publishedTimeMs, float score);
     private native void nativeOnMoreButtonViewed(long nativeFeedLoggingBridge, int position);

diff --git a/chrome/browser/android/feed/feed_logging_bridge.cc b/chrome/browser/android/feed/feed_logging_bridge.cc
@@ -48,12 +48,12 @@ void FeedLoggingBridge::OnContentViewed(
     JNIEnv* j_env,
     const base::android::JavaRef<jobject>& j_this,
     const jint j_position,
-    const jlong j_publishedTimeSeconds,
-    const jlong j_timeContentBecameAvailableSeconds,
+    const jlong j_publishedTimeMs,
+    const jlong j_timeContentBecameAvailableMs,
     const jfloat j_score) {
   feed_logging_metrics_->OnSuggestionShown(
-      j_position, base::Time::FromJavaTime(j_publishedTimeSeconds), j_score,
-      base::Time::FromJavaTime(j_timeContentBecameAvailableSeconds));
+      j_position, base::Time::FromJavaTime(j_publishedTimeMs), j_score,
+      base::Time::FromJavaTime(j_timeContentBecameAvailableMs));
 }
 
 void FeedLoggingBridge::OnContentDismissed(
@@ -71,20 +71,15 @@ void FeedLoggingBridge::OnContentSwiped(
   feed_logging_metrics_->OnSuggestionSwiped();
 }
 
-void FeedLoggingBridge::OnContentClicked(
+void FeedLoggingBridge::OnClientAction(
     JNIEnv* j_env,
     const base::android::JavaRef<jobject>& j_this,
+    const jint j_window_open_disposition,
     const jint j_position,
-    const jlong j_publishedTimeSeconds,
+    const jlong j_publishedTimeMs,
     const jfloat j_score) {
   feed_logging_metrics_->OnSuggestionOpened(
-      j_position, base::Time::FromJavaTime(j_publishedTimeSeconds), j_score);
-}
-
-void FeedLoggingBridge::OnClientAction(
-    JNIEnv* j_env,
-    const base::android::JavaRef<jobject>& j_this,
-    const jint j_window_open_disposition) {
+      j_position, base::Time::FromJavaTime(j_publishedTimeMs), j_score);
   feed_logging_metrics_->OnSuggestionWindowOpened(
       static_cast<WindowOpenDisposition>(j_window_open_disposition));
 }
@@ -93,10 +88,10 @@ void FeedLoggingBridge::OnContentContextMenuOpened(
     JNIEnv* j_env,
     const base::android::JavaRef<jobject>& j_this,
     const jint j_position,
-    const jlong j_publishedTimeSeconds,
+    const jlong j_publishedTimeMs,
     const jfloat j_score) {
   feed_logging_metrics_->OnSuggestionMenuOpened(
-      j_position, base::Time::FromJavaTime(j_publishedTimeSeconds), j_score);
+      j_position, base::Time::FromJavaTime(j_publishedTimeMs), j_score);
 }
 
 void FeedLoggingBridge::OnMoreButtonViewed(JNIEnv* j_env,

diff --git a/chrome/browser/android/feed/feed_logging_bridge.h b/chrome/browser/android/feed/feed_logging_bridge.h
@@ -38,15 +38,12 @@ class FeedLoggingBridge {
   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,
-                        const jlong j_publishedTimeMs,
-                        const jfloat j_score);
-
   void OnClientAction(JNIEnv* j_env,
                       const base::android::JavaRef<jobject>& j_this,
-                      const jint j_window_open_disposition);
+                      const jint j_window_open_disposition,
+                      const jint j_position,
+                      const jlong j_publishedTimeMs,
+                      const jfloat j_score);
 
   void OnContentContextMenuOpened(JNIEnv* j_env,
                                   const base::android::JavaRef<jobject>& j_this,