[ImportantSites] JNI bindings for CBD filtering and Important Sites.

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 25953f931889d2c80aead61edeec3b117f2d01d1..6fd0f38dd2319a0552584b53f8d4b13781e0eef2 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
@@ -41,6 +41,8 @@ public final class PrefServiceBridge {
 
     // Object to notify when "clear browsing data" completes.
     private OnClearBrowsingDataListener mClearBrowsingDataListener;
+    // Object to notify when we finished fetching important sites.
+    private ImportantSitesCallback mImportantSitesCallback;
     private static final String LOG_TAG = "PrefServiceBridge";
 
     // Constants related to the Contextual Search preference.
@@ -80,6 +82,13 @@ public final class PrefServiceBridge {
     }
 
     /**
+     * Interface for a class that is fetching important site information.
+     */
+    public interface ImportantSitesCallback {
+        public abstract void setImportantRegisterableDomains(String[] domains);

[Theresa, 2016/04/20 00:45:30]

This public method needs a JavaDoc.

[dmurph, 2016/04/20 22:06:28]

Done.

+    }
+
+    /**
      * Interface to a class that receives callbacks instructing it to inform the user about other
      * forms of browsing history.
      */
@@ -730,9 +739,20 @@ public final class PrefServiceBridge {
      */
     public void clearBrowsingData(
             OnClearBrowsingDataListener listener, int[] dataTypes, int timePeriod) {
+        clearBrowsingDataExcludingDomains(listener, dataTypes, timePeriod, new String[0]);
+    }
+
+    /**
+     * Same as above, but now we can specify a list of domains to exclude from clearing browsing
+     * data.
+     * DO NOT USE THIS METHOD UNLESS CALLER KNOWS WHAT THEY'RE DOING. NOT ALL BACKENDS ARE

[Theresa, 2016/04/20 00:45:30]

nit: I'd prefer this wasn't in all caps.

[dmurph, 2016/04/20 22:06:27]

Done.

+     * SUPPORTED YET, AND MORE DATA THAN EXPECTED COULD BE DELETED. See crbug.com/113621.
+     */
+    public void clearBrowsingDataExcludingDomains(OnClearBrowsingDataListener listener,
+            int[] dataTypes, int timePeriod, String[] origins) {
         assert mClearBrowsingDataListener == null;
         mClearBrowsingDataListener = listener;
-        nativeClearBrowsingData(dataTypes, timePeriod);
+        nativeClearBrowsingData(dataTypes, timePeriod, origins);
     }
 
     /*
@@ -750,6 +770,19 @@ public final class PrefServiceBridge {
         }
     }
 
+    public void fetchImportantSites(ImportantSitesCallback callback) {

[Theresa, 2016/04/20 00:45:30]

This needs a JavaDoc too

[dmurph, 2016/04/20 22:06:28]

Done.

+        assert mImportantSitesCallback == null;

[Theresa, 2016/04/20 00:45:30]

Is there a case where this may get called multiple times? I'm wondering if it
may be better just to return if mImportantSitesCallback != null rather than
assert null.

[dmurph, 2016/04/20 22:06:28]

Well, I think we're trying to protect against accidentally having this called
multiple times on the same object before the callback is triggered. Protecting
against programmer mistake, which I think we should prefer failing for.

Let me know if you feel strongly though, I can switch it to anything. 

[Theresa, 2016/04/21 16:05:28]

Just to confirm, there should only ever be one active caller for this method
(e.g. two classes won't both be requesting important site info)? If that's the
case, an assert is fine with me.

+        mImportantSitesCallback = callback;

[Theresa, 2016/04/20 00:45:30]

Since this method just sets the important sets callback would it be more
appropriately named "setImportantSitesCallback"?

[dmurph, 2016/04/20 22:06:28]

Whoops, I meant to call the native method. I'm modelling this off of
"clearBrowsingData" above, which does it like this. Let me know if you want me
to do it a different way, I fixed it to match the above method.

[Theresa, 2016/04/21 16:05:28]

This looks good.

+    }
+
+    @CalledByNative
+    private void importantSitesFetched(String[] domains) {
+        if (mImportantSitesCallback != null) {
+            mImportantSitesCallback.setImportantRegisterableDomains(domains);
+            mImportantSitesCallback = 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
@@ -1050,10 +1083,12 @@ public final class PrefServiceBridge {
     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 nativeClearBrowsingData(
+            int[] dataTypes, int timePeriod, String[] blacklistDomains);
     private native void nativeRequestInfoAboutOtherFormsOfBrowsingHistory(
             OtherFormsOfBrowsingHistoryListener listener);
     private native boolean nativeCanDeleteBrowsingHistory();
+    private native void nativeFetchImportantSites();
     private native void nativeSetAllowCookiesEnabled(boolean allow);
     private native void nativeSetBackgroundSyncEnabled(boolean allow);
     private native void nativeSetBlockThirdPartyCookiesEnabled(boolean enabled);