From a6b0a5e3ffb4dae39ea69dba6d56806d77a5a141 Mon Sep 17 00:00:00 2001 From: Aleksey Seren Date: Fri, 24 Feb 2023 13:43:02 -0500 Subject: [PATCH] Remove BraveAds dependency from dom_distiller --- browser/brave_ads/ads_service_factory.cc | 2 -- browser/brave_ads/ads_tab_helper.cc | 26 +++++++++++++------ browser/brave_ads/ads_tab_helper.h | 10 ------- browser/brave_ads/sources.gni | 1 - browser/sources.gni | 1 - .../chrome/app/chrome_main_delegate.cc | 6 ----- 6 files changed, 18 insertions(+), 28 deletions(-) diff --git a/browser/brave_ads/ads_service_factory.cc b/browser/brave_ads/ads_service_factory.cc index 40d4a5140ca7..b148ebfe33d4 100644 --- a/browser/brave_ads/ads_service_factory.cc +++ b/browser/brave_ads/ads_service_factory.cc @@ -17,7 +17,6 @@ #include "brave/components/brave_federated/brave_federated_service.h" #include "brave/components/brave_federated/data_store_service.h" #include "brave/components/brave_federated/notification_ad_task_constants.h" -#include "chrome/browser/dom_distiller/dom_distiller_service_factory.h" #include "chrome/browser/history/history_service_factory.h" #include "chrome/browser/notifications/notification_display_service_factory.h" #include "chrome/browser/profiles/incognito_helpers.h" @@ -47,7 +46,6 @@ AdsServiceFactory::AdsServiceFactory() "AdsService", BrowserContextDependencyManager::GetInstance()) { DependsOn(NotificationDisplayServiceFactory::GetInstance()); - DependsOn(dom_distiller::DomDistillerServiceFactory::GetInstance()); DependsOn(brave_rewards::RewardsServiceFactory::GetInstance()); DependsOn(HistoryServiceFactory::GetInstance()); DependsOn(brave_federated::BraveFederatedServiceFactory::GetInstance()); diff --git a/browser/brave_ads/ads_tab_helper.cc b/browser/brave_ads/ads_tab_helper.cc index 18f222ef34cb..aab358d9c8fa 100644 --- a/browser/brave_ads/ads_tab_helper.cc +++ b/browser/brave_ads/ads_tab_helper.cc @@ -9,8 +9,7 @@ #include "brave/browser/brave_ads/ads_service_factory.h" #include "chrome/browser/profiles/profile.h" -#include "components/dom_distiller/content/browser/distiller_javascript_utils.h" -#include "components/dom_distiller/content/browser/distiller_page_web_contents.h" +#include "chrome/common/chrome_isolated_world_ids.h" #include "components/sessions/content/session_tab_helper.h" #include "content/public/browser/navigation_handle.h" #include "content/public/browser/render_frame_host.h" @@ -27,6 +26,15 @@ namespace brave_ads { +namespace { + +constexpr char16_t kGetDocumentHTMLScript[] = + u"new XMLSerializer().serializeToString(document)"; + +constexpr char16_t kGetInnerTextScript[] = u"document?.body?.innerText"; + +} // namespace + AdsTabHelper::AdsTabHelper(content::WebContents* web_contents) : WebContentsObserver(web_contents), content::WebContentsUserData(*web_contents), @@ -67,15 +75,17 @@ void AdsTabHelper::RunIsolatedJavaScript( content::RenderFrameHost* render_frame_host) { DCHECK(render_frame_host); - dom_distiller::RunIsolatedJavaScript( - render_frame_host, "new XMLSerializer().serializeToString(document)", + render_frame_host->ExecuteJavaScriptInIsolatedWorld( + kGetDocumentHTMLScript, base::BindOnce(&AdsTabHelper::OnJavaScriptHtmlResult, - weak_factory_.GetWeakPtr())); + weak_factory_.GetWeakPtr()), + ISOLATED_WORLD_ID_BRAVE_INTERNAL); - dom_distiller::RunIsolatedJavaScript( - render_frame_host, "document?.body?.innerText", + render_frame_host->ExecuteJavaScriptInIsolatedWorld( + kGetInnerTextScript, base::BindOnce(&AdsTabHelper::OnJavaScriptTextResult, - weak_factory_.GetWeakPtr())); + weak_factory_.GetWeakPtr()), + ISOLATED_WORLD_ID_BRAVE_INTERNAL); } void AdsTabHelper::OnJavaScriptHtmlResult(base::Value value) { diff --git a/browser/brave_ads/ads_tab_helper.h b/browser/brave_ads/ads_tab_helper.h index 88c28f9be70e..bb05bc72c666 100644 --- a/browser/brave_ads/ads_tab_helper.h +++ b/browser/brave_ads/ads_tab_helper.h @@ -23,16 +23,6 @@ class Browser; class GURL; -namespace dom_distiller { - -class DistillerPage; - -namespace proto { -class DomDistillerResult; -} // namespace proto - -} // namespace dom_distiller - namespace brave_ads { class AdsService; diff --git a/browser/brave_ads/sources.gni b/browser/brave_ads/sources.gni index 470a9f7b92f0..eaa06e47ed97 100644 --- a/browser/brave_ads/sources.gni +++ b/browser/brave_ads/sources.gni @@ -39,7 +39,6 @@ brave_browser_brave_ads_deps = [ "//chrome/browser/profiles", "//chrome/browser/profiles:profile", "//chrome/browser/ui", - "//components/dom_distiller/content/browser", "//components/history/core/browser", "//components/keyed_service/content", "//components/sessions", diff --git a/browser/sources.gni b/browser/sources.gni index 7d55f2323b0b..a3d61f8d29c2 100644 --- a/browser/sources.gni +++ b/browser/sources.gni @@ -197,7 +197,6 @@ brave_chrome_browser_deps = [ "//components/content_settings/browser", "//components/content_settings/core/browser", "//components/content_settings/core/common", - "//components/dom_distiller/core", "//components/embedder_support", "//components/feed/core/shared_prefs:feed_shared_prefs", "//components/gcm_driver:gcm_buildflags", diff --git a/chromium_src/chrome/app/chrome_main_delegate.cc b/chromium_src/chrome/app/chrome_main_delegate.cc index 1f1e3cdb7233..0625d8b3023b 100644 --- a/chromium_src/chrome/app/chrome_main_delegate.cc +++ b/chromium_src/chrome/app/chrome_main_delegate.cc @@ -52,12 +52,6 @@ void AdjustSyncServiceUrlForAndroid(std::string* brave_sync_service_url) { // Because of that, upstream tests won't get BraveMainDelegate instantiated and // therefore we won't get any of the features below disabled/enabled when // running those browser tests, which is not what we want. -// -// For instance, on DCHECK-enabled builds, not enabling the DOM distiller will -// get all browser tests crashing when dom_distiller::RunIsolatedJavaScript() -// gets called from AdsTabHelper::RunIsolatedJavaScript() (called via the -// WebContentsObserver machinery), since the JS World ID won't be set yet when -// dom_distiller::RunIsolatedJavaScript() gets called. absl::optional ChromeMainDelegate::BasicStartupComplete() { BraveCommandLineHelper command_line(base::CommandLine::ForCurrentProcess()); command_line.AppendSwitch(switches::kDisableDomainReliability);