[Subresource Filter] Add ability to index ruleset from resource bundle

This CL adds the ability for the subresource filter to index and
publish the ruleset from unindexed ruleset data stored as a resource in
the resource bundle. This functionality paves the way for WebLayer's
upcoming packaging of the in-repo unindexed ruleset data in the
resource bundle.

We also add a unittest of the new functionality. To facilitate adding
the unittest, we add the ability to mock out LoadDataResourceString()
to ui::ResourceBundle::Delegate and ui::MockResourceBundleDelegate.

TBR=spang@chromium.org

Bug: 1116095
Change-Id: Idb2c1b85465ab97e787c8191070f00acd571fadb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2516519
Commit-Queue: Colin Blundell <blundell@chromium.org>
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#824925}

chromecast/common/cast_resource_delegate.cc

@@ -66,6 +66,11 @@ base::RefCountedStaticMemory* CastResourceDelegate::LoadDataResourceBytes(
   return NULL;
 }
 
+base::Optional<std::string> CastResourceDelegate::LoadDataResourceString(
+    int resource_id) {
+  return base::nullopt;
+}
+
 bool CastResourceDelegate::GetRawDataResource(int resource_id,
                                               ui::ScaleFactor scale_factor,
                                               base::StringPiece* value) const {

chromecast/common/cast_resource_delegate.h

@@ -45,6 +45,7 @@ class CastResourceDelegate : public ui::ResourceBundle::Delegate {
   base::RefCountedStaticMemory* LoadDataResourceBytes(
       int resource_id,
       ui::ScaleFactor scale_factor) override;
+  base::Optional<std::string> LoadDataResourceString(int resource_id) override;
   bool GetRawDataResource(int resource_id,
                           ui::ScaleFactor scale_factor,
                           base::StringPiece* value) const override;

components/subresource_filter/content/browser/BUILD.gn

@@ -136,6 +136,7 @@ source_set("unit_tests") {
     "//ipc",
     "//ipc:test_support",
     "//testing/gtest",
+    "//ui/base:test_support",
   ]
   public_deps = [ "//components/subresource_filter/content/mojom" ]
 }

components/subresource_filter/content/browser/DEPS

@@ -10,5 +10,5 @@ include_rules = [
   "+mojo/public",
   "+net/base",
   "+services/metrics/public/cpp",
-  "+ui/base/page_transition_types.h",
+  "+ui/base",
 ]

components/subresource_filter/content/browser/ruleset_service_unittest.cc

@@ -15,6 +15,7 @@
 
 #include "base/bind.h"
 #include "base/environment.h"
+#include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/logging.h"
@@ -35,6 +36,8 @@
 #include "components/url_pattern_index/proto/rules.pb.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "ui/base/resource/mock_resource_bundle_delegate.h"
+#include "ui/base/resource/resource_bundle.h"
 
 namespace subresource_filter {
 
@@ -167,8 +170,10 @@ bool MockFailingIndexRuleset(UnindexedRulesetStreamGenerator*,
 
 // Test fixtures --------------------------------------------------------------
 
-using testing::TestRulesetPair;
+using ::testing::_;
+using ::testing::Return;
 using testing::TestRulesetCreator;
+using testing::TestRulesetPair;
 
 class SubresourceFilteringRulesetServiceTest : public ::testing::Test {
  public:
@@ -251,6 +256,50 @@ class SubresourceFilteringRulesetServiceTest : public ::testing::Test {
     RunBlockingUntilIdle();
   }
 
+  void WaitForIndexAndStoreAndPublishUpdatedRulesetFromResourceBundle(
+      const TestRulesetPair& test_ruleset_pair,
+      const std::string& new_content_version) {
+    int unindexed_ruleset_resource_id = 42;
+    auto unindexed_ruleset_contents = test_ruleset_pair.unindexed.contents;
+    std::string unindexed_ruleset(
+        reinterpret_cast<const char*>(unindexed_ruleset_contents.data()),
+        unindexed_ruleset_contents.size());
+
+    UnindexedRulesetInfo ruleset_info;
+    ruleset_info.resource_id = unindexed_ruleset_resource_id;
+    ruleset_info.content_version = new_content_version;
+
+    // Configure the resource bundle to return |unindexed_ruleset| as the
+    // contents for |unindexed_ruleset_resource_id|.
+    ui::MockResourceBundleDelegate resource_bundle_delegate;
+    EXPECT_CALL(resource_bundle_delegate,
+                LoadDataResourceString(unindexed_ruleset_resource_id))
+        .Times(1)
+        .WillOnce(Return(unindexed_ruleset));
+
+    // Suppress "uninteresting mock function call" warning output that would
+    // occur as part of resource bundle initialization.
+    EXPECT_CALL(resource_bundle_delegate, GetPathForLocalePack(_, _))
+        .WillRepeatedly(Return(base::FilePath()));
+
+    ui::ResourceBundle* orig_resource_bundle =
+        ui::ResourceBundle::SwapSharedInstanceForTesting(nullptr);
+    ui::ResourceBundle::InitSharedInstanceWithLocale(
+        "en-US", &resource_bundle_delegate,
+        ui::ResourceBundle::DO_NOT_LOAD_COMMON_RESOURCES);
+
+    // Now that everything has been set up, do the actual indexing.
+    service()->IndexAndStoreAndPublishRulesetIfNeeded(ruleset_info);
+
+    // Wait for indexing on background task runner.
+    RunBackgroundUntilIdle();
+    // Wait for file to be opened on blocking task runner.
+    RunBlockingUntilIdle();
+
+    ui::ResourceBundle::CleanupSharedInstance();
+    ui::ResourceBundle::SwapSharedInstanceForTesting(orig_resource_bundle);
+  }
+
   // Mark the initialization complete and run task queues until all are empty.
   void SimulateStartupCompletedAndWaitForTasks() {
     DCHECK(mock_publisher());
@@ -622,6 +671,19 @@ TEST_F(SubresourceFilteringRulesetServiceTest, NewRuleset_Published) {
       test_ruleset_1().indexed.contents));
 }
 
+TEST_F(SubresourceFilteringRulesetServiceTest,
+       RulesetFromResourceId_Published) {
+  SimulateStartupCompletedAndWaitForTasks();
+
+  WaitForIndexAndStoreAndPublishUpdatedRulesetFromResourceBundle(
+      test_ruleset_1(), kTestContentVersion1);
+
+  ASSERT_EQ(1u, mock_publisher()->published_rulesets().size());
+  ASSERT_NO_FATAL_FAILURE(AssertValidRulesetFileWithContents(
+      &mock_publisher()->published_rulesets()[0],
+      test_ruleset_1().indexed.contents));
+}
+
 TEST_F(SubresourceFilteringRulesetServiceTest,
        NewRulesetWithEmptyVersion_NotPublished) {
   SimulateStartupCompletedAndWaitForTasks();
@@ -768,7 +830,8 @@ TEST_F(SubresourceFilteringRulesetServiceTest,
   mock_publisher()->RunBestEffortUntilIdle();
 
   UnindexedRulesetInfo ruleset_info;
-  ruleset_info.ruleset_path = base::FilePath();  // Non-existent.
+  ruleset_info.ruleset_path =
+      base::FilePath(FILE_PATH_LITERAL("non/existent/path"));  // Non-existent.
   ruleset_info.content_version = kTestContentVersion1;
   service()->IndexAndStoreAndPublishRulesetIfNeeded(ruleset_info);
   RunBackgroundUntilIdle();

components/subresource_filter/content/browser/ruleset_version.cc

@@ -26,6 +26,10 @@ const char kSubresourceFilterRulesetChecksum[] =
 
 UnindexedRulesetInfo::UnindexedRulesetInfo() = default;
 UnindexedRulesetInfo::~UnindexedRulesetInfo() = default;
+UnindexedRulesetInfo::UnindexedRulesetInfo(const UnindexedRulesetInfo&) =
+    default;
+UnindexedRulesetInfo& UnindexedRulesetInfo::operator=(
+    const UnindexedRulesetInfo&) = default;
 
 IndexedRulesetVersion::IndexedRulesetVersion() = default;
 IndexedRulesetVersion::IndexedRulesetVersion(const std::string& content_version,

components/subresource_filter/content/browser/ruleset_version.h

@@ -26,6 +26,8 @@ namespace subresource_filter {
 struct UnindexedRulesetInfo {
   UnindexedRulesetInfo();
   ~UnindexedRulesetInfo();
+  UnindexedRulesetInfo(const UnindexedRulesetInfo&);
+  UnindexedRulesetInfo& operator=(const UnindexedRulesetInfo&);
 
   // The version of the ruleset contents. Because the wire format of unindexed
   // rules is expected to be stable over time (at least backwards compatible),
@@ -35,9 +37,17 @@ struct UnindexedRulesetInfo {
   // There is no ordering defined on versions.
   std::string content_version;
 
-  // The path to the file containing the unindexed subresource filtering rules.
+  // The (optional) path to the file containing the unindexed subresource
+  // filtering rules. One (but not both) of |ruleset_path| and |resource_id|
+  // should be set.
   base::FilePath ruleset_path;
 
+  // The (optional) grit resource id containing the unindexed subresource
+  // filtering rules, which if supplied is given to the ResourceBundle to
+  // resolve to a string. One (but not both) of |ruleset_path| and |resource_id|
+  // should be set.
+  int resource_id = 0;
+
   // The (optional) path to a file containing the applicable license, which will
   // be copied next to the indexed ruleset. For convenience, the lack of license
   // can be indicated not only by setting |license_path| to empty, but also by

components/subresource_filter/content/browser/unindexed_ruleset_stream_generator.cc

@@ -9,13 +9,24 @@
 #include "base/files/file_path.h"
 #include "components/subresource_filter/content/browser/ruleset_version.h"
 #include "components/subresource_filter/core/browser/copying_file_stream.h"
+#include "third_party/protobuf/src/google/protobuf/io/zero_copy_stream_impl.h"
 #include "third_party/protobuf/src/google/protobuf/io/zero_copy_stream_impl_lite.h"
+#include "ui/base/resource/resource_bundle.h"
 
 namespace subresource_filter {
 
 UnindexedRulesetStreamGenerator::UnindexedRulesetStreamGenerator(
     const UnindexedRulesetInfo& ruleset_info) {
-  GenerateStreamFromFile(ruleset_info.ruleset_path);
+  bool has_ruleset_file = !ruleset_info.ruleset_path.empty();
+
+  DCHECK(has_ruleset_file || ruleset_info.resource_id);
+  DCHECK(!(has_ruleset_file && ruleset_info.resource_id));
+
+  if (has_ruleset_file) {
+    GenerateStreamFromFile(ruleset_info.ruleset_path);
+  } else {
+    GenerateStreamFromResourceId(ruleset_info.resource_id);
+  }
 }
 
 UnindexedRulesetStreamGenerator::~UnindexedRulesetStreamGenerator() = default;
@@ -42,4 +53,15 @@ void UnindexedRulesetStreamGenerator::GenerateStreamFromFile(
           copying_stream_.get(), 4096 /* buffer_size */);
 }
 
+void UnindexedRulesetStreamGenerator::GenerateStreamFromResourceId(
+    int resource_id) {
+  ui::ResourceBundle& bundle = ui::ResourceBundle::GetSharedInstance();
+  std::string data_as_string = bundle.LoadDataResourceString(resource_id);
+  ruleset_size_ = data_as_string.size();
+
+  string_stream_.str(data_as_string);
+  ruleset_stream_ = std::make_unique<google::protobuf::io::IstreamInputStream>(
+      &string_stream_);
+}
+
 }  // namespace subresource_filter

components/subresource_filter/content/browser/unindexed_ruleset_stream_generator.h

@@ -8,6 +8,7 @@
 #include <stdint.h>
 
 #include <memory>
+#include <sstream>
 
 namespace base {
 class FilePath;
@@ -34,6 +35,11 @@ class UnindexedRulesetStreamGenerator {
       const UnindexedRulesetInfo& ruleset_info);
   ~UnindexedRulesetStreamGenerator();
 
+  UnindexedRulesetStreamGenerator(const UnindexedRulesetStreamGenerator&) =
+      delete;
+  UnindexedRulesetStreamGenerator& operator=(
+      const UnindexedRulesetStreamGenerator&) = delete;
+
   // Returns a ZeroCopyInputStream* via which the unindexed ruleset data can be
   // streamed. If the returned pointer is null, the stream is not valid.
   // NOTE: The returned pointer will be valid only for the lifetime of this
@@ -50,11 +56,18 @@ class UnindexedRulesetStreamGenerator {
   // Generates |ruleset_stream_| from the file at |ruleset_path_|.
   void GenerateStreamFromFile(base::FilePath ruleset_path);
 
+  // Generates |ruleset_stream_| from the contents of the string stored in the
+  // resource bundle at |resource_id|.
+  void GenerateStreamFromResourceId(int resource_id);
+
   int64_t ruleset_size_ = -1;
 
   // Used when the stream is generated from a file on disk.
   std::unique_ptr<CopyingFileInputStream> copying_stream_;
 
+  // Used when the stream is generated from a resource ID.
+  std::istringstream string_stream_;
+
   // The stream via which a client of this class can read the data of the
   // unindexed ruleset.
   std::unique_ptr<google::protobuf::io::ZeroCopyInputStream> ruleset_stream_;

ui/base/resource/mock_resource_bundle_delegate.h

@@ -26,6 +26,8 @@ class MockResourceBundleDelegate : public ResourceBundle::Delegate {
   MOCK_METHOD2(LoadDataResourceBytes,
                base::RefCountedMemory*(int resource_id,
                                        ScaleFactor scale_factor));
+  MOCK_METHOD1(LoadDataResourceString,
+               base::Optional<std::string>(int resource_id));
   MOCK_CONST_METHOD3(GetRawDataResource,
                      bool(int resource_id,
                           ScaleFactor scale_factor,

ui/base/resource/resource_bundle.cc

@@ -659,6 +659,13 @@ base::StringPiece ResourceBundle::GetRawDataResourceForScale(
 }
 
 std::string ResourceBundle::LoadDataResourceString(int resource_id) const {
+  if (delegate_) {
+    base::Optional<std::string> data =
+        delegate_->LoadDataResourceString(resource_id);
+    if (data)
+      return data.value();
+  }
+
   return LoadDataResourceStringForScale(resource_id, ui::SCALE_FACTOR_NONE);
 }
 

ui/base/resource/resource_bundle.h

@@ -18,6 +18,7 @@
 #include "base/files/memory_mapped_file.h"
 #include "base/gtest_prod_util.h"
 #include "base/macros.h"
+#include "base/optional.h"
 #include "base/sequence_checker.h"
 #include "base/strings/string16.h"
 #include "base/strings/string_piece.h"
@@ -73,6 +74,8 @@ class COMPONENT_EXPORT(UI_BASE) ResourceBundle {
 
   // Delegate class that allows interception of pack file loading and resource
   // requests. The methods of this class may be called on multiple threads.
+  // TODO(crbug.com/1146446): The interface and usage model of this class are
+  // clunky; it would be good to clean them up.
   class Delegate {
    public:
     // Called before a resource pack file is loaded. Return the full path for
@@ -105,6 +108,14 @@ class COMPONENT_EXPORT(UI_BASE) ResourceBundle {
         int resource_id,
         ScaleFactor scale_factor) = 0;
 
+    // Supports intercepting of ResourceBundle::LoadDataResourceString(): Return
+    // a populated base::Optional instance to override the value that
+    // ResourceBundle::LoadDataResourceString() would return by default, or an
+    // empty base::Optional instance to pass through to the default behavior of
+    // ResourceBundle::LoadDataResourceString().
+    virtual base::Optional<std::string> LoadDataResourceString(
+        int resource_id) = 0;
+
     // Retrieve a raw data resource. Return true if a resource was provided or
     // false to attempt retrieval of the default resource.
     virtual bool GetRawDataResource(int resource_id,