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

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

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 package org.chromium.chrome.browser.preferences;
 
 import android.content.Context;
 import android.content.SharedPreferences;
 import android.preference.PreferenceManager;
 import android.util.Log;
@@ skipping 712 matching lines @@
     }
 
     @CalledByNative
     private void browsingDataCleared() {
         if (mClearBrowsingDataListener != null) {
             mClearBrowsingDataListener.onBrowsingDataCleared();
             mClearBrowsingDataListener = null;
         }
     }
 
+    /**
+     * 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);
     }
 
     public void setBlockThirdPartyCookiesEnabled(boolean enabled) {
         nativeSetBlockThirdPartyCookiesEnabled(enabled);
     }
 
     public void setDoNotTrackEnabled(boolean enabled) {
         nativeSetDoNotTrackEnabled(enabled);
@@ skipping 271 matching lines @@
     private native boolean nativeGetSupervisedUserSafeSitesEnabled();
     private native void nativeSetTranslateEnabled(boolean enabled);
     private native void nativeSetAutoDetectEncodingEnabled(boolean enabled);
     private native void nativeResetTranslateDefaults();
     private native void nativeMigrateJavascriptPreference();
     private native boolean nativeGetBrowsingDataDeletionPreference(int dataType);
     private native void nativeSetBrowsingDataDeletionPreference(int dataType, boolean value);
     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);
     private native void nativeSetBlockThirdPartyCookiesEnabled(boolean enabled);
     private native void nativeSetDoNotTrackEnabled(boolean enabled);
     private native void nativeSetFullscreenAllowed(boolean allowed);
     private native void nativeSetRememberPasswordsEnabled(boolean allow);
     private native void nativeSetPasswordManagerAutoSigninEnabled(boolean enabled);
     private native void nativeSetProtectedMediaIdentifierEnabled(boolean enabled);
     private native boolean nativeGetAllowLocationEnabled();
@@ skipping 34 matching lines @@
     private native String nativeGetSupervisedUserSecondCustodianName();
     private native String nativeGetSupervisedUserSecondCustodianEmail();
     private native String nativeGetSupervisedUserSecondCustodianProfileImageURL();
     private native boolean nativeGetMetricsReportingEnabled();
     private native void nativeSetMetricsReportingEnabled(boolean enabled);
     private native boolean nativeHasSetMetricsReporting();
     private native void nativeSetClickedUpdateMenuItem(boolean clicked);
     private native boolean nativeGetClickedUpdateMenuItem();
     private native void nativeSetSupervisedUserId(String supervisedUserId);
 }