Worker: Fix DCHECK failures during WorkerFetchContext creation

This CL removes a thread check in
WebDocumentSubresourceFilterImpl::BuilderImpl::Build() that is called during
WorkerFetchContext creation.

When the check was added, Build() was supposed to be called from the worker
thread. However, PaintWorklet, which is a kind of workers but runs on the main
thread, was implemented and broke the assumption. It should be safe to run it on
the main thread, so this CL removes the check.

This CL also adds tests for subresource filter for worklets to check the change.

Bug: 887269, 1011208
Change-Id: Ieaa0ed4adb80523ec72af5916781c8593de28c85
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1242730Reviewed-by: Tsuyoshi Horo <horo@chromium.org>
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Cr-Commit-Position: refs/heads/master@{#706596}

chrome/browser/subresource_filter/subresource_filter_worker_browsertest.cc

@@ -4,20 +4,17 @@
 
 #include <vector>
 
-#include "base/feature_list.h"
 #include "base/macros.h"
 #include "base/strings/string16.h"
 #include "base/strings/string_piece.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
-#include "base/test/scoped_feature_list.h"
 #include "chrome/browser/subresource_filter/subresource_filter_browser_test_harness.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "content/public/browser/web_contents.h"
-#include "content/public/common/content_features.h"
 #include "content/public/test/browser_test.h"
 #include "content/public/test/browser_test_utils.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -28,61 +25,83 @@ namespace subresource_filter {
 class SubresourceFilterWorkerFetchBrowserTest
     : public SubresourceFilterBrowserTest {
  public:
-  SubresourceFilterWorkerFetchBrowserTest() {}
-
-  ~SubresourceFilterWorkerFetchBrowserTest() override {}
+  SubresourceFilterWorkerFetchBrowserTest() = default;
+  ~SubresourceFilterWorkerFetchBrowserTest() override = default;
 
  protected:
+  void RunTest(const std::string& document_path,
+               const std::string& filter_path) {
+    const base::string16 fetch_succeeded_title =
+        base::ASCIIToUTF16("FetchSucceeded");
+    const base::string16 fetch_failed_title = base::ASCIIToUTF16("FetchFailed");
+    const base::string16 fetch_partially_failed_title =
+        base::ASCIIToUTF16("FetchPartiallyFailed");
+
+    GURL url(GetTestUrl(document_path));
+    ConfigureAsPhishingURL(url);
+
+    // This unrelated rule shouldn't block fetch.
+    ASSERT_NO_FATAL_FAILURE(SetRulesetToDisallowURLsWithPathSuffix(
+        "suffix-that-does-not-match-anything"));
+    {
+      content::TitleWatcher title_watcher(
+          browser()->tab_strip_model()->GetActiveWebContents(),
+          fetch_succeeded_title);
+      title_watcher.AlsoWaitForTitle(fetch_failed_title);
+      title_watcher.AlsoWaitForTitle(fetch_partially_failed_title);
+      ui_test_utils::NavigateToURL(browser(), url);
+      EXPECT_EQ(fetch_succeeded_title, title_watcher.WaitAndGetTitle());
+    }
+    ClearTitle();
+
+    // This rule should block fetch.
+    ASSERT_NO_FATAL_FAILURE(
+        SetRulesetToDisallowURLsWithPathSuffix(filter_path));
+    {
+      content::TitleWatcher title_watcher(
+          browser()->tab_strip_model()->GetActiveWebContents(),
+          fetch_succeeded_title);
+      title_watcher.AlsoWaitForTitle(fetch_failed_title);
+      title_watcher.AlsoWaitForTitle(fetch_partially_failed_title);
+      ui_test_utils::NavigateToURL(browser(), url);
+      EXPECT_EQ(fetch_failed_title, title_watcher.WaitAndGetTitle());
+    }
+    ClearTitle();
+  }
+
   void ClearTitle() {
     ASSERT_TRUE(content::ExecuteScript(web_contents()->GetMainFrame(),
                                        "document.title = \"\";"));
   }
 
  private:
-  base::test::ScopedFeatureList scoped_feature_list_;
-
   DISALLOW_COPY_AND_ASSIGN(SubresourceFilterWorkerFetchBrowserTest);
 };
 
+// TODO(https://crbug.com/1011208): Add more tests for workers like top-level
+// worker script fetch and module script fetch.
+
+// Test if fetch() on dedicated workers is blocked by the subresource filter.
 IN_PROC_BROWSER_TEST_F(SubresourceFilterWorkerFetchBrowserTest, WorkerFetch) {
-  const base::string16 fetch_succeeded_title =
-      base::ASCIIToUTF16("FetchSucceeded");
-  const base::string16 fetch_failed_title = base::ASCIIToUTF16("FetchFailed");
-  GURL url(GetTestUrl("subresource_filter/worker_fetch.html"));
-  ConfigureAsPhishingURL(url);
-
-  ASSERT_NO_FATAL_FAILURE(SetRulesetToDisallowURLsWithPathSuffix(
-      "suffix-that-does-not-match-anything"));
-  {
-    content::TitleWatcher title_watcher(
-        browser()->tab_strip_model()->GetActiveWebContents(),
-        fetch_succeeded_title);
-    title_watcher.AlsoWaitForTitle(fetch_failed_title);
-    ui_test_utils::NavigateToURL(browser(), url);
-    EXPECT_EQ(fetch_succeeded_title, title_watcher.WaitAndGetTitle());
-  }
-  ClearTitle();
-  ASSERT_NO_FATAL_FAILURE(
-      SetRulesetToDisallowURLsWithPathSuffix("worker_fetch_data.txt"));
-  {
-    content::TitleWatcher title_watcher(
-        browser()->tab_strip_model()->GetActiveWebContents(),
-        fetch_succeeded_title);
-    title_watcher.AlsoWaitForTitle(fetch_failed_title);
-    ui_test_utils::NavigateToURL(browser(), url);
-    EXPECT_EQ(fetch_failed_title, title_watcher.WaitAndGetTitle());
-  }
-  ClearTitle();
-  // The main frame document should never be filtered.
-  SetRulesetToDisallowURLsWithPathSuffix("worker_fetch.html");
-  {
-    content::TitleWatcher title_watcher(
-        browser()->tab_strip_model()->GetActiveWebContents(),
-        fetch_succeeded_title);
-    title_watcher.AlsoWaitForTitle(fetch_failed_title);
-    ui_test_utils::NavigateToURL(browser(), url);
-    EXPECT_EQ(fetch_succeeded_title, title_watcher.WaitAndGetTitle());
-  }
+  // This fetches "worklet_fetch_data.txt" by fetch().
+  RunTest("subresource_filter/worker_fetch.html", "worker_fetch_data.txt");
+}
+
+// Test if top-level worklet script fetch is blocked by the subresource filter.
+IN_PROC_BROWSER_TEST_F(SubresourceFilterWorkerFetchBrowserTest,
+                       WorkletScriptFetch) {
+  RunTest("subresource_filter/worklet_script_fetch.html",
+          "worklet_script_fetch.js");
+}
+
+// Test if static import on worklets is blocked by the subresource filter.
+IN_PROC_BROWSER_TEST_F(SubresourceFilterWorkerFetchBrowserTest,
+                       WorkletStaticImport) {
+  // This fetches "empty.js" by static import.
+  RunTest("subresource_filter/worklet_script_fetch.html", "empty.js");
 }
 
+// Any network APIs including dynamic import are disallowed on worklets, so we
+// don't have to test them.
+
 }  // namespace subresource_filter

chrome/test/data/subresource_filter/empty.js

+// Copyright 2019 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.
+
+// Do nothing.

chrome/test/data/subresource_filter/worklet_script_fetch.html

+<script>
+const scriptURL = 'worklet_script_fetch.js';
+
+// Test AudioWorklet.
+const audioContext = new OfflineAudioContext(2,44100*40,44100);
+const audioWorkletPromise = audioContext.audioWorklet.addModule(scriptURL);
+
+// Test PaintWorklet.
+const paintWorkletPromise = CSS.paintWorklet.addModule(scriptURL);
+
+// TODO(https://crbug.com/1011208): Test AnimationWorklet and LayoutWorklet.
+
+const promises = [audioWorkletPromise, paintWorkletPromise];
+Promise.allSettled(promises)
+  .then(results => {
+    // The subresource filter should pass or block them all.
+    const numOfSuccess = results.filter(r => r.status === 'fulfilled').length;
+    if (numOfSuccess === promises.length)
+      document.title = 'FetchSucceeded';
+    else if (numOfSuccess === 0)
+      document.title = 'FetchFailed';
+    else
+      document.title = 'FetchPartiallyFailed';
+  });
+</script>

chrome/test/data/subresource_filter/worklet_script_fetch.js

+// Copyright 2019 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.
+
+import './empty.js';

components/subresource_filter/content/renderer/web_document_subresource_filter_impl.cc

@@ -163,7 +163,6 @@ WebDocumentSubresourceFilterImpl::BuilderImpl::~BuilderImpl() {}
 std::unique_ptr<blink::WebDocumentSubresourceFilter>
 WebDocumentSubresourceFilterImpl::BuilderImpl::Build() {
   DCHECK(ruleset_file_.IsValid());
-  DCHECK(!main_task_runner_->BelongsToCurrentThread());
   scoped_refptr<MemoryMappedRuleset> ruleset =
       MemoryMappedRuleset::CreateAndInitialize(std::move(ruleset_file_));
   if (!ruleset)

components/subresource_filter/content/renderer/web_document_subresource_filter_impl.h

@@ -24,8 +24,10 @@ class WebDocumentSubresourceFilterImpl
     : public blink::WebDocumentSubresourceFilter,
       public base::SupportsWeakPtr<WebDocumentSubresourceFilterImpl> {
  public:
-  // This builder class is created on the main thread and passed to a worker
-  // thread to create the subresource filter for the worker thread.
+  // This builder class is used for creating the subresource filter for workers
+  // and worklets. For workers and threaded worklets, this is created on the
+  // main thread and passed to a worker thread. For main thread worklets, this
+  // is created and used on the main thread.
   class BuilderImpl : public blink::WebDocumentSubresourceFilter::Builder {
    public:
     BuilderImpl(url::Origin document_origin,