From b419b06e3ce1d3fe3aed7dc63c49af204a0455ee Mon Sep 17 00:00:00 2001 From: Simone Arpe Date: Wed, 20 Nov 2024 18:22:49 +0100 Subject: [PATCH] Introduce better class names --- .../browser/content/youtube_tab_helper.cc | 10 +++--- .../browser/content/youtube_tab_helper.h | 6 ++-- .../browser/core/BUILD.gn | 8 ++--- .../core/youtube_component_installer.cc | 12 +++---- ...e_rule_registry.cc => youtube_registry.cc} | 30 ++++++++-------- ...ube_rule_registry.h => youtube_registry.h} | 36 +++++++++---------- .../{youtube_rule.cc => youtube_script.cc} | 28 +++++++-------- .../core/{youtube_rule.h => youtube_script.h} | 36 +++++++------------ 8 files changed, 77 insertions(+), 89 deletions(-) rename components/youtube_script_injector/browser/core/{youtube_rule_registry.cc => youtube_registry.cc} (77%) rename components/youtube_script_injector/browser/core/{youtube_rule_registry.h => youtube_registry.h} (72%) rename components/youtube_script_injector/browser/core/{youtube_rule.cc => youtube_script.cc} (75%) rename components/youtube_script_injector/browser/core/{youtube_rule.h => youtube_script.h} (69%) diff --git a/components/youtube_script_injector/browser/content/youtube_tab_helper.cc b/components/youtube_script_injector/browser/content/youtube_tab_helper.cc index 9a8e3ec49463..865130b7f8a1 100644 --- a/components/youtube_script_injector/browser/content/youtube_tab_helper.cc +++ b/components/youtube_script_injector/browser/content/youtube_tab_helper.cc @@ -12,8 +12,8 @@ #include "base/strings/utf_string_conversions.h" #include "base/task/thread_pool.h" #include "base/values.h" -#include "brave/components/youtube_script_injector/browser/core/youtube_rule.h" -#include "brave/components/youtube_script_injector/browser/core/youtube_rule_registry.h" +#include "brave/components/youtube_script_injector/browser/core/youtube_script.h" +#include "brave/components/youtube_script_injector/browser/core/youtube_registry.h" #include "brave/components/youtube_script_injector/common/features.h" #include "components/sessions/content/session_tab_helper.h" #include "content/public/browser/browser_context.h" @@ -41,8 +41,8 @@ YouTubeTabHelper::YouTubeTabHelper(content::WebContents* web_contents, : WebContentsObserver(web_contents), content::WebContentsUserData(*web_contents), world_id_(world_id), - youtube_rule_registry_(YouTubeRuleRegistry::GetInstance()) { - DCHECK(youtube_rule_registry_); + youtube_registry_(YouTubeRegistry::GetInstance()) { + DCHECK(youtube_registry_); } YouTubeTabHelper::~YouTubeTabHelper() = default; @@ -91,7 +91,7 @@ void YouTubeTabHelper::DidFinishNavigation( content::GlobalRenderFrameHostId render_frame_host_id = web_contents()->GetPrimaryMainFrame()->GetGlobalId(); - youtube_rule_registry_->CheckIfMatch( + youtube_registry_->CheckIfMatch( url, base::BindOnce(&YouTubeTabHelper::InsertScriptInPage, weak_factory_.GetWeakPtr(), render_frame_host_id)); } diff --git a/components/youtube_script_injector/browser/content/youtube_tab_helper.h b/components/youtube_script_injector/browser/content/youtube_tab_helper.h index 1cdbe11a96cf..9cde57f579e5 100644 --- a/components/youtube_script_injector/browser/content/youtube_tab_helper.h +++ b/components/youtube_script_injector/browser/content/youtube_tab_helper.h @@ -11,7 +11,7 @@ #include "base/component_export.h" #include "base/memory/raw_ptr.h" #include "base/memory/weak_ptr.h" -#include "brave/components/youtube_script_injector/browser/core/youtube_rule.h" +#include "brave/components/youtube_script_injector/browser/core/youtube_script.h" #include "brave/components/script_injector/common/mojom/script_injector.mojom.h" #include "build/build_config.h" #include "components/sessions/core/session_id.h" @@ -22,7 +22,7 @@ namespace youtube_script_injector { -class YouTubeRuleRegistry; +class YouTubeRegistry; // Used to inject JS scripts into the page. class COMPONENT_EXPORT(YOUTUBE_SCRIPT_INJECTOR_BROWSER_CONTENT) YouTubeTabHelper @@ -50,7 +50,7 @@ class COMPONENT_EXPORT(YOUTUBE_SCRIPT_INJECTOR_BROWSER_CONTENT) YouTubeTabHelper content::NavigationHandle* navigation_handle) override; const int32_t world_id_; - const raw_ptr youtube_rule_registry_; // NOT OWNED + const raw_ptr youtube_registry_; // NOT OWNED // The remote used to send the script to the renderer. mojo::AssociatedRemote script_injector_remote_; diff --git a/components/youtube_script_injector/browser/core/BUILD.gn b/components/youtube_script_injector/browser/core/BUILD.gn index 4b20f898a0a4..12c45af9256a 100644 --- a/components/youtube_script_injector/browser/core/BUILD.gn +++ b/components/youtube_script_injector/browser/core/BUILD.gn @@ -11,10 +11,10 @@ component("core") { sources = [ "youtube_component_installer.cc", "youtube_component_installer.h", - "youtube_rule.cc", - "youtube_rule.h", - "youtube_rule_registry.cc", - "youtube_rule_registry.h", + "youtube_script.cc", + "youtube_script.h", + "youtube_registry.cc", + "youtube_registry.h", ] deps = [ diff --git a/components/youtube_script_injector/browser/core/youtube_component_installer.cc b/components/youtube_script_injector/browser/core/youtube_component_installer.cc index 1b62cc769647..f6649046dd36 100644 --- a/components/youtube_script_injector/browser/core/youtube_component_installer.cc +++ b/components/youtube_script_injector/browser/core/youtube_component_installer.cc @@ -16,7 +16,7 @@ #include "base/functional/callback_forward.h" #include "base/no_destructor.h" #include "brave/components/brave_component_updater/browser/brave_on_demand_updater.h" -#include "brave/components/youtube_script_injector/browser/core/youtube_rule_registry.h" +#include "brave/components/youtube_script_injector/browser/core/youtube_registry.h" #include "brave/components/youtube_script_injector/common/features.h" #include "components/component_updater/component_installer.h" #include "components/component_updater/component_updater_service.h" @@ -31,11 +31,9 @@ namespace { // |_ manifest.json // |_ youtube.json // |_ scripts/ -// |_ keep-playing-audio/ -// |_ policy.js -// |_ set-fullscreen/ -// |_ policy.js -// See youtube_rule.cc for the format of youtube.json. +// |_ keep-playing-audio.js +// |_ fullscreen.js +// See youtube_script.cc for the format of youtube.json. constexpr size_t kHashSize = 32; constexpr char kYouTubeComponentName[] = @@ -114,7 +112,7 @@ void YouTubeComponentInstallerPolicy::OnCustomUninstall() {} void YouTubeComponentInstallerPolicy::ComponentReady(const base::Version& version, const base::FilePath& path, base::Value::Dict manifest) { - YouTubeRuleRegistry::GetInstance()->LoadRules(path); + YouTubeRegistry::GetInstance()->LoadScripts(path); } bool YouTubeComponentInstallerPolicy::VerifyInstallation( diff --git a/components/youtube_script_injector/browser/core/youtube_rule_registry.cc b/components/youtube_script_injector/browser/core/youtube_registry.cc similarity index 77% rename from components/youtube_script_injector/browser/core/youtube_rule_registry.cc rename to components/youtube_script_injector/browser/core/youtube_registry.cc index ef6d52c5ad76..199dee7d5fc6 100644 --- a/components/youtube_script_injector/browser/core/youtube_rule_registry.cc +++ b/components/youtube_script_injector/browser/core/youtube_registry.cc @@ -3,7 +3,7 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this file, // You can obtain one at https://mozilla.org/MPL/2.0/. -#include "brave/components/youtube_script_injector/browser/core/youtube_rule_registry.h" +#include "brave/components/youtube_script_injector/browser/core/youtube_registry.h" #include #include @@ -18,7 +18,7 @@ #include "base/logging.h" #include "base/memory/singleton.h" #include "base/task/thread_pool.h" -#include "brave/components/youtube_script_injector/browser/core/youtube_rule.h" +#include "brave/components/youtube_script_injector/browser/core/youtube_script.h" #include "brave/components/youtube_script_injector/common/features.h" #include "net/base/registry_controlled_domains/registry_controlled_domain.h" #include "url/gurl.h" @@ -53,45 +53,45 @@ MatchedRule CreateMatchedRule(const base::FilePath& component_path, } // namespace // static -YouTubeRuleRegistry* YouTubeRuleRegistry::GetInstance() { +YouTubeRegistry* YouTubeRegistry::GetInstance() { // Check if feature flag is enabled. if (!base::FeatureList::IsEnabled(youtube_script_injector::features::kBraveYouTubeScriptInjector)) { return nullptr; } - return base::Singleton::get(); + return base::Singleton::get(); } -YouTubeRuleRegistry::YouTubeRuleRegistry() = default; +YouTubeRegistry::YouTubeRegistry() = default; -YouTubeRuleRegistry::~YouTubeRuleRegistry() = default; +YouTubeRegistry::~YouTubeRegistry() = default; -void YouTubeRuleRegistry::CheckIfMatch( +void YouTubeRegistry::CheckIfMatch( const GURL& url, base::OnceCallback cb) const { - if (rule_ && rule_->IsYouTubeDomain(url)) { + if (script_ && script_->IsYouTubeDomain(url)) { base::ThreadPool::PostTaskAndReplyWithResult( FROM_HERE, {base::MayBlock()}, base::BindOnce(&CreateMatchedRule, component_path_, - rule_->GetFeatureScript(), - rule_->GetVersion()), + script_->GetFeatureScript(), + script_->GetVersion()), std::move(cb)); } } -void YouTubeRuleRegistry::LoadRules(const base::FilePath& path) { +void YouTubeRegistry::LoadScripts(const base::FilePath& path) { SetComponentPath(path); base::ThreadPool::PostTaskAndReplyWithResult( FROM_HERE, {base::MayBlock()}, base::BindOnce(&ReadFile, path.Append(kJsonFile)), - base::BindOnce(&YouTubeRuleRegistry::OnLoadRules, + base::BindOnce(&YouTubeRegistry::OnLoadScripts, weak_factory_.GetWeakPtr())); } -void YouTubeRuleRegistry::OnLoadRules(const std::string& contents) { - rule_ = YouTubeRule::ParseRules(contents); +void YouTubeRegistry::OnLoadScripts(const std::string& contents) { + script_ = YouTubeScript::ParseScripts(contents); } -void YouTubeRuleRegistry::SetComponentPath(const base::FilePath& path) { +void YouTubeRegistry::SetComponentPath(const base::FilePath& path) { component_path_ = path; } diff --git a/components/youtube_script_injector/browser/core/youtube_rule_registry.h b/components/youtube_script_injector/browser/core/youtube_registry.h similarity index 72% rename from components/youtube_script_injector/browser/core/youtube_rule_registry.h rename to components/youtube_script_injector/browser/core/youtube_registry.h index f814789cb7f7..f80a04a72fff 100644 --- a/components/youtube_script_injector/browser/core/youtube_rule_registry.h +++ b/components/youtube_script_injector/browser/core/youtube_registry.h @@ -3,8 +3,8 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this file, // You can obtain one at https://mozilla.org/MPL/2.0/. -#ifndef BRAVE_COMPONENTS_YOUTUBE_SCRIPT_INJECTOR_BROWSER_CORE_YOUTUBE_RULE_REGISTRY_H_ -#define BRAVE_COMPONENTS_YOUTUBE_SCRIPT_INJECTOR_BROWSER_CORE_YOUTUBE_RULE_REGISTRY_H_ +#ifndef BRAVE_COMPONENTS_YOUTUBE_SCRIPT_INJECTOR_BROWSER_CORE_YOUTUBE_REGISTRY_H_ +#define BRAVE_COMPONENTS_YOUTUBE_SCRIPT_INJECTOR_BROWSER_CORE_YOUTUBE_REGISTRY_H_ #include #include @@ -15,7 +15,7 @@ #include "base/memory/raw_ptr.h" #include "base/memory/singleton.h" #include "base/memory/weak_ptr.h" -#include "brave/components/youtube_script_injector/browser/core/youtube_rule.h" +#include "brave/components/youtube_script_injector/browser/core/youtube_script.h" class GURL; @@ -27,32 +27,32 @@ FORWARD_DECLARE_TEST(YouTubeTabHelperBrowserTest, RuleMatchTestScriptTrue); // This class loads and stores the rules from the youtube.json file. // It is also used for matching based on the URL. -class COMPONENT_EXPORT(YOUTUBE_SCRIPT_INJECTOR_BROWSER_CORE) YouTubeRuleRegistry { +class COMPONENT_EXPORT(YOUTUBE_SCRIPT_INJECTOR_BROWSER_CORE) YouTubeRegistry { public: - YouTubeRuleRegistry(const YouTubeRuleRegistry&) = delete; - YouTubeRuleRegistry& operator=(const YouTubeRuleRegistry&) = delete; - ~YouTubeRuleRegistry(); - static YouTubeRuleRegistry* GetInstance(); // singleton + YouTubeRegistry(const YouTubeRegistry&) = delete; + YouTubeRegistry& operator=(const YouTubeRegistry&) = delete; + ~YouTubeRegistry(); + static YouTubeRegistry* GetInstance(); // singleton // Returns the matched YouTube script injector rule, if any. void CheckIfMatch(const GURL& url, base::OnceCallback cb) const; - // Given a path to youtube.json, loads the rules from the file into memory. - void LoadRules(const base::FilePath& path); + // Given a path to youtube.json, loads the scripts from the file into memory. + void LoadScripts(const base::FilePath& path); private: - YouTubeRuleRegistry(); + YouTubeRegistry(); // These methods are also called by YouTubeTabHelperBrowserTest. - // Given contents of youtube.json, loads the rules from the file into memory. - // Called by |LoadRules| after the file is read. - void OnLoadRules(const std::string& data); + // Given contents of youtube.json, loads the scripts from the file into memory. + // Called by |LoadScripts| after the file is read. + void OnLoadScripts(const std::string& data); // Sets the component path used to resolve the paths to the scripts. void SetComponentPath(const base::FilePath& path); base::FilePath component_path_; - std::optional rule_; + std::optional script_; - base::WeakPtrFactory weak_factory_{this}; + base::WeakPtrFactory weak_factory_{this}; // Needed for testing private methods in YouTubeTabHelperBrowserTest. FRIEND_TEST_ALL_PREFIXES(YouTubeTabHelperBrowserTest, NoMatch); @@ -60,9 +60,9 @@ class COMPONENT_EXPORT(YOUTUBE_SCRIPT_INJECTOR_BROWSER_CORE) YouTubeRuleRegistry FRIEND_TEST_ALL_PREFIXES(YouTubeTabHelperBrowserTest, RuleMatchTestScriptTrue); friend class YouTubeTabHelperBrowserTest; - friend struct base::DefaultSingletonTraits; + friend struct base::DefaultSingletonTraits; }; } // namespace youtube_script_injector -#endif // BRAVE_COMPONENTS_YOUTUBE_SCRIPT_INJECTOR_BROWSER_CORE_YOUTUBE_RULE_REGISTRY_H_ +#endif // BRAVE_COMPONENTS_YOUTUBE_SCRIPT_INJECTOR_BROWSER_CORE_YOUTUBE_REGISTRY_H_ diff --git a/components/youtube_script_injector/browser/core/youtube_rule.cc b/components/youtube_script_injector/browser/core/youtube_script.cc similarity index 75% rename from components/youtube_script_injector/browser/core/youtube_rule.cc rename to components/youtube_script_injector/browser/core/youtube_script.cc index 27af6a82a01a..a79b82ad53b8 100644 --- a/components/youtube_script_injector/browser/core/youtube_rule.cc +++ b/components/youtube_script_injector/browser/core/youtube_script.cc @@ -3,7 +3,7 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this file, // You can obtain one at https://mozilla.org/MPL/2.0/. -#include "brave/components/youtube_script_injector/browser/core/youtube_rule.h" +#include "brave/components/youtube_script_injector/browser/core/youtube_script.h" #include #include @@ -54,43 +54,43 @@ bool GetFilePathFromValue(const base::Value* value, base::FilePath* result) { namespace youtube_script_injector { -YouTubeRule::YouTubeRule() = default; -YouTubeRule::~YouTubeRule() = default; -YouTubeRule::YouTubeRule(const YouTubeRule& other) { +YouTubeScript::YouTubeScript() = default; +YouTubeScript::~YouTubeScript() = default; +YouTubeScript::YouTubeScript(const YouTubeScript& other) { feature_script_path_ = other.feature_script_path_; version_ = other.version_; } // static -void YouTubeRule::RegisterJSONConverter( - base::JSONValueConverter* converter) { +void YouTubeScript::RegisterJSONConverter( + base::JSONValueConverter* converter) { converter->RegisterCustomValueField( - kFeatureScript, &YouTubeRule::feature_script_path_, GetFilePathFromValue); - converter->RegisterIntField(kVersion, &YouTubeRule::version_); + kFeatureScript, &YouTubeScript::feature_script_path_, GetFilePathFromValue); + converter->RegisterIntField(kVersion, &YouTubeScript::version_); } // static -std::optional YouTubeRule::ParseRules( +std::optional YouTubeScript::ParseScripts( const std::string& contents) { if (contents.empty()) { return std::nullopt; } std::optional root = base::JSONReader::Read(contents); if (!root) { - VLOG(1) << "YouTubeRule::ParseRules: invalid JSON"; + VLOG(1) << "YouTubeScript::ParseRules: invalid JSON"; return std::nullopt; } - YouTubeRule rule = YouTubeRule(); - base::JSONValueConverter converter; + YouTubeScript rule = YouTubeScript(); + base::JSONValueConverter converter; if (!converter.Convert(*root, &rule)) { - VLOG(1) << "YouTubeRule::ParseRules: invalid rule"; + VLOG(1) << "YouTubeScript::ParseRules: invalid rule"; return std::nullopt; } return rule; } -bool YouTubeRule::IsYouTubeDomain(const GURL& url) const { +bool YouTubeScript::IsYouTubeDomain(const GURL& url) const { if (net::registry_controlled_domains::SameDomainOrHost( url, GURL(kYouTubeUrl), net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES)) { diff --git a/components/youtube_script_injector/browser/core/youtube_rule.h b/components/youtube_script_injector/browser/core/youtube_script.h similarity index 69% rename from components/youtube_script_injector/browser/core/youtube_rule.h rename to components/youtube_script_injector/browser/core/youtube_script.h index c10c630e9a2a..6ab5ec381f76 100644 --- a/components/youtube_script_injector/browser/core/youtube_rule.h +++ b/components/youtube_script_injector/browser/core/youtube_script.h @@ -29,33 +29,25 @@ struct MatchedRule { }; // Format of the youtube.json file: -// [ -// { -// "include": [ -// "https://twitter.com/*" -// ], -// "exclude": [ -// ], -// "version": 1, -// "policy_script": "twitter/policy.js" -// }, ... -// ] -// Note that "policy_script" gives a path -// relative to the component under scripts/ -// This class describes a single rule in the youtube.json file. -class YouTubeRule { +// { +// "version": 1, +// "feature_script": "keep-playing-audio.js" +// } +// Note that "feature_script" gives a path +// relative to the component under scripts directory. +class YouTubeScript { public: - YouTubeRule(); - ~YouTubeRule(); - YouTubeRule(const YouTubeRule& other); // needed for std::vector + YouTubeScript(); + ~YouTubeScript(); + YouTubeScript(const YouTubeScript& other); // needed for std::optional // Registers the mapping between JSON field names and the members in this // class. static void RegisterJSONConverter( - base::JSONValueConverter* converter); + base::JSONValueConverter* converter); - // Parse the youtube.json file contents into a vector of YouTubeRule. - static std::optional ParseRules( + // Parse the youtube.json file contents into an optional YouTubeScript. + static std::optional ParseScripts( const std::string& contents); // Check if this rule matches the given URL. // bool ShouldInsertScript(const GURL& url) const; @@ -66,8 +58,6 @@ class YouTubeRule { 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 feature_script_path_; // Used for checking if the last inserted script is the latest version.