Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(630)

Unified Diff: chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchRankerLoggerImpl.java

Issue 2857333002: [TTS] Write initial Tap-features to Ranker. (Closed)
Patch Set: Added logging of the CS-model URL and updated console-logging output. Created 3 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchRankerLoggerImpl.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchRankerLoggerImpl.java b/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchRankerLoggerImpl.java
index f56846a9e1520392f10e63580250c007eebfb28f..075671525b54919d783121707f637d064118a1b3 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchRankerLoggerImpl.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchRankerLoggerImpl.java
@@ -6,16 +6,81 @@ package org.chromium.chrome.browser.contextualsearch;
import org.chromium.base.Log;
+import java.net.URL;
+
/**
* Implements the UMA logging for Ranker that's used for Contextual Search Tap Suppression.
*/
public class ContextualSearchRankerLoggerImpl implements ContextualSearchRankerLogger {
private static final String TAG = "ContextualSearch";
+ // The current version of the Ranker model that we use for Contextual Search.
+ private static final int CONTEXTUAL_SEARCH_RANKER_MODEL_VERSION = 0;
+
+ // Pointer to the native instance of this class.
+ private long mNativePointer;
+
+ // The URL of the base page to log with Ranker.
+ private URL mBasePageUrl;
+
+ public ContextualSearchRankerLoggerImpl() {
+ if (isEnabled()) {
+ mNativePointer = nativeInit();
+ if (mBasePageUrl != null) {
Theresa 2017/05/12 01:02:25 Won't mBasePageUrl will always be null in the cons
Donn Denman 2017/05/12 23:08:55 You're right, we can remove this section. Done.
+ nativeSetUkmServiceAndUrls(
+ mNativePointer, mBasePageUrl.toString(), getContextualSearchRankerUrl());
+ }
+ }
+ }
+
+ /**
+ * This method should be called to clean up storage when an instance of this class is
+ * no longer in use. The nativeDestroy will call the destructor on the native instance.
+ */
+ void destroy() {
+ if (isEnabled()) {
+ assert mNativePointer != 0;
+ writeLogAndReset();
+ nativeDestroy(mNativePointer);
+ mNativePointer = 0;
+ }
+ }
+
+ @Override
+ public void setBasePageUrl(URL basePageUrl) {
+ if (isEnabled()) {
+ mBasePageUrl = basePageUrl;
+ if (mBasePageUrl != null) {
Donn Denman 2017/05/12 23:08:55 I noticed that this is null for URIs that are not
+ nativeSetUkmServiceAndUrls(
+ mNativePointer, mBasePageUrl.toString(), getContextualSearchRankerUrl());
+ }
+ }
+ }
+
@Override
public void log(Feature feature, Object value) {
- // TODO(donnd): log to an actual persistent proto ASAP!
- Log.v(TAG, "log %s with value %s", feature.toString(), value);
+ if (!isEnabled()) {
+ Log.v(TAG, "Ranker data %s: %s", feature, value);
+ return;
+ }
+
+ if (value instanceof String) {
+ nativeLogString(mNativePointer, feature.toString(), value.toString());
Roger McFarlane (Chromium) 2017/05/12 15:43:26 Do we have go ahead to do string logging like this
Donn Denman 2017/05/12 23:08:55 Good point! Removed all string logging from this
+ } else if (value instanceof Boolean) {
+ nativeLogLong(mNativePointer, feature.toString(), ((boolean) value ? 1 : 0));
+ } else if (value instanceof Integer) {
+ Integer i = (int) value;
+ nativeLogLong(mNativePointer, feature.toString(), Long.valueOf(i));
+ } else if (value instanceof Long) {
+ nativeLogLong(mNativePointer, feature.toString(), (long) value);
+ } else if (value instanceof Character) {
+ nativeLogLong(
+ mNativePointer, feature.toString(), Character.getNumericValue((char) value));
+ } else {
+ Log.w(TAG,
+ "Could not log feature to Ranker: " + feature.toString() + " of class "
+ + value.getClass());
+ }
}
@Override
@@ -25,6 +90,30 @@ public class ContextualSearchRankerLoggerImpl implements ContextualSearchRankerL
@Override
public void writeLogAndReset() {
- Log.v(TAG, "Reset!\n");
+ if (isEnabled()) nativeWriteLogAndReset(mNativePointer);
+ }
+
+ // Whether actually writing data is enabled. If not, we may do nothing or print.
+ private boolean isEnabled() {
+ return ContextualSearchFieldTrial.isRankerLoggingEnabled();
+ }
+
+ // Gets the URL to use for the current Ranker model.
+ private String getContextualSearchRankerUrl() {
+ return "org.chromium.chrome.browser.contextualsearch.v"
+ + CONTEXTUAL_SEARCH_RANKER_MODEL_VERSION;
}
+
+ // ============================================================================================
+ // Native methods.
+ // ============================================================================================
+ private native long nativeInit();
+ private native void nativeDestroy(long nativeContextualSearchRankerLoggerImpl);
+ private native void nativeLogLong(
+ long nativeContextualSearchRankerLoggerImpl, String featureString, long value);
+ private native void nativeLogString(
+ long nativeContextualSearchRankerLoggerImpl, String featureString, String value);
+ private native void nativeSetUkmServiceAndUrls(
+ long nativeContextualSearchRankerLoggerImpl, String basePageUrl, String modelUrl);
+ private native void nativeWriteLogAndReset(long nativeContextualSearchRankerLoggerImpl);
}

Powered by Google App Engine
This is Rietveld 408576698