[subresource_filter] Move test-only JNI calls out of binary

Before, the JNI shim called into native code that was compiled into
Chrome proper.

Now, due to r517541 there is a better way to do this by including the
JNI code in libchromefortest.

Bug: None
Change-Id: I9f34f5105366b927d6dd6676d97a9da6d2540385
Reviewed-on: https://chromium-review.googlesource.com/997877
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Reviewed-by: John Budorick <jbudorick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551131}

chrome/android/BUILD.gn

@@ -858,6 +858,7 @@ chrome_shared_library("libchromefortest") {
     "//base/test:test_support",
     "//chrome/browser/profiling_host:jni_headers",
     "//components/heap_profiling:test_support",
+    "//chrome/browser/subresource_filter:subresource_filter_test_support",
   ]
 }
 
@@ -1120,6 +1121,7 @@ chrome_public_apk_tmpl_shared("chrome_public_apk_for_test") {
   }
   deps = [
     "//chrome/browser/profiling_host:profiling_host_java_test_support",
+    "//chrome/browser/subresource_filter:subresource_filter_java_test_support",
   ]
 }
 
@@ -1244,6 +1246,7 @@ instrumentation_test_apk("chrome_public_test_apk") {
     "//chrome/android/webapk/libs/runtime_library:runtime_library_javatests",
     "//chrome/android/webapk/shell_apk:shell_apk_javatests",
     "//chrome/browser/profiling_host:profiling_host_javatests",
+    "//chrome/browser/subresource_filter:subresource_filter_javatests",
     "//third_party/android_support_test_runner:runner_java",
   ]
 

chrome/android/java_sources.gni

@@ -1092,7 +1092,6 @@ chrome_java_sources = [
   "java/src/org/chromium/chrome/browser/preferences/website/WebsitePreferenceBridge.java",
   "java/src/org/chromium/chrome/browser/prerender/ChromePrerenderService.java",
   "java/src/org/chromium/chrome/browser/prerender/ExternalPrerenderHandler.java",
-  "java/src/org/chromium/chrome/browser/subresource_filter/TestSubresourceFilterPublisher.java",
   "java/src/org/chromium/chrome/browser/printing/PrintShareActivity.java",
   "java/src/org/chromium/chrome/browser/printing/TabPrinter.java",
   "java/src/org/chromium/chrome/browser/profiles/Profile.java",
@@ -1841,7 +1840,6 @@ chrome_test_java_sources = [
   "javatests/src/org/chromium/chrome/browser/provider/ProviderTestRule.java",
   "javatests/src/org/chromium/chrome/browser/push_messaging/PushMessagingTest.java",
   "javatests/src/org/chromium/chrome/browser/SafeBrowsingTest.java",
-  "javatests/src/org/chromium/chrome/browser/SubresourceFilterTest.java",
   "javatests/src/org/chromium/chrome/browser/MockSafeBrowsingApiHandler.java",
   "javatests/src/org/chromium/chrome/browser/search_engines/TemplateUrlServiceTest.java",
   "javatests/src/org/chromium/chrome/browser/searchwidget/SearchActivityTest.java",

chrome/android/javatests/src/org/chromium/chrome/browser/SubresourceFilterTest.java → chrome/android/javatests/src/org/chromium/chrome/browser/subresource_filter/SubresourceFilterTest.java

@@ -2,7 +2,7 @@
 // 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;
+package org.chromium.chrome.browser.subresource_filter;
 
 import android.support.test.InstrumentationRegistry;
 import android.support.test.filters.MediumTest;
@@ -17,9 +17,11 @@ import org.junit.runner.RunWith;
 import org.chromium.base.ThreadUtils;
 import org.chromium.base.test.util.CallbackHelper;
 import org.chromium.base.test.util.CommandLineFlags;
+import org.chromium.chrome.browser.ChromeActivity;
+import org.chromium.chrome.browser.ChromeSwitches;
+import org.chromium.chrome.browser.MockSafeBrowsingApiHandler;
 import org.chromium.chrome.browser.infobar.AdsBlockedInfoBar;
 import org.chromium.chrome.browser.infobar.InfoBar;
-import org.chromium.chrome.browser.subresource_filter.TestSubresourceFilterPublisher;
 import org.chromium.chrome.browser.tab.Tab;
 import org.chromium.chrome.browser.tabmodel.EmptyTabModelObserver;
 import org.chromium.chrome.browser.tabmodel.TabModel;
@@ -58,7 +60,7 @@ public final class SubresourceFilterTest {
             "{\"matches\":[{\"threat_type\":\"13\",\"sf_bas\":\"warn\"}]}";
 
     private void createAndPublishRulesetDisallowingSuffix(String suffix) {
-        TestSubresourceFilterPublisher publisher = new TestSubresourceFilterPublisher();
+        TestRulesetPublisher publisher = new TestRulesetPublisher();
         ThreadUtils.runOnUiThreadBlocking(
                 (Runnable) ()
                         -> publisher.createAndPublishRulesetDisallowingSuffixForTesting(suffix));

chrome/android/java/src/org/chromium/chrome/browser/subresource_filter/TestSubresourceFilterPublisher.java → chrome/android/javatests/src/org/chromium/chrome/browser/subresource_filter/TestRulesetPublisher.java

@@ -10,7 +10,7 @@ import org.chromium.base.annotations.CalledByNative;
  * Class which aids in publishing test rulesets for SubresourceFilter instrumentation tests.
  * All methods and members must be called on the UI thread.
  */
-public final class TestSubresourceFilterPublisher {
+public final class TestRulesetPublisher {
     private boolean mPublished = false;
 
     public void createAndPublishRulesetDisallowingSuffixForTesting(String suffix) {

chrome/browser/BUILD.gn

@@ -2159,7 +2159,6 @@ jumbo_split_static_library("browser") {
       "android/signin/signin_manager_android.h",
       "android/signin/signin_promo_util_android.cc",
       "android/signin/signin_promo_util_android.h",
-      "android/subresource_filter/test_subresource_filter_publisher.cc",
       "android/tab_android.cc",
       "android/tab_android.h",
       "android/tab_state.cc",
@@ -4406,7 +4405,6 @@ if (is_android) {
       "../android/java/src/org/chromium/chrome/browser/snackbar/smartlockautosignin/AutoSigninSnackbarController.java",
       "../android/java/src/org/chromium/chrome/browser/ssl/CaptivePortalHelper.java",
       "../android/java/src/org/chromium/chrome/browser/ssl/SecurityStateModel.java",
-      "../android/java/src/org/chromium/chrome/browser/subresource_filter/TestSubresourceFilterPublisher.java",
       "../android/java/src/org/chromium/chrome/browser/suggestions/MostVisitedSites.java",
       "../android/java/src/org/chromium/chrome/browser/suggestions/MostVisitedSitesBridge.java",
       "../android/java/src/org/chromium/chrome/browser/suggestions/SuggestionsEventReporterBridge.java",

chrome/browser/android/subresource_filter/test_subresource_filter_publisher.cc

-// Copyright 2018 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 <memory>
-#include <string>
-#include <utility>
-#include <vector>
-
-#include <jni.h>
-
-#include "base/android/jni_android.h"
-#include "base/android/jni_string.h"
-#include "base/android/scoped_java_ref.h"
-#include "base/bind.h"
-#include "base/bind_helpers.h"
-#include "base/callback.h"
-#include "base/files/file_util.h"
-#include "base/files/scoped_temp_dir.h"
-#include "base/numerics/safe_conversions.h"
-#include "base/threading/thread_restrictions.h"
-#include "chrome/browser/browser_process.h"
-#include "components/subresource_filter/content/browser/content_ruleset_service.h"
-#include "components/subresource_filter/core/browser/ruleset_service.h"
-#include "components/url_pattern_index/unindexed_ruleset.h"
-#include "content/public/browser/browser_thread.h"
-#include "jni/TestSubresourceFilterPublisher_jni.h"
-#include "third_party/protobuf/src/google/protobuf/io/zero_copy_stream_impl_lite.h"
-
-// TODO(csharrison): This whole file is a hack, because Android cannot use
-// native files that are test-only. So, this is a duplication and simplification
-// of a lot of subresource_filter test harness code. Because it is compiled into
-// a normal build of Chrome, I've tried to strip it down as much as possible.
-// Once native test files can be used on Android, most all of this can be
-// deleted.
-
-using content::BrowserThread;
-
-namespace {
-
-url_pattern_index::proto::UrlRule CreateSuffixRule(const std::string& suffix) {
-  url_pattern_index::proto::UrlRule rule;
-  rule.set_semantics(url_pattern_index::proto::RULE_SEMANTICS_BLACKLIST);
-  rule.set_source_type(url_pattern_index::proto::SOURCE_TYPE_ANY);
-  rule.set_element_types(url_pattern_index::proto::ELEMENT_TYPE_ALL);
-  rule.set_url_pattern_type(
-      url_pattern_index::proto::URL_PATTERN_TYPE_SUBSTRING);
-  rule.set_anchor_left(url_pattern_index::proto::ANCHOR_TYPE_NONE);
-  rule.set_anchor_right(url_pattern_index::proto::ANCHOR_TYPE_BOUNDARY);
-  rule.set_url_pattern(suffix);
-  return rule;
-}
-
-void OnPublished(std::unique_ptr<base::ScopedTempDir> scoped_temp_dir,
-                 subresource_filter::ContentRulesetService* service,
-                 base::android::ScopedJavaGlobalRef<jobject> publisher) {
-  base::ScopedAllowBlockingForTesting allow_blocking;
-  // Ensure the callback does not retain |publisher| by resetting it.
-  service->SetRulesetPublishedCallbackForTesting(base::RepeatingClosure());
-  scoped_temp_dir.reset();
-  Java_TestSubresourceFilterPublisher_onRulesetPublished(
-      base::android::AttachCurrentThread(), publisher);
-}
-
-}  // namespace
-
-void JNI_TestSubresourceFilterPublisher_CreateAndPublishRulesetDisallowingSuffixForTesting(
-    JNIEnv* env,
-    const base::android::JavaParamRef<jobject>& publisher_param,
-    const base::android::JavaParamRef<jstring>& suffix) {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  base::android::ScopedJavaGlobalRef<jobject> publisher;
-  publisher.Reset(env, publisher_param);
-
-  // Create the ruleset contents.
-  std::string ruleset_contents_str;
-  google::protobuf::io::StringOutputStream output(&ruleset_contents_str);
-  url_pattern_index::UnindexedRulesetWriter ruleset_writer(&output);
-  ruleset_writer.AddUrlRule(
-      CreateSuffixRule(base::android::ConvertJavaStringToUTF8(env, suffix)));
-  ruleset_writer.Finish();
-  auto* data = reinterpret_cast<const uint8_t*>(ruleset_contents_str.data());
-  std::vector<uint8_t> ruleset_contents(data,
-                                        data + ruleset_contents_str.size());
-
-  // Create the ruleset directory and write the ruleset data into a file there.
-  base::ScopedAllowBlockingForTesting allow_blocking;
-  auto scoped_temp_dir = std::make_unique<base::ScopedTempDir>();
-  CHECK(scoped_temp_dir->CreateUniqueTempDir());
-  base::FilePath ruleset_path = scoped_temp_dir->GetPath().AppendASCII("1");
-  int ruleset_size_as_int = base::checked_cast<int>(ruleset_contents.size());
-  CHECK_EQ(
-      ruleset_size_as_int,
-      base::WriteFile(ruleset_path,
-                      reinterpret_cast<const char*>(ruleset_contents.data()),
-                      ruleset_size_as_int));
-
-  subresource_filter::ContentRulesetService* service =
-      g_browser_process->subresource_filter_ruleset_service();
-  service->SetIsAfterStartupForTesting();
-  service->SetRulesetPublishedCallbackForTesting(base::BindRepeating(
-      &OnPublished, base::Passed(&scoped_temp_dir), service, publisher));
-
-  subresource_filter::UnindexedRulesetInfo unindexed_ruleset_info;
-  unindexed_ruleset_info.content_version = "1";
-  unindexed_ruleset_info.ruleset_path = ruleset_path;
-  service->IndexAndStoreAndPublishRulesetIfNeeded(unindexed_ruleset_info);
-}

chrome/browser/subresource_filter/BUILD.gn

+# Copyright 2017 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.
+
+# TODO(csharrison): Move subresource_filter_browsertests here.
+
+if (is_android) {
+  import("//build/config/android/rules.gni")
+
+  generate_jni("jni_headers") {
+    sources = [
+      "../../android/javatests/src/org/chromium/chrome/browser/subresource_filter/TestRulesetPublisher.java",
+    ]
+    jni_package = "chrome_subresource_filter"
+  }
+
+  # This library must be included by the apk_under_test in order for the JNI
+  # shim to function correctly.
+  android_library("subresource_filter_java_test_support") {
+    testonly = true
+    java_files = [ "../../android/javatests/src/org/chromium/chrome/browser/subresource_filter/TestRulesetPublisher.java" ]
+    deps = [
+      "//base:base_java",
+    ]
+  }
+
+  # This library must be included by the instrumentation_test. It must not be
+  # included by the apk_under_test, since in debug builds, java classes that
+  # appear in the apk_under_test are stripped from the instrumentation_test.
+  # This library contains dependencies that must not be stripped from the
+  # instrumentation_test.
+  android_library("subresource_filter_javatests") {
+    testonly = true
+    java_files = [ "../../android/javatests/src/org/chromium/chrome/browser/subresource_filter/SubresourceFilterTest.java" ]
+    deps = [
+      ":subresource_filter_java_test_support",
+      "//base:base_java",
+      "//base:base_java_test_support",
+      "//chrome/android:chrome_java",
+      "//chrome/android:chrome_test_java",
+      "//chrome/test/android:chrome_java_test_support",
+      "//content/public/test/android:content_java_test_support",
+      "//third_party/android_support_test_runner:rules_java",
+      "//third_party/android_support_test_runner:runner_java",
+      "//third_party/junit",
+    ]
+  }
+
+  source_set("subresource_filter_test_support") {
+    testonly = true
+    sources = [
+      "android_test_ruleset_publisher.cc",
+    ]
+    deps = [
+      ":jni_headers",
+      "//components/subresource_filter/core/common:test_support",
+      "//net",
+      "//third_party/protobuf:protobuf_lite",
+    ]
+  }
+}

chrome/browser/subresource_filter/android_test_ruleset_publisher.cc

+// Copyright 2018 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 <memory>
+#include <string>
+
+#include "base/android/jni_android.h"
+#include "base/android/jni_string.h"
+#include "base/android/scoped_java_ref.h"
+#include "base/strings/string_number_conversions.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/subresource_filter/test_ruleset_publisher.h"
+#include "components/subresource_filter/content/browser/content_ruleset_service.h"
+#include "components/subresource_filter/core/browser/ruleset_service.h"
+#include "components/subresource_filter/core/common/test_ruleset_creator.h"
+#include "jni/TestRulesetPublisher_jni.h"
+
+namespace {
+
+void OnRulesetPublished(
+    std::unique_ptr<subresource_filter::testing::TestRulesetCreator> creator,
+    subresource_filter::ContentRulesetService* service,
+    base::android::ScopedJavaGlobalRef<jobject> publisher) {
+  // Ensure the callback does not retain |publisher| by resetting it.
+  service->SetRulesetPublishedCallbackForTesting(base::RepeatingClosure());
+  creator.reset();
+  Java_TestRulesetPublisher_onRulesetPublished(
+      base::android::AttachCurrentThread(), publisher);
+}
+
+}  // namespace
+
+void JNI_TestRulesetPublisher_CreateAndPublishRulesetDisallowingSuffixForTesting(
+    JNIEnv* env,
+    const base::android::JavaParamRef<jobject>& publisher_param,
+    const base::android::JavaParamRef<jstring>& suffix) {
+  subresource_filter::testing::TestRulesetPair test_ruleset_pair;
+  auto creator =
+      std::make_unique<subresource_filter::testing::TestRulesetCreator>();
+  std::string suffix_str = base::android::ConvertJavaStringToUTF8(env, suffix);
+  creator->CreateRulesetToDisallowURLsWithPathSuffix(suffix_str,
+                                                     &test_ruleset_pair);
+
+  subresource_filter::UnindexedRulesetInfo unindexed_ruleset_info;
+  unindexed_ruleset_info.content_version =
+      base::NumberToString(base::Hash(suffix_str));
+  unindexed_ruleset_info.ruleset_path = test_ruleset_pair.unindexed.path;
+
+  base::android::ScopedJavaGlobalRef<jobject> publisher;
+  publisher.Reset(env, publisher_param);
+  auto* ruleset_service =
+      g_browser_process->subresource_filter_ruleset_service();
+  ruleset_service->SetIsAfterStartupForTesting();
+  ruleset_service->SetRulesetPublishedCallbackForTesting(base::BindRepeating(
+      &OnRulesetPublished, base::Passed(&creator), ruleset_service, publisher));
+  ruleset_service->IndexAndStoreAndPublishRulesetIfNeeded(
+      unindexed_ruleset_info);
+}