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

chrome/browser/android/preferences/pref_service_bridge.cc

 // 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.
 
 #include "chrome/browser/android/preferences/pref_service_bridge.h"
 
 #include <jni.h>
 #include <stddef.h>
 #include <vector>
 
 #include "base/android/build_info.h"
 #include "base/android/jni_android.h"
 #include "base/android/jni_array.h"
 #include "base/android/jni_string.h"
 #include "base/android/jni_weak_ref.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/scoped_observer.h"
 #include "base/strings/string_util.h"
 #include "base/values.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/browsing_data/browsing_data_counter_utils.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/origin_filter_builder.h"
 #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
 #include "chrome/browser/net/prediction_options.h"
 #include "chrome/browser/prefs/incognito_mode_prefs.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/translate/chrome_translate_client.h"
 #include "chrome/browser/ui/android/android_about_app_info.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/grit/locale_settings.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "components/content_settings/core/common/content_settings.h"
 #include "components/content_settings/core/common/content_settings_pattern.h"
 #include "components/content_settings/core/common/pref_names.h"
 #include "components/metrics/metrics_pref_names.h"
 #include "components/password_manager/core/common/password_manager_pref_names.h"
 #include "components/prefs/pref_service.h"
 #include "components/signin/core/common/signin_pref_names.h"
 #include "components/strings/grit/components_locale_settings.h"
 #include "components/translate/core/browser/translate_prefs.h"
 #include "components/translate/core/common/translate_pref_names.h"
 #include "components/version_info/version_info.h"
 #include "components/web_resource/web_resource_pref_names.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/user_metrics.h"
 #include "jni/PrefServiceBridge_jni.h"
 #include "ui/base/l10n/l10n_util.h"
+#include "url/origin.h"
 
 using base::android::AttachCurrentThread;
 using base::android::CheckException;
 using base::android::ConvertJavaStringToUTF8;
 using base::android::ConvertUTF8ToJavaString;
 using base::android::ScopedJavaLocalRef;
 using base::android::ScopedJavaGlobalRef;
 using content::BrowserThread;
 
 namespace {
@@ skipping 427 matching lines @@
 
     Java_PrefServiceBridge_browsingDataCleared(
         env, weak_chrome_native_preferences_.get(env).obj());
   }
 
  private:
   JavaObjectWeakGlobalRef weak_chrome_native_preferences_;
   ScopedObserver<BrowsingDataRemover, BrowsingDataRemover::Observer> observer_;
 };
 
+// Redirects a BrowsingDataRemover completion callback back into the java
+// callback.
+class ClearBrowsingDataObserverWithCallback

[newt (away), 2016/03/09 05:40:02]

There's no reason to add this new class; just modify ClearBrowsingDataObserver
above (which is currently unused in this patch set).

Also, please remove mClearBrowsingDataListener and browsingDataCleared() from
PrefServiceBridge, since they won't be used after this patch.

+    : public BrowsingDataRemover::Observer {
+ public:
+  ClearBrowsingDataObserverWithCallback(
+      JNIEnv* env,
+      jobject java_callback,
+      BrowsingDataRemover* browsing_data_remover)
+      : weak_java_callback_(env, java_callback), observer_(this) {
+    observer_.Add(browsing_data_remover);
+  }
+
+  void OnBrowsingDataRemoverDone() override {
+    // We delete ourselves when done.
+    scoped_ptr<ClearBrowsingDataObserverWithCallback> auto_delete(this);
+
+    JNIEnv* env = AttachCurrentThread();
+    if (weak_java_callback_.get(env).is_null())

[newt (away), 2016/03/09 05:40:02]

Incidentally, this use of the weak pointer is broken and prone to NPEs. If
garbage collection runs after line 519, but before line 522, then we'll get a
null pointer exception when calling "weak_java_callback_.get(env).obj()" on line
523.

As explained here
https://www.ibm.com/support/knowledgecenter/SSYKE2_8.0.0/com.ibm.java.lnx.80....
:

"It is not safe to call most JNI functions with a weak global reference, even if
you have tested that the reference is not null, because the weak global
reference could become a null reference after it has been tested or even during
the JNI function. Instead, a weak global reference should always be promoted to
a strong reference before it is used. You can promote a weak global reference
using the NewLocalRef or NewGlobalRef JNI functions."

Anyway, avoid weak references when possible for reasons mentioned below.

+      return;
+
+    Java_OnClearBrowsingDataListener_onBrowsingDataCleared(
+        env, weak_java_callback_.get(env).obj());
+  }
+
+ private:
+  JavaObjectWeakGlobalRef weak_java_callback_;

[newt (away), 2016/03/09 05:40:02]

This shouldn't be weak; it should just be a regular ScopedJavaGlobalRef. Use
weak pointers sparingly, since they can introduce nondeterministic behavior that
depends on the whims of the garbage collector. For example, imagine this call

  PrefServiceBridge.clearBrowsingDataExcludingOrigins(new
ClearBrowsingDataObserver() {
      @Override
      public void onBrowsingDataCleared() {
          hideProgressDialog();
      }
  }, ...);

What happens if the garbage collector runs before the browsing data has been
cleared? In that case, the otherwise unreferenced ClearBrowsingDataObserver
would be gc'd and would never receive a callback and the progress dialog would
never be dismissed. This is definitely not what we want. A normal non-weak
reference should be used here.

+  ScopedObserver<BrowsingDataRemover, BrowsingDataRemover::Observer> observer_;
+};
+
 }  // namespace
 
 static jboolean GetBrowsingDataDeletionPreference(
     JNIEnv* env,
     const JavaParamRef<jobject>& obj,
     jint data_type) {
   DCHECK_GE(data_type, 0);
   DCHECK_LT(data_type, BrowsingDataType::NUM_TYPES);
 
   // If there is no corresponding preference for this |data_type|, pretend
@@ skipping 41 matching lines @@
   GetPrefService()->SetInteger(prefs::kDeleteTimePeriod, time_period);
 }
 
 static void ClearBrowsingData(JNIEnv* env,
                               const JavaParamRef<jobject>& obj,
                               const JavaParamRef<jintArray>& data_types) {
   BrowsingDataRemover* browsing_data_remover =
       BrowsingDataRemoverFactory::GetForBrowserContext(GetOriginalProfile());
   // ClearBrowsingDataObserver deletes itself when |browsing_data_remover| is
   // done.
-  new ClearBrowsingDataObserver(env, obj, browsing_data_remover);
+  new ClearBrowsingDataObserverWithCallback(env, obj, browsing_data_remover);
 
   std::vector<int> data_types_vector;
   base::android::JavaIntArrayToIntVector(env, data_types, &data_types_vector);
 
   int remove_mask = 0;
   for (const int data_type : data_types_vector) {
     switch (static_cast<BrowsingDataType>(data_type)) {
       case HISTORY:
         remove_mask |= BrowsingDataRemover::REMOVE_HISTORY;
         break;
@@ skipping 19 matching lines @@
     }
   }
 
   int period_selected = GetPrefService()->GetInteger(prefs::kDeleteTimePeriod);
   browsing_data_remover->Remove(
       BrowsingDataRemover::Period(
           static_cast<BrowsingDataRemover::TimePeriod>(period_selected)),
       remove_mask, BrowsingDataHelper::UNPROTECTED_WEB);
 }
 
+static void ClearBrowsingDataExcludingOrigins(

[newt (away), 2016/03/09 05:40:02]

As Martin mentioned, please combine this method with the method above. If we're
clearing all browsing data (and excluding any origins), you could pass "null"
instead of an array of excluded origins.

+    JNIEnv* env,
+    const JavaParamRef<jclass>& clazz,
+    const JavaParamRef<jobject>& java_callback,
+    const JavaParamRef<jintArray>& data_types,
+    const JavaParamRef<jobjectArray>& excluding_origins) {

[newt (away), 2016/03/09 05:40:02]

call this "excluded_origins" or just "origins"

+  BrowsingDataRemover* browsing_data_remover =
+      BrowsingDataRemoverFactory::GetForBrowserContext(GetOriginalProfile());
+  // ClearBrowsingDataObserver deletes itself when |browsing_data_remover| is
+  // done.
+  new ClearBrowsingDataObserver(env, java_callback.obj(),
+                                browsing_data_remover);
+
+  std::vector<int> data_types_vector;
+  base::android::JavaIntArrayToIntVector(env, data_types, &data_types_vector);
+
+  int remove_mask = 0;
+  for (const int data_type : data_types_vector) {
+    switch (static_cast<BrowsingDataType>(data_type)) {
+      case HISTORY:
+        remove_mask |= BrowsingDataRemover::REMOVE_HISTORY;
+        break;
+      case CACHE:
+        remove_mask |= BrowsingDataRemover::REMOVE_CACHE;
+        break;
+      case COOKIES:
+        remove_mask |= BrowsingDataRemover::REMOVE_COOKIES;
+        remove_mask |= BrowsingDataRemover::REMOVE_SITE_DATA;
+        break;
+      case PASSWORDS:
+        remove_mask |= BrowsingDataRemover::REMOVE_PASSWORDS;
+        break;
+      case FORM_DATA:
+        remove_mask |= BrowsingDataRemover::REMOVE_FORM_DATA;
+        break;
+      case BOOKMARKS:
+        // Bookmarks are deleted separately on the Java side.
+        NOTREACHED();
+        break;
+      case NUM_TYPES:
+        NOTREACHED();
+    }
+  }
+
+  OriginFilterBuilder filter_builder(OriginFilterBuilder::BLACKLIST);
+  jsize origin_count = env->GetArrayLength(excluding_origins.obj());
+  for (jsize i = 0; i < origin_count; ++i) {
+    jstring url =
+        (jstring)env->GetObjectArrayElement(excluding_origins.obj(), i);
+    filter_builder.AddOrigin(url::Origin(ConvertJavaStringToUTF8(env, url)))
+  }
+
+  /* Commented out, as we don't have the filterbuilder hookup yet.
+    int period_selected =
+    GetPrefService()->GetInteger(prefs::kDeleteTimePeriod);
+    browsing_data_remover->Remove(
+        BrowsingDataRemover::Period(
+            static_cast<BrowsingDataRemover::TimePeriod>(period_selected)),
+        remove_mask, filter_builder, BrowsingDataHelper::UNPROTECTED_WEB);
+  */
+}
+
 static jboolean CanDeleteBrowsingHistory(JNIEnv* env,
                                          const JavaParamRef<jobject>& obj) {
   return GetPrefService()->GetBoolean(prefs::kAllowDeletingBrowserHistory);
 }
 
 static void SetAllowCookiesEnabled(JNIEnv* env,
                                    const JavaParamRef<jobject>& obj,
                                    jboolean allow) {
   HostContentSettingsMap* host_content_settings_map =
       HostContentSettingsMapFactory::GetForProfile(GetOriginalProfile());
@@ skipping 426 matching lines @@
 
   return ConvertJavaStringToUTF8(android_permission);
 }
 
 static void SetSupervisedUserId(JNIEnv* env,
                                 const JavaParamRef<jobject>& obj,
                                 const JavaParamRef<jstring>& pref) {
   GetPrefService()->SetString(prefs::kSupervisedUserId,
                               ConvertJavaStringToUTF8(env, pref));
 }