From 979a039b65280f4c6d244154f618499b47edc82e Mon Sep 17 00:00:00 2001 From: Simone Arpe Date: Wed, 20 Nov 2024 17:20:48 +0100 Subject: [PATCH] Single rule implementation refactoring --- .../browser/core/youtube_rule.cc | 46 ++++--------------- .../browser/core/youtube_rule.h | 6 +-- .../browser/core/youtube_rule_registry.cc | 30 +++++------- .../browser/core/youtube_rule_registry.h | 2 +- 4 files changed, 25 insertions(+), 59 deletions(-) diff --git a/components/youtube_script_injector/browser/core/youtube_rule.cc b/components/youtube_script_injector/browser/core/youtube_rule.cc index f2235c358622..27af6a82a01a 100644 --- a/components/youtube_script_injector/browser/core/youtube_rule.cc +++ b/components/youtube_script_injector/browser/core/youtube_rule.cc @@ -22,10 +22,8 @@ namespace { // youtube.json keys -// constexpr char kInclude[] = "include"; -// constexpr char kExclude[] = "exclude"; constexpr char kVersion[] = "version"; -constexpr char kPolicyScript[] = "policy_script"; +constexpr char kFeatureScript[] = "feature_script"; constexpr char kYouTubeUrl[] = "https://youtube.com"; @@ -59,26 +57,20 @@ namespace youtube_script_injector { YouTubeRule::YouTubeRule() = default; YouTubeRule::~YouTubeRule() = default; YouTubeRule::YouTubeRule(const YouTubeRule& other) { - // include_pattern_set_ = other.include_pattern_set_.Clone(); - // exclude_pattern_set_ = other.exclude_pattern_set_.Clone(); - policy_script_path_ = other.policy_script_path_; + feature_script_path_ = other.feature_script_path_; version_ = other.version_; } // static void YouTubeRule::RegisterJSONConverter( base::JSONValueConverter* converter) { - // converter->RegisterCustomValueField( - // kInclude, &YouTubeRule::include_pattern_set_, GetURLPatternSetFromValue); - // converter->RegisterCustomValueField( - // kExclude, &YouTubeRule::exclude_pattern_set_, GetURLPatternSetFromValue); converter->RegisterCustomValueField( - kPolicyScript, &YouTubeRule::policy_script_path_, GetFilePathFromValue); + kFeatureScript, &YouTubeRule::feature_script_path_, GetFilePathFromValue); converter->RegisterIntField(kVersion, &YouTubeRule::version_); } // static -std::optional> YouTubeRule::ParseRules( +std::optional YouTubeRule::ParseRules( const std::string& contents) { if (contents.empty()) { return std::nullopt; @@ -88,17 +80,14 @@ std::optional> YouTubeRule::ParseRules( VLOG(1) << "YouTubeRule::ParseRules: invalid JSON"; return std::nullopt; } - std::vector rules; + + YouTubeRule rule = YouTubeRule(); base::JSONValueConverter converter; - for (base::Value& it : root->GetList()) { - YouTubeRule rule = YouTubeRule(); - if (!converter.Convert(it, &rule)) { - VLOG(1) << "YouTubeRule::ParseRules: invalid rule"; - continue; - } - rules.emplace_back(rule); + if (!converter.Convert(*root, &rule)) { + VLOG(1) << "YouTubeRule::ParseRules: invalid rule"; + return std::nullopt; } - return rules; + return rule; } bool YouTubeRule::IsYouTubeDomain(const GURL& url) const { @@ -111,19 +100,4 @@ bool YouTubeRule::IsYouTubeDomain(const GURL& url) const { return false; } -// bool YouTubeRule::ShouldInsertScript(const GURL& url) const { -// // If URL matches an explicitly excluded pattern, this rule does not -// // apply. -// if (exclude_pattern_set_.MatchesURL(url)) { -// return false; -// } -// // If URL does not match an explicitly included pattern, this rule does not -// // apply. -// if (!include_pattern_set_.MatchesURL(url)) { -// return false; -// } - -// return true; -// } - } // namespace youtube_script_injector diff --git a/components/youtube_script_injector/browser/core/youtube_rule.h b/components/youtube_script_injector/browser/core/youtube_rule.h index 55816ad60fa6..c10c630e9a2a 100644 --- a/components/youtube_script_injector/browser/core/youtube_rule.h +++ b/components/youtube_script_injector/browser/core/youtube_rule.h @@ -55,21 +55,21 @@ class YouTubeRule { base::JSONValueConverter* converter); // Parse the youtube.json file contents into a vector of YouTubeRule. - static std::optional> ParseRules( + static std::optional ParseRules( const std::string& contents); // Check if this rule matches the given URL. // bool ShouldInsertScript(const GURL& url) const; bool IsYouTubeDomain(const GURL& url) const; // Getters. - const base::FilePath& GetPolicyScript() const { return policy_script_path_; } + const base::FilePath& GetFeatureScript() const { return feature_script_path_; } int GetVersion() const { return version_; } private: // extensions::URLPatternSet include_pattern_set_; // extensions::URLPatternSet exclude_pattern_set_; // This is a path (not content) relative to the component under scripts/. - base::FilePath policy_script_path_; + base::FilePath feature_script_path_; // Used for checking if the last inserted script is the latest version. int version_; }; diff --git a/components/youtube_script_injector/browser/core/youtube_rule_registry.cc b/components/youtube_script_injector/browser/core/youtube_rule_registry.cc index 0d6624a7f79d..ef6d52c5ad76 100644 --- a/components/youtube_script_injector/browser/core/youtube_rule_registry.cc +++ b/components/youtube_script_injector/browser/core/youtube_rule_registry.cc @@ -68,17 +68,13 @@ YouTubeRuleRegistry::~YouTubeRuleRegistry() = default; void YouTubeRuleRegistry::CheckIfMatch( const GURL& url, base::OnceCallback cb) const { - for (const YouTubeRule& rule : rules_) { - if (rule.IsYouTubeDomain(url)) { - base::ThreadPool::PostTaskAndReplyWithResult( - FROM_HERE, {base::MayBlock()}, - base::BindOnce(&CreateMatchedRule, component_path_, - rule.GetPolicyScript(), - rule.GetVersion()), - std::move(cb)); - // Only ever find one matching rule. - return; - } + if (rule_ && rule_->IsYouTubeDomain(url)) { + base::ThreadPool::PostTaskAndReplyWithResult( + FROM_HERE, {base::MayBlock()}, + base::BindOnce(&CreateMatchedRule, component_path_, + rule_->GetFeatureScript(), + rule_->GetVersion()), + std::move(cb)); } } @@ -91,16 +87,12 @@ void YouTubeRuleRegistry::LoadRules(const base::FilePath& path) { weak_factory_.GetWeakPtr())); } -void YouTubeRuleRegistry::SetComponentPath(const base::FilePath& path) { - component_path_ = path; +void YouTubeRuleRegistry::OnLoadRules(const std::string& contents) { + rule_ = YouTubeRule::ParseRules(contents); } -void YouTubeRuleRegistry::OnLoadRules(const std::string& contents) { - auto parsed_rules = YouTubeRule::ParseRules(contents); - if (!parsed_rules) { - return; - } - rules_ = std::move(parsed_rules.value()); +void YouTubeRuleRegistry::SetComponentPath(const base::FilePath& path) { + component_path_ = path; } } // namespace youtube_script_injector diff --git a/components/youtube_script_injector/browser/core/youtube_rule_registry.h b/components/youtube_script_injector/browser/core/youtube_rule_registry.h index c2a7a8001d17..f814789cb7f7 100644 --- a/components/youtube_script_injector/browser/core/youtube_rule_registry.h +++ b/components/youtube_script_injector/browser/core/youtube_rule_registry.h @@ -50,7 +50,7 @@ class COMPONENT_EXPORT(YOUTUBE_SCRIPT_INJECTOR_BROWSER_CORE) YouTubeRuleRegistry void SetComponentPath(const base::FilePath& path); base::FilePath component_path_; - std::vector rules_; + std::optional rule_; base::WeakPtrFactory weak_factory_{this};