Modularize brave news internals (#26445)

Modularize brave_news_internals

.github/CODEOWNERS

@@ -10,6 +10,9 @@ DEPS @brave/deps-reviewers
 script/deps.py @brave/deps-reviewers
 package*.json @brave/js-deps-reviewers
 
+# brave/browser/ui/BUILD.gn should not have new files added to it
+browser/ui/BUILD.gn @bridiver
+
 # BraveBrowserProcess public interface
 browser/brave_browser_process.h @bridiver
 

browser/extensions/BUILD.gn

@@ -133,7 +133,6 @@ source_set("extensions") {
     "//brave/browser/extensions/api:generated_api_registration",
     "//brave/browser/themes",
     "//brave/browser/ui:brave_tab_prefs",
-    "//brave/browser/ui:ui_public_dependencies",
     "//brave/common",
     "//brave/common/extensions/api",
     "//brave/components/ai_chat/core/common",

browser/sources.gni

@@ -38,6 +38,7 @@ import("//brave/browser/search_engines/sources.gni")
 import("//brave/browser/speedreader/sources.gni")
 import("//brave/browser/sync/sources.gni")
 import("//brave/browser/themes/sources.gni")
+import("//brave/browser/ui/config.gni")
 import("//brave/browser/url_sanitizer/sources.gni")
 import("//brave/browser/web_discovery/sources.gni")
 import("//brave/browser/webcompat_reporter/sources.gni")
@@ -285,6 +286,7 @@ if (!is_android) {
 
   brave_chrome_browser_deps += [
     "//brave/browser/ui/ai_chat",
+    "//brave/browser/ui/webui/brave_news_internals",
     "//components/feed:feature_list",
     "//components/feed/core/v2:feed_core_v2",
     "//components/feed/mojom:mojo_bindings",
@@ -575,6 +577,10 @@ brave_chrome_browser_deps += brave_browser_webcompat_reporter_deps
 brave_chrome_browser_deps += brave_chromium_src_chrome_browser_deps
 brave_chrome_browser_deps += brave_chromium_src_chrome_browser_prefs_deps
 
+brave_chrome_browser_ui_allow_circular_includes_from = [ "//brave/browser/ui" ]
+brave_chrome_browser_ui_allow_circular_includes_from +=
+    brave_ui_allow_circular_includes_from
+
 brave_chrome_browser_allow_circular_includes_from = [
   "//brave/browser/ui",
   "//brave/browser/brave_ads:impl",

browser/ui/BUILD.gn

@@ -5,6 +5,7 @@
 
 import("//brave/browser/ethereum_remote_client/buildflags/buildflags.gni")
 import("//brave/browser/shell_integrations/buildflags/buildflags.gni")
+import("//brave/browser/ui/config.gni")
 import("//brave/build/config.gni")
 import("//brave/components/ai_rewriter/common/buildflags/buildflags.gni")
 import("//brave/components/brave_vpn/common/buildflags/buildflags.gni")
@@ -23,11 +24,12 @@ import("//mojo/public/tools/bindings/mojom.gni")
 import("//printing/buildflags/buildflags.gni")
 import("//third_party/widevine/cdm/widevine.gni")
 
-# Some targets have allow_circular_includes_from for //chrome/browser/ui so they
-# are missing the dependency that would otherwise be required for it. This is a
-# workaround to add the public deps from //brave/browser/ui that would be added
-# automatically if the //chrome/browser/ui dependency was present.
+# See //chrome/browser/ui:ui_public_depdendencies.
 group("ui_public_dependencies") {
+  visibility = [
+    "//brave/browser/ui",
+    "//chrome/browser/ui:ui_public_dependencies",
+  ]
   public_deps = [
     ":brave_rewards_source",
     ":brave_tab_features",
@@ -57,6 +59,9 @@ source_set("ui") {
     "//chrome/browser/ui:browser_navigator_params_headers",
   ]
 
+  allow_circular_includes_from = brave_ui_allow_circular_includes_from
+  deps += brave_ui_allow_circular_includes_from
+
   public_deps = [ ":ui_public_dependencies" ]
 
   sources = [
@@ -771,6 +776,7 @@ source_set("ui") {
     "//brave/components/brave_ads/core",
     "//brave/components/brave_federated",
     "//brave/components/brave_federated/public/interfaces",
+    "//brave/components/brave_news/browser",
     "//brave/components/brave_news/common",
     "//brave/components/brave_perf_predictor/common",
     "//brave/components/brave_rewards/browser",
@@ -1046,7 +1052,6 @@ source_set("ui") {
       "//brave/common/importer",
       "//brave/components/brave_new_tab_ui:generated_resources",
       "//brave/components/brave_new_tab_ui:mojom",
-      "//brave/components/brave_news/browser",
       "//brave/components/brave_private_new_tab_ui/common",
       "//brave/components/brave_private_new_tab_ui/common:mojom",
       "//brave/components/brave_private_new_tab_ui/resources/page:generated_resources",
@@ -1245,8 +1250,6 @@ source_set("ui") {
       "views/wallet_bubble_focus_observer.h",
       "wallet_bubble_manager_delegate_impl.cc",
       "wallet_bubble_manager_delegate_impl.h",
-      "webui/brave_news_internals/brave_news_internals_ui.cc",
-      "webui/brave_news_internals/brave_news_internals_ui.h",
       "webui/brave_wallet/ledger/ledger_ui.cc",
       "webui/brave_wallet/ledger/ledger_ui.h",
       "webui/brave_wallet/page_handler/wallet_page_handler.cc",
@@ -1267,8 +1270,6 @@ source_set("ui") {
       "//brave/browser/resources/settings/shortcuts_page:generated_resources",
       "//brave/browser/ui/brave_wallet",
       "//brave/browser/ui/webui/brave_wallet/common_handler",
-      "//brave/components/brave_news/browser",
-      "//brave/components/brave_news/browser/resources:generated_resources",
       "//brave/components/brave_wallet/browser:permission_utils",
       "//brave/components/brave_wallet/common",
       "//brave/components/brave_wallet/common:mojom",

browser/ui/config.gni

@@ -0,0 +1,10 @@
+# Copyright (c) 2024 The Brave Authors. All rights reserved.
+# This Source Code Form is subject to the terms of the Mozilla Public
+# 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/.
+
+brave_ui_allow_circular_includes_from = []
+if (!is_android) {
+  brave_ui_allow_circular_includes_from +=
+      [ "//brave/browser/ui/webui/brave_news_internals" ]
+}

browser/ui/webui/brave_news_internals/BUILD.gn

@@ -0,0 +1,18 @@
+# Copyright (c) 2024 The Brave Authors. All rights reserved.
+# This Source Code Form is subject to the terms of the Mozilla Public
+# 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/.
+
+source_set("brave_news_internals") {
+  sources = [
+    "brave_news_internals_ui.cc",
+    "brave_news_internals_ui.h",
+  ]
+
+  deps = [
+    "//brave/components/brave_news/browser",
+    "//brave/components/brave_news/browser/resources:generated_resources",
+    "//brave/components/brave_news/common:mojom",
+    "//chrome/browser:browser_public_dependencies",
+  ]
+}

browser/ui/webui/brave_news_internals/brave_news_internals_ui.cc

@@ -8,17 +8,17 @@
 #include <string>
 #include <utility>
 
-#include "brave/browser/brave_news/brave_news_controller_factory.h"
 #include "brave/browser/ui/webui/brave_webui_source.h"
 #include "brave/components/brave_news/browser/brave_news_controller.h"
 #include "brave/components/brave_news/browser/resources/grit/brave_news_internals_generated_map.h"
 #include "brave/components/brave_news/common/brave_news.mojom.h"
-#include "chrome/browser/profiles/profile.h"
 #include "components/grit/brave_components_resources.h"
 
-BraveNewsInternalsUI::BraveNewsInternalsUI(content::WebUI* web_ui,
-                                           const std::string& host)
-    : content::WebUIController(web_ui) {
+BraveNewsInternalsUI::BraveNewsInternalsUI(
+    content::WebUI* web_ui,
+    const std::string& host,
+    brave_news::BraveNewsController* controller)
+    : content::WebUIController(web_ui), controller_(controller) {
   auto* source = CreateAndAddWebUIDataSource(
       web_ui, host, kBraveNewsInternalsGenerated,
       kBraveNewsInternalsGeneratedSize, IDR_BRAVE_NEWS_INTERNALS_HTML);
@@ -30,24 +30,18 @@ WEB_UI_CONTROLLER_TYPE_IMPL(BraveNewsInternalsUI)
 
 void BraveNewsInternalsUI::BindInterface(
     mojo::PendingReceiver<brave_news::mojom::BraveNewsController> receiver) {
-  auto* profile = Profile::FromWebUI(web_ui());
-  auto* controller =
-      brave_news::BraveNewsControllerFactory::GetForBrowserContext(profile);
-  if (!controller) {
+  if (!controller_) {
     return;
   }
 
-  controller->Bind(std::move(receiver));
+  controller_->Bind(std::move(receiver));
 }
 
 void BraveNewsInternalsUI::BindInterface(
     mojo::PendingReceiver<brave_news::mojom::BraveNewsInternals> receiver) {
-  auto* profile = Profile::FromWebUI(web_ui());
-  auto* controller =
-      brave_news::BraveNewsControllerFactory::GetForBrowserContext(profile);
-  if (!controller) {
+  if (!controller_) {
     return;
   }
 
-  controller->Bind(std::move(receiver));
+  controller_->Bind(std::move(receiver));
 }

browser/ui/webui/brave_news_internals/brave_news_internals_ui.h

@@ -6,17 +6,22 @@
 #ifndef BRAVE_BROWSER_UI_WEBUI_BRAVE_NEWS_INTERNALS_BRAVE_NEWS_INTERNALS_UI_H_
 #define BRAVE_BROWSER_UI_WEBUI_BRAVE_NEWS_INTERNALS_BRAVE_NEWS_INTERNALS_UI_H_
 
-#include <memory>
 #include <string>
 
+#include "base/memory/raw_ptr.h"
 #include "brave/components/brave_news/common/brave_news.mojom-forward.h"
 #include "content/public/browser/web_ui_controller.h"
 #include "mojo/public/cpp/bindings/pending_receiver.h"
 
+namespace brave_news {
+class BraveNewsController;
+}
+
 class BraveNewsInternalsUI : public content::WebUIController {
  public:
   explicit BraveNewsInternalsUI(content::WebUI* web_ui,
-                                const std::string& host);
+                                const std::string& host,
+                                brave_news::BraveNewsController* controller);
   BraveNewsInternalsUI(const BraveNewsInternalsUI&) = delete;
   BraveNewsInternalsUI& operator=(const BraveNewsInternalsUI&) = delete;
   ~BraveNewsInternalsUI() override;
@@ -27,6 +32,7 @@ class BraveNewsInternalsUI : public content::WebUIController {
       mojo::PendingReceiver<brave_news::mojom::BraveNewsInternals> receiver);
 
  private:
+  raw_ptr<brave_news::BraveNewsController> controller_ = nullptr;
   WEB_UI_CONTROLLER_TYPE_DECL();
 };
 

browser/ui/webui/brave_web_ui_controller_factory.cc

@@ -11,6 +11,7 @@
 #include "base/feature_list.h"
 #include "base/memory/ptr_util.h"
 #include "base/no_destructor.h"
+#include "brave/browser/brave_news/brave_news_controller_factory.h"
 #include "brave/browser/brave_rewards/rewards_util.h"
 #include "brave/browser/ethereum_remote_client/buildflags/buildflags.h"
 #include "brave/browser/ui/webui/brave_rewards/rewards_page_ui.h"
@@ -128,7 +129,9 @@ WebUIController* NewWebUI(WebUI* web_ui, const GURL& url) {
   } else if (base::FeatureList::IsEnabled(
                  brave_news::features::kBraveNewsFeedUpdate) &&
              host == kBraveNewsInternalsHost) {
-    return new BraveNewsInternalsUI(web_ui, url.host());
+    return new BraveNewsInternalsUI(
+        web_ui, url.host(),
+        brave_news::BraveNewsControllerFactory::GetForBrowserContext(profile));
   } else if (host == kWelcomeHost && !profile->IsGuestSession()) {
     return new BraveWelcomeUI(web_ui, url.host());
   } else if (host == chrome::kChromeUINewTabHost) {

patches/chrome-browser-ui-BUILD.gn.patch

@@ -1,12 +1,12 @@
 diff --git a/chrome/browser/ui/BUILD.gn b/chrome/browser/ui/BUILD.gn
-index 4c9e41d73cedd5eafeb65832c1a8e172484a84cd..e5897fcb367c20c7235d56c11e6f35b51a69f554 100644
+index 4c9e41d73cedd5eafeb65832c1a8e172484a84cd..2a59f5e6a414f61dbc39a6e305109db366539352 100644
 --- a/chrome/browser/ui/BUILD.gn
 +++ b/chrome/browser/ui/BUILD.gn
 @@ -741,6 +741,7 @@ static_library("ui") {
      "//chrome/browser/content_settings:impl",
      "//chrome/browser/content_settings:content_settings_util_impl",
    ]
-+  public_deps += [ "//brave/browser/ui" ] allow_circular_includes_from += [ "//brave/browser/ui" ]
++  import("//brave/browser/sources.gni") public_deps += [ "//brave/browser/ui" ] deps += brave_chrome_browser_ui_allow_circular_includes_from allow_circular_includes_from += brave_chrome_browser_ui_allow_circular_includes_from
 
    if (enable_vr && is_win) {
      deps += [ "//chrome/browser/vr:vr_base" ]
@@ -22,3 +22,11 @@ index 4c9e41d73cedd5eafeb65832c1a8e172484a84cd..e5897fcb367c20c7235d56c11e6f35b5
      }
    }
 
+@@ -5709,6 +5713,7 @@ static_library("ui_public_dependencies") {
+     "//content/public/browser",
+     "//mojo/public/cpp/bindings",
+   ]
++  public_deps += [ "//brave/browser/ui:ui_public_dependencies" ]
+   if (!is_android) {
+     public_deps += [
+       "//build:branding_buildflags",