Introduce better class names

brave · Nov 21, 2024 · 5c2fc60 · 5c2fc60
1 parent 979a039
commit 5c2fc60
Show file tree

Hide file tree

Showing 8 changed files with 82 additions and 93 deletions.
diff --git 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_json.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<YouTubeTabHelper>(*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
@@ -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_json.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<YouTubeRuleRegistry> youtube_rule_registry_;  // NOT OWNED
+  const raw_ptr<YouTubeRegistry> youtube_registry_;  // NOT OWNED
   // The remote used to send the script to the renderer.
   mojo::AssociatedRemote<script_injector::mojom::ScriptInjector>
       script_injector_remote_;

diff --git 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_json.cc",
+    "youtube_json.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
@@ -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_json.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/...ipt_injector/browser/core/youtube_rule.cc → ...ipt_injector/browser/core/youtube_json.cc b/...ipt_injector/browser/core/youtube_rule.cc → ...ipt_injector/browser/core/youtube_json.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_json.h"
 
 #include <memory>
 #include <optional>
@@ -24,6 +24,8 @@ namespace {
 // youtube.json keys
 constexpr char kVersion[] = "version";
 constexpr char kFeatureScript[] = "feature_script";
+// TODO
+// constexpr char kFullScreenScript[] = "fullscreen_script";
 
 constexpr char kYouTubeUrl[] = "https://youtube.com";
 
@@ -54,43 +56,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) {
+YouTubeJson::YouTubeJson() = default;
+YouTubeJson::~YouTubeJson() = default;
+YouTubeJson::YouTubeJson(const YouTubeJson& other) {
   feature_script_path_ = other.feature_script_path_;
   version_ = other.version_;
 }
 
 // static
-void YouTubeRule::RegisterJSONConverter(
-    base::JSONValueConverter<YouTubeRule>* converter) {
+void YouTubeJson::RegisterJSONConverter(
+    base::JSONValueConverter<YouTubeJson>* converter) {
   converter->RegisterCustomValueField<base::FilePath>(
-      kFeatureScript, &YouTubeRule::feature_script_path_, GetFilePathFromValue);
-  converter->RegisterIntField(kVersion, &YouTubeRule::version_);
+      kFeatureScript, &YouTubeJson::feature_script_path_, GetFilePathFromValue);
+  converter->RegisterIntField(kVersion, &YouTubeJson::version_);
 }
 
 // static
-std::optional<YouTubeRule> YouTubeRule::ParseRules(
+std::optional<YouTubeJson> YouTubeJson::ParseJson(
     const std::string& contents) {
   if (contents.empty()) {
     return std::nullopt;
   }
   std::optional<base::Value> root = base::JSONReader::Read(contents);
   if (!root) {
-    VLOG(1) << "YouTubeRule::ParseRules: invalid JSON";
+    VLOG(1) << "YouTubeJson::ParseJson: invalid JSON";
     return std::nullopt;
   }
 
-  YouTubeRule rule = YouTubeRule();
-  base::JSONValueConverter<YouTubeRule> converter;
+  YouTubeJson rule = YouTubeJson();
+  base::JSONValueConverter<YouTubeJson> converter;
   if (!converter.Convert(*root, &rule)) {
-    VLOG(1) << "YouTubeRule::ParseRules: invalid rule";
+    VLOG(1) << "YouTubeJson::YouTubeJson: invalid rule";
     return std::nullopt;
   }
   return rule;
 }
 
-bool YouTubeRule::IsYouTubeDomain(const GURL& url) const {
+bool YouTubeJson::IsYouTubeDomain(const GURL& url) const {
   if (net::registry_controlled_domains::SameDomainOrHost(
           url, GURL(kYouTubeUrl),
           net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES)) {

diff --git a/...ript_injector/browser/core/youtube_rule.h → ...ript_injector/browser/core/youtube_json.h b/...ript_injector/browser/core/youtube_rule.h → ...ript_injector/browser/core/youtube_json.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_H_
-#define BRAVE_COMPONENTS_YOUTUBE_SCRIPT_INJECTOR_BROWSER_CORE_YOUTUBE_RULE_H_
+#ifndef BRAVE_COMPONENTS_YOUTUBE_SCRIPT_INJECTOR_BROWSER_CORE_YOUTUBE_JSON_H_
+#define BRAVE_COMPONENTS_YOUTUBE_SCRIPT_INJECTOR_BROWSER_CORE_YOUTUBE_JSON_H_
 
 #include <memory>
 #include <optional>
@@ -29,34 +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 YouTubeJson {
  public:
-  YouTubeRule();
-  ~YouTubeRule();
-  YouTubeRule(const YouTubeRule& other);  // needed for std::vector<YouTubeRule>
+  YouTubeJson();
+  ~YouTubeJson();
+  YouTubeJson(const YouTubeJson& other);  // needed for std::optional<YouTubeJson>
 
   // Registers the mapping between JSON field names and the members in this
   // class.
   static void RegisterJSONConverter(
-      base::JSONValueConverter<YouTubeRule>* converter);
+      base::JSONValueConverter<YouTubeJson>* converter);
 
-  // Parse the youtube.json file contents into a vector of YouTubeRule.
-  static std::optional<YouTubeRule> ParseRules(
-      const std::string& contents);
+  // Parse the youtube.json file contents into an optional YouTubeJson.
+  static std::optional<YouTubeJson> ParseJson(const std::string& contents);
   // Check if this rule matches the given URL.
   // bool ShouldInsertScript(const GURL& url) const;
   bool IsYouTubeDomain(const GURL& url) const;
@@ -66,8 +57,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.
@@ -76,4 +65,4 @@ class YouTubeRule {
 
 }  // namespace youtube_script_injector
 
-#endif  // BRAVE_COMPONENTS_YOUTUBE_SCRIPT_INJECTOR_BROWSER_CORE_YOUTUBE_RULE_H_
+#endif  // BRAVE_COMPONENTS_YOUTUBE_SCRIPT_INJECTOR_BROWSER_CORE_YOUTUBE_JSON_H_
diff --git a/...tor/browser/core/youtube_rule_registry.cc → ...injector/browser/core/youtube_registry.cc b/...tor/browser/core/youtube_rule_registry.cc → ...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 <memory>
 #include <string>
@@ -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_json.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<YouTubeRuleRegistry>::get();
+  return base::Singleton<YouTubeRegistry>::get();
 }
 
-YouTubeRuleRegistry::YouTubeRuleRegistry() = default;
+YouTubeRegistry::YouTubeRegistry() = default;
 
-YouTubeRuleRegistry::~YouTubeRuleRegistry() = default;
+YouTubeRegistry::~YouTubeRegistry() = default;
 
-void YouTubeRuleRegistry::CheckIfMatch(
+void YouTubeRegistry::CheckIfMatch(
     const GURL& url,
     base::OnceCallback<void(MatchedRule)> cb) const {
-  if (rule_ && rule_->IsYouTubeDomain(url)) {
+  if (json_ && json_->IsYouTubeDomain(url)) {
     base::ThreadPool::PostTaskAndReplyWithResult(
         FROM_HERE, {base::MayBlock()},
         base::BindOnce(&CreateMatchedRule, component_path_,
-                       rule_->GetFeatureScript(),
-                       rule_->GetVersion()),
+                       json_->GetFeatureScript(),
+                       json_->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) {
+  json_ = YouTubeJson::ParseJson(contents);
 }
 
-void YouTubeRuleRegistry::SetComponentPath(const base::FilePath& path) {
+void YouTubeRegistry::SetComponentPath(const base::FilePath& path) {
   component_path_ = path;
 }