Merge pull request #10198 from brave/pr10192_mitigate-scriptlet-injec…

…tion-races_1.31.x

Mitigate scriptlet injection races (uplift to 1.31.x)

components/cosmetic_filters/browser/cosmetic_filters_resources.cc

@@ -80,32 +80,35 @@ void CosmeticFiltersResources::HiddenClassIdSelectorsOnUI(
 }
 
 void CosmeticFiltersResources::UrlCosmeticResourcesOnUI(
-    UrlCosmeticResourcesCallback callback,
+    base::OnceCallback<void(base::Value)> callback,
     absl::optional<base::Value> resources) {
   std::move(callback).Run(resources ? std::move(resources.value())
                                     : base::Value());
 }
 
-void CosmeticFiltersResources::ShouldDoCosmeticFiltering(
+void CosmeticFiltersResources::UrlCosmeticResources(
     const std::string& url,
-    ShouldDoCosmeticFilteringCallback callback) {
+    UrlCosmeticResourcesCallback callback) {
   bool enabled =
       brave_shields::ShouldDoCosmeticFiltering(settings_map_, GURL(url));
+
+  if (!enabled) {
+    std::move(callback).Run(enabled, false, base::Value());
+    return;
+  }
+
   bool first_party_enabled =
       brave_shields::IsFirstPartyCosmeticFilteringEnabled(settings_map_,
                                                           GURL(url));
-  std::move(callback).Run(enabled, first_party_enabled);
-}
 
-void CosmeticFiltersResources::UrlCosmeticResources(
-    const std::string& url,
-    UrlCosmeticResourcesCallback callback) {
   ad_block_service_->GetTaskRunner()->PostTaskAndReplyWithResult(
       FROM_HERE,
       base::BindOnce(&brave_shields::AdBlockService::UrlCosmeticResources,
                      base::Unretained(ad_block_service_), url),
-      base::BindOnce(&CosmeticFiltersResources::UrlCosmeticResourcesOnUI,
-                     weak_factory_.GetWeakPtr(), std::move(callback)));
+      base::BindOnce(
+          &CosmeticFiltersResources::UrlCosmeticResourcesOnUI,
+          weak_factory_.GetWeakPtr(),
+          base::BindOnce(std::move(callback), enabled, first_party_enabled)));
 }
 
 }  // namespace cosmetic_filters

components/cosmetic_filters/browser/cosmetic_filters_resources.h

@@ -10,6 +10,7 @@
 #include <string>
 #include <vector>
 
+#include "base/callback.h"
 #include "base/memory/weak_ptr.h"
 #include "base/values.h"
 #include "brave/components/cosmetic_filters/common/cosmetic_filters.mojom.h"
@@ -35,28 +36,25 @@ class CosmeticFiltersResources final
                            brave_shields::AdBlockService* ad_block_service);
   ~CosmeticFiltersResources() override;
 
-  // Sends back to renderer a response: do we need to apply cosmetic filters
-  // for the url.
-  void ShouldDoCosmeticFiltering(
-      const std::string& url,
-      ShouldDoCosmeticFilteringCallback callback) override;
-
   // Sends back to renderer a response about rules that has to be applied
   // for the specified selectors.
   void HiddenClassIdSelectors(const std::string& input,
                               const std::vector<std::string>& exceptions,
                               HiddenClassIdSelectorsCallback callback) override;
 
-  // Sends back to renderer a response what rules and scripts has to be
-  // applied for the specified url.
+  // If cosmetic filtering is enabled, sends the renderer a response including
+  // whether or not to apply cosmetic filtering to first party elements along
+  // with an initial set of rules and scripts to apply for the given URL.
+  // If cosmetic filtering is disabled, `enabled` will be set to false and the
+  // other return values should be ignored.
   void UrlCosmeticResources(const std::string& url,
                             UrlCosmeticResourcesCallback callback) override;
 
  private:
   void HiddenClassIdSelectorsOnUI(HiddenClassIdSelectorsCallback callback,
                                   absl::optional<base::Value> resources);
 
-  void UrlCosmeticResourcesOnUI(UrlCosmeticResourcesCallback callback,
+  void UrlCosmeticResourcesOnUI(base::OnceCallback<void(base::Value)> callback,
                                 absl::optional<base::Value> resources);
 
   HostContentSettingsMap* settings_map_;             // Not owned

components/cosmetic_filters/common/cosmetic_filters.mojom

@@ -3,10 +3,11 @@ module cosmetic_filters.mojom;
 import "mojo/public/mojom/base/values.mojom";
 
 interface CosmeticFiltersResources {
-  ShouldDoCosmeticFiltering(string url) => (bool enabled,
-                                            bool first_party_enabled);
-  UrlCosmeticResources(string url) => (mojo_base.mojom.Value result);
   // Receives an input string which is JSON object.
   HiddenClassIdSelectors(string input, array<string> exceptions) => (
       mojo_base.mojom.Value result);
+
+  UrlCosmeticResources(string url) => (bool enabled,
+                                       bool first_party_enabled,
+                                       mojo_base.mojom.Value result);
 };

components/cosmetic_filters/renderer/cosmetic_filters_js_handler.cc

@@ -305,20 +305,6 @@ void CosmeticFiltersJSHandler::ProcessURL(const GURL& url,
   if (!EnsureConnected() || url_.is_empty() || !url_.is_valid())
     return;
 
-  cosmetic_filters_resources_->ShouldDoCosmeticFiltering(
-      url_.spec(),
-      base::BindOnce(&CosmeticFiltersJSHandler::OnShouldDoCosmeticFiltering,
-                     base::Unretained(this), std::move(callback)));
-}
-
-void CosmeticFiltersJSHandler::OnShouldDoCosmeticFiltering(
-    base::OnceClosure callback,
-    bool enabled,
-    bool first_party_enabled) {
-  if (!enabled || !EnsureConnected())
-    return;
-
-  enabled_1st_party_cf_ = first_party_enabled;
   cosmetic_filters_resources_->UrlCosmeticResources(
       url_.spec(),
       base::BindOnce(&CosmeticFiltersJSHandler::OnUrlCosmeticResources,
@@ -327,7 +313,12 @@ void CosmeticFiltersJSHandler::OnShouldDoCosmeticFiltering(
 
 void CosmeticFiltersJSHandler::OnUrlCosmeticResources(
     base::OnceClosure callback,
+    bool enabled,
+    bool first_party_enabled,
     base::Value result) {
+  if (!enabled || !EnsureConnected())
+    return;
+  enabled_1st_party_cf_ = first_party_enabled;
   resources_dict_ = base::DictionaryValue::From(
       base::Value::ToUniquePtrValue(std::move(result)));
   std::move(callback).Run();

components/cosmetic_filters/renderer/cosmetic_filters_js_handler.h

@@ -57,10 +57,10 @@ class CosmeticFiltersJSHandler {
   // A function to be called from JS
   void HiddenClassIdSelectors(const std::string& input);
 
-  void OnShouldDoCosmeticFiltering(base::OnceClosure callback,
-                                   bool enabled,
-                                   bool first_party_enabled);
-  void OnUrlCosmeticResources(base::OnceClosure callback, base::Value result);
+  void OnUrlCosmeticResources(base::OnceClosure callback,
+                              bool enabled,
+                              bool first_party_enabled,
+                              base::Value result);
   void CSSRulesRoutine(base::DictionaryValue* resources_dict);
   void OnHiddenClassIdSelectors(base::Value result);
   bool OnIsFirstParty(const std::string& url_string);

components/cosmetic_filters/renderer/cosmetic_filters_js_render_frame_observer.cc

@@ -84,9 +84,14 @@ void CosmeticFiltersJsRenderFrameObserver::ReadyToCommitNavigation(
 }
 
 void CosmeticFiltersJsRenderFrameObserver::RunScriptsAtDocumentStart() {
-  ready_->Post(FROM_HERE,
-               base::BindOnce(&CosmeticFiltersJsRenderFrameObserver::ApplyRules,
-                              weak_factory_.GetWeakPtr()));
+  if (ready_->is_signaled()) {
+    ApplyRules();
+  } else {
+    ready_->Post(
+        FROM_HERE,
+        base::BindOnce(&CosmeticFiltersJsRenderFrameObserver::ApplyRules,
+                       weak_factory_.GetWeakPtr()));
+  }
 }
 
 void CosmeticFiltersJsRenderFrameObserver::ApplyRules() {