Skip to content

Commit

Permalink
Remove BraveAds dependency from dom_distiller
Browse files Browse the repository at this point in the history
  • Loading branch information
aseren committed Feb 27, 2023
1 parent b880200 commit a6b0a5e
Show file tree
Hide file tree
Showing 6 changed files with 18 additions and 28 deletions.
2 changes: 0 additions & 2 deletions browser/brave_ads/ads_service_factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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());
Expand Down
26 changes: 18 additions & 8 deletions browser/brave_ads/ads_tab_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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<AdsTabHelper>(*web_contents),
Expand Down Expand Up @@ -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) {
Expand Down
10 changes: 0 additions & 10 deletions browser/brave_ads/ads_tab_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 0 additions & 1 deletion browser/brave_ads/sources.gni
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
1 change: 0 additions & 1 deletion browser/sources.gni
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
6 changes: 0 additions & 6 deletions chromium_src/chrome/app/chrome_main_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<int> ChromeMainDelegate::BasicStartupComplete() {
BraveCommandLineHelper command_line(base::CommandLine::ForCurrentProcess());
command_line.AppendSwitch(switches::kDisableDomainReliability);
Expand Down

0 comments on commit a6b0a5e

Please sign in to comment.