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

chrome/browser/android/preferences/pref_service_bridge.cc

Index: chrome/browser/android/preferences/pref_service_bridge.cc
diff --git a/chrome/browser/android/preferences/pref_service_bridge.cc b/chrome/browser/android/preferences/pref_service_bridge.cc
index 65eff4b91db7f4b087e349cb6f8a67b108c73cdf..96b1d8d562b53260e154a455fbfdfe79214552ad 100644
--- a/chrome/browser/android/preferences/pref_service_bridge.cc
+++ b/chrome/browser/android/preferences/pref_service_bridge.cc
@@ -20,11 +20,14 @@
 #include "base/scoped_observer.h"
 #include "base/strings/string_util.h"
 #include "base/values.h"
+#include "chrome/browser/android/preferences/important_sites_util.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/browsing_data/browsing_data_counter_utils.h"
+#include "chrome/browser/browsing_data/browsing_data_filter_builder.h"
 #include "chrome/browser/browsing_data/browsing_data_helper.h"
 #include "chrome/browser/browsing_data/browsing_data_remover.h"
 #include "chrome/browser/browsing_data/browsing_data_remover_factory.h"
+#include "chrome/browser/browsing_data/registrable_domain_filter_builder.h"
 #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
 #include "chrome/browser/history/web_history_service_factory.h"
 #include "chrome/browser/net/prediction_options.h"
@@ -63,6 +66,7 @@ using base::android::ScopedJavaGlobalRef;
 using content::BrowserThread;
 
 namespace {
+const size_t kMaxImportantSites = 5;
 
 Profile* GetOriginalProfile() {
   return ProfileManager::GetActiveUserProfile()->GetOriginalProfile();
@@ -558,10 +562,12 @@ static void SetBrowsingDataDeletionTimePeriod(
   GetPrefService()->SetInteger(prefs::kDeleteTimePeriod, time_period);
 }
 
-static void ClearBrowsingData(JNIEnv* env,
-                              const JavaParamRef<jobject>& obj,
-                              const JavaParamRef<jintArray>& data_types,
-                              jint time_period) {
+static void ClearBrowsingData(
+    JNIEnv* env,
+    const JavaParamRef<jobject>& obj,
+    const JavaParamRef<jintArray>& data_types,
+    jint time_period,
+    const JavaParamRef<jobjectArray>& excluding_domains) {
   BrowsingDataRemover* browsing_data_remover =
       BrowsingDataRemoverFactory::GetForBrowserContext(GetOriginalProfile());
   // ClearBrowsingDataObserver deletes itself when |browsing_data_remover| is
@@ -598,11 +604,19 @@ static void ClearBrowsingData(JNIEnv* env,
         NOTREACHED();
     }
   }
+  std::vector<std::string> cpp_excluding_domains;

[Ted C, 2016/04/21 20:18:16]

I would say the common naming scheme would be to call the java param
jexcluding_domains then this would be just excluding_domains

[dmurph, 2016/04/22 18:06:22]

Done.

+  base::android::AppendJavaStringArrayToStringVector(
+      env, excluding_domains.obj(), &cpp_excluding_domains);
+  RegistrableDomainFilterBuilder filter_builder(
+      BrowsingDataFilterBuilder::BLACKLIST);
+  for (const std::string& domain : cpp_excluding_domains) {
+    filter_builder.AddRegisterableDomain(domain);
+  }
 
-  browsing_data_remover->Remove(
+  browsing_data_remover->RemoveWithFilter(
       BrowsingDataRemover::Period(
           static_cast<BrowsingDataRemover::TimePeriod>(time_period)),
-      remove_mask, BrowsingDataHelper::UNPROTECTED_WEB);
+      remove_mask, BrowsingDataHelper::UNPROTECTED_WEB, filter_builder);
 }
 
 static jboolean CanDeleteBrowsingHistory(JNIEnv* env,
@@ -610,6 +624,16 @@ static jboolean CanDeleteBrowsingHistory(JNIEnv* env,
   return GetPrefService()->GetBoolean(prefs::kAllowDeletingBrowserHistory);
 }
 
+static void FetchImportantSites(JNIEnv* env, const JavaParamRef<jobject>& obj) {
+  std::vector<std::string> important_domains =
+      ImportantSitesUtil::GetImportantRegisterableDomains(GetOriginalProfile(),

[Ted C, 2016/04/21 20:18:16]

is this guaranteed to be fast?  looks "scary-ish" to be done on the UI thread,
but maybe it is all in memory/quick/etc...

[dmurph, 2016/04/21 20:31:23]

It's all in memory. If we ever need to hit disk (which we may need to soon),
then I'll be putting this on a different thread. But we have the callback set up
to handle this asynchronous behavior.

+                                                          kMaxImportantSites);
+  ScopedJavaLocalRef<jobjectArray> string_array =
+      base::android::ToJavaArrayOfStrings(env, important_domains);
+  Java_PrefServiceBridge_importantSitesFetched(env, obj.obj(),

[Ted C, 2016/04/21 20:18:16]

Looking at the java side, I expected this to be an async call since you are
tracking a callback.  why not just return the string list instead of taking a
callback?

It is a bit clearer from an API standpoint and you wouldn't get unexpected
timing issues w/ the immediate callback triggered here.

[dmurph, 2016/04/21 20:31:23]

I expect this call to not always be synchronous, so I'd rather make the API
support that right away.

[Ted C, 2016/04/21 21:23:39]

Then a couple things.

1.) Pass down the java callback to native and track it along with the request. 
Having it a one at a time thing in java land is gross and following clear
browsing isn't what I would do.

2.) Post the callback in the javaside for now to ensure it is infact async
because otherwise the callers might not be happy with what happens.

[dmurph, 2016/04/22 18:06:22]

How does this look? I pass the callback down.

[Ted C, 2016/04/22 18:37:48]

Perfect

+                                               string_array.obj());
+}
+
 static void ShowNoticeAboutOtherFormsOfBrowsingHistory(
     ScopedJavaGlobalRef<jobject>* listener,
     bool show) {