Connect the "other forms of browsing history" frontend on Android to backend.

chrome/android/java/src/org/chromium/chrome/browser/preferences/PrefServiceBridge.java

Index: chrome/android/java/src/org/chromium/chrome/browser/preferences/PrefServiceBridge.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/preferences/PrefServiceBridge.java b/chrome/android/java/src/org/chromium/chrome/browser/preferences/PrefServiceBridge.java
index 381b475c7c400a90417dee405a9d81ab8f441937..85d5ee165f23e93c3f0c6caea6a765bb0ca52e07 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/preferences/PrefServiceBridge.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/preferences/PrefServiceBridge.java
@@ -730,6 +730,55 @@ public final class PrefServiceBridge {
         }
     }
 
+    /**
+     * Interface to a class that receives callbacks instructing it to inform the user about other
+     * forms of browsing history.
+     */
+    public interface OtherFormsOfBrowsingHistoryListener {

[gone, 2016/04/07 22:27:55]

public inner classes go at the top of the class.  also add javadocs for the
functions.

[msramek, 2016/04/08 16:03:17]

Done. I also moved the existing OnClearBrowsingDataListener up - that is
actually what I took as an example when I put this class lower.

+        public abstract void enableDialogAboutOtherFormsOfBrowsingHistory();
+        public abstract void showNoticeAboutOtherFormsOfBrowsingHistory();
+    }
+
+    private OtherFormsOfBrowsingHistoryListener mShowNoticeListener;
+    private OtherFormsOfBrowsingHistoryListener mEnableDialogListener;

[gone, 2016/04/07 22:27:55]

Same comment as the other CL: don't randomly put member fields in the middle of
a class.  The class is admittedly already a bit messy, but these should go
beneath:

private static PrefServiceBridge sInstance;

[msramek, 2016/04/08 16:03:18]

Acknowledged. This was removed, as per the other comment.

+
+    @CalledByNative
+    private void showNoticeAboutOtherFormsOfBrowsingHistory(boolean show) {
+        if (mShowNoticeListener != null) {
+            if (show) {
+                mShowNoticeListener.showNoticeAboutOtherFormsOfBrowsingHistory();
+            }
+
+            // Null the listener so that ClearBrowsingDataPreferences can be garbage-collected.

[gone, 2016/04/07 22:27:55]

This would happen regardless, so the comment isn't useful.

[msramek, 2016/04/08 16:03:18]

Acknowledged. This part is now removed.

+            mShowNoticeListener = null;
+        }
+    }
+
+    @CalledByNative
+    private void enableDialogAboutOtherFormsOfBrowsingHistory(boolean enable) {

[gone, 2016/04/07 22:27:55]

I don't understand why you grouped those functions together in your interface,
then kept two separate references to the same class; it just clutters up the
logic.  You may as well just make them Runnables:

private Runnable mShowNoticeRunnable;
private Runnable mEnableDialogRunnable;

@CalledByNative
private void showNoticeAboutOtherFormsOfBrowsingHistory(boolean show) {
    if (mShowNoticeRunnable != null && show) mShowNoticeRunnable.run();
    mShowNoticeRunnable = null;
}

@CalledByNative
private void enableDialogAboutOtherFormsOfBrowsingHistory(boolean enable) {
    if (mEnableDialogRunnable != null && enable) mEnableDialogRunnable.run();
    mEnableDialogRunnable = null;
}

[msramek, 2016/04/08 16:03:18]

I kept two separate references, because I don't know which callback will return
first; I need to keep the reference until both callbacks fire, and then to null
it, so that PrefServiceBridge doesn't unnecesarily hold
ClearBrowsingDataPreferences.

Nevertheless, I'll pass the callback to the native side, as you suggested in the
other comment.

+        if (mEnableDialogListener != null) {
+            if (enable) {
+                mEnableDialogListener.enableDialogAboutOtherFormsOfBrowsingHistory();
+            }
+
+            // Null the listener so that ClearBrowsingDataPreferences can be garbage-collected.
+            mEnableDialogListener = null;
+        }
+    }
+
+    /**
+     * Requests that the web history service finds out if we should inform the user about the
+     * existence of other forms of browsing history. The response will be asynchronous, through
+     * {@link PrefServiceBridge#enableDialogAboutOtherFormsOfBrowsingHistory} and
+     * {@link PrefServiceBridge#showNoticeAboutOtherFormsOfBrowsingHistory}.
+     */
+    public void requestInfoAboutOtherFormsOfBrowsingHistory(
+            OtherFormsOfBrowsingHistoryListener listener) {
+        mShowNoticeListener = listener;
+        mEnableDialogListener = listener;
+        nativeRequestInfoAboutOtherFormsOfBrowsingHistory();

[gone, 2016/04/07 22:27:55]

Instead of storing the listeners, you should pass the callback directly to the
native code and run the appropriate one when necessary.  Would avoid new member
fields, and would avoid any weirdness where the function was called twice with
two different OtherFormsOfBrowsingHistoryListeners.

[msramek, 2016/04/08 16:03:18]

Done. That was actually my first idea, but then I decided to follow the example
of browsingDataCleared() which stores the pointer here.

But yes, it turns out that implementation is even simpler.

+    }
+
     public void setAllowCookiesEnabled(boolean allow) {
         nativeSetAllowCookiesEnabled(allow);
     }
@@ -1021,6 +1070,7 @@ public final class PrefServiceBridge {
     private native int nativeGetBrowsingDataDeletionTimePeriod();
     private native void nativeSetBrowsingDataDeletionTimePeriod(int timePeriod);
     private native void nativeClearBrowsingData(int[] dataTypes, int timePeriod);
+    private native void nativeRequestInfoAboutOtherFormsOfBrowsingHistory();
     private native boolean nativeCanDeleteBrowsingHistory();
     private native void nativeSetAllowCookiesEnabled(boolean allow);
     private native void nativeSetBackgroundSyncEnabled(boolean allow);