diff --git a/browser/brave_news/direct_feed_fetcher_delegate_impl.cc b/browser/brave_news/direct_feed_fetcher_delegate_impl.cc index 160180aa09cc..22ce8cfeeaf5 100644 --- a/browser/brave_news/direct_feed_fetcher_delegate_impl.cc +++ b/browser/brave_news/direct_feed_fetcher_delegate_impl.cc @@ -17,6 +17,8 @@ DirectFeedFetcherDelegateImpl::DirectFeedFetcherDelegateImpl( https_upgrade_exceptions_service_( g_brave_browser_process->https_upgrade_exceptions_service()) {} +DirectFeedFetcherDelegateImpl::~DirectFeedFetcherDelegateImpl() = default; + bool DirectFeedFetcherDelegateImpl::ShouldUpgradeToHttps(const GURL& url) { DCHECK_CURRENTLY_ON(content::BrowserThread::UI); if (!host_content_settings_map_ || !https_upgrade_exceptions_service_) { @@ -26,10 +28,9 @@ bool DirectFeedFetcherDelegateImpl::ShouldUpgradeToHttps(const GURL& url) { https_upgrade_exceptions_service_); } -void DirectFeedFetcherDelegateImpl::Shutdown() { - DCHECK_CURRENTLY_ON(content::BrowserThread::UI); - host_content_settings_map_ = nullptr; - https_upgrade_exceptions_service_ = nullptr; +base::WeakPtr +DirectFeedFetcherDelegateImpl::AsWeakPtr() { + return weak_ptr_factory_.GetWeakPtr(); } } // namespace brave_news diff --git a/browser/brave_news/direct_feed_fetcher_delegate_impl.h b/browser/brave_news/direct_feed_fetcher_delegate_impl.h index d72daf8f6329..339dd4af5a55 100644 --- a/browser/brave_news/direct_feed_fetcher_delegate_impl.h +++ b/browser/brave_news/direct_feed_fetcher_delegate_impl.h @@ -21,7 +21,7 @@ class DirectFeedFetcherDelegateImpl : public DirectFeedFetcher::Delegate { public: explicit DirectFeedFetcherDelegateImpl( HostContentSettingsMap* host_content_settings_map); - ~DirectFeedFetcherDelegateImpl() override = default; + ~DirectFeedFetcherDelegateImpl() override; DirectFeedFetcherDelegateImpl(const DirectFeedFetcherDelegateImpl&) = delete; DirectFeedFetcherDelegateImpl& operator=( @@ -29,12 +29,15 @@ class DirectFeedFetcherDelegateImpl : public DirectFeedFetcher::Delegate { // Must be called on UI thread bool ShouldUpgradeToHttps(const GURL& url) override; - void Shutdown() override; + + base::WeakPtr AsWeakPtr() override; private: raw_ptr host_content_settings_map_; raw_ptr https_upgrade_exceptions_service_; + + base::WeakPtrFactory weak_ptr_factory_{this}; }; } // namespace brave_news diff --git a/components/brave_news/browser/brave_news_controller.cc b/components/brave_news/browser/brave_news_controller.cc index f8cca44fbb6a..082264f325de 100644 --- a/components/brave_news/browser/brave_news_controller.cc +++ b/components/brave_news/browser/brave_news_controller.cc @@ -117,15 +117,14 @@ BraveNewsController::BraveNewsController( url_loader_factory), history_service_(history_service), url_loader_factory_(url_loader_factory), - task_runner_(base::ThreadPool::CreateSingleThreadTaskRunner( - {base::MayBlock(), base::TaskPriority::BEST_EFFORT, - base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN})), - direct_feed_fetcher_delegate_(direct_feed_fetcher_delegate.release(), - base::OnTaskRunnerDeleter(task_runner_)), + direct_feed_fetcher_delegate_(std::move(direct_feed_fetcher_delegate)), pref_manager_(*prefs), news_metrics_(prefs, pref_manager_), direct_feed_controller_(url_loader_factory, - direct_feed_fetcher_delegate_.get()), + direct_feed_fetcher_delegate_->AsWeakPtr()), + task_runner_(base::ThreadPool::CreateSingleThreadTaskRunner( + {base::MayBlock(), base::TaskPriority::BEST_EFFORT, + base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN})), engine_(nullptr, base::OnTaskRunnerDeleter(task_runner_)), initialization_promise_( 3, @@ -147,7 +146,6 @@ BraveNewsController::BraveNewsController( } BraveNewsController::~BraveNewsController() { - direct_feed_fetcher_delegate_->Shutdown(); net::NetworkChangeNotifier::RemoveNetworkChangeObserver(this); } @@ -800,9 +798,9 @@ void BraveNewsController::Prefetch() { } void BraveNewsController::ResetEngine() { - engine_.reset(new BraveNewsEngine(url_loader_factory_->Clone(), - MakeHistoryQuerier(), - direct_feed_fetcher_delegate_.get())); + engine_.reset( + new BraveNewsEngine(url_loader_factory_->Clone(), MakeHistoryQuerier(), + direct_feed_fetcher_delegate_->AsWeakPtr())); } void BraveNewsController::ConditionallyStartOrStopTimer() { diff --git a/components/brave_news/browser/brave_news_controller.h b/components/brave_news/browser/brave_news_controller.h index 29682a87fe9c..bfda58fae042 100644 --- a/components/brave_news/browser/brave_news_controller.h +++ b/components/brave_news/browser/brave_news_controller.h @@ -185,9 +185,7 @@ class BraveNewsController raw_ptr history_service_; scoped_refptr url_loader_factory_; - scoped_refptr task_runner_; - std::unique_ptr - direct_feed_fetcher_delegate_; + std::unique_ptr direct_feed_fetcher_delegate_; BraveNewsPrefManager pref_manager_; SubscriptionsSnapshot last_subscriptions_; @@ -196,6 +194,7 @@ class BraveNewsController DirectFeedController direct_feed_controller_; + scoped_refptr task_runner_; // Created on this sequence but lives on |task_runner_|. std::unique_ptr engine_; diff --git a/components/brave_news/browser/brave_news_engine.cc b/components/brave_news/browser/brave_news_engine.cc index e1ebf8d07fea..c513f29c8438 100644 --- a/components/brave_news/browser/brave_news_engine.cc +++ b/components/brave_news/browser/brave_news_engine.cc @@ -33,7 +33,7 @@ BraveNewsEngine::BraveNewsEngine( std::unique_ptr pending_shared_url_loader_factory, BackgroundHistoryQuerier history_querier, - DirectFeedFetcher::Delegate* direct_feed_fetcher_delegate) + base::WeakPtr direct_feed_fetcher_delegate) : pending_shared_url_loader_factory_( std::move(pending_shared_url_loader_factory)), history_querier_(std::move(history_querier)), diff --git a/components/brave_news/browser/brave_news_engine.h b/components/brave_news/browser/brave_news_engine.h index 2e6f9d7e4b3a..dff7d35bbd67 100644 --- a/components/brave_news/browser/brave_news_engine.h +++ b/components/brave_news/browser/brave_news_engine.h @@ -51,7 +51,7 @@ class BraveNewsEngine { std::unique_ptr pending_shared_url_loader_factory, BackgroundHistoryQuerier history_querier, - DirectFeedFetcher::Delegate* direct_feed_fetcher_delegate); + base::WeakPtr direct_feed_fetcher_delegate); BraveNewsEngine(const BraveNewsEngine&) = delete; BraveNewsEngine& operator=(const BraveNewsEngine&) = delete; ~BraveNewsEngine(); @@ -107,7 +107,7 @@ class BraveNewsEngine { BackgroundHistoryQuerier history_querier_ GUARDED_BY_CONTEXT(sequence_checker_); - raw_ptr direct_feed_fetcher_delegate_; + base::WeakPtr direct_feed_fetcher_delegate_; std::unique_ptr api_request_helper_ GUARDED_BY_CONTEXT(sequence_checker_); diff --git a/components/brave_news/browser/direct_feed_controller.cc b/components/brave_news/browser/direct_feed_controller.cc index bfca6f8c695d..d8081ca4c620 100644 --- a/components/brave_news/browser/direct_feed_controller.cc +++ b/components/brave_news/browser/direct_feed_controller.cc @@ -37,7 +37,7 @@ DirectFeedController::FindFeedRequest::~FindFeedRequest() = default; DirectFeedController::DirectFeedController( scoped_refptr url_loader_factory, - DirectFeedFetcher::Delegate* direct_feed_fetcher_delegate) + base::WeakPtr direct_feed_fetcher_delegate) : fetcher_(url_loader_factory, direct_feed_fetcher_delegate) {} DirectFeedController::~DirectFeedController() = default; diff --git a/components/brave_news/browser/direct_feed_controller.h b/components/brave_news/browser/direct_feed_controller.h index c90313325a4f..430fafa63ea6 100644 --- a/components/brave_news/browser/direct_feed_controller.h +++ b/components/brave_news/browser/direct_feed_controller.h @@ -30,7 +30,7 @@ class DirectFeedController { public: explicit DirectFeedController( scoped_refptr url_loader_factory, - DirectFeedFetcher::Delegate* direct_feed_fetcher_delegate); + base::WeakPtr direct_feed_fetcher_delegate); ~DirectFeedController(); DirectFeedController(const DirectFeedController&) = delete; DirectFeedController& operator=(const DirectFeedController&) = delete; diff --git a/components/brave_news/browser/direct_feed_controller_unittest.cc b/components/brave_news/browser/direct_feed_controller_unittest.cc index 7b6236dd89a8..04f49b0651ed 100644 --- a/components/brave_news/browser/direct_feed_controller_unittest.cc +++ b/components/brave_news/browser/direct_feed_controller_unittest.cc @@ -78,10 +78,8 @@ constexpr char kFeedURL[] = "https://example.com/feed"; class BraveNewsDirectFeedControllerTest : public testing::Test { public: BraveNewsDirectFeedControllerTest() - : direct_feed_controller_( - - test_url_loader_factory_.GetSafeWeakWrapper(), - &direct_feed_fetcher_delegate_) {} + : direct_feed_controller_(test_url_loader_factory_.GetSafeWeakWrapper(), + direct_feed_fetcher_delegate_.AsWeakPtr()) {} protected: class MockDirectFeedFetcherDelegate : public DirectFeedFetcher::Delegate { @@ -90,7 +88,12 @@ class BraveNewsDirectFeedControllerTest : public testing::Test { bool ShouldUpgradeToHttps(const GURL& url) override { return true; } - void Shutdown() override {} + base::WeakPtr AsWeakPtr() override { + return weak_ptr_factory_.GetWeakPtr(); + } + + private: + base::WeakPtrFactory weak_ptr_factory_{this}; }; std::tuple VerifyFeedUrl(GURL feed_url) { diff --git a/components/brave_news/browser/direct_feed_fetcher.cc b/components/brave_news/browser/direct_feed_fetcher.cc index 1d1f4fb66e36..d3e7a7c0b1b3 100644 --- a/components/brave_news/browser/direct_feed_fetcher.cc +++ b/components/brave_news/browser/direct_feed_fetcher.cc @@ -147,7 +147,7 @@ DirectFeedResponse::DirectFeedResponse(DirectFeedResponse&&) = default; DirectFeedFetcher::DirectFeedFetcher( scoped_refptr url_loader_factory, - Delegate* delegate) + base::WeakPtr delegate) : url_loader_factory_(url_loader_factory), delegate_(delegate) {} DirectFeedFetcher::~DirectFeedFetcher() = default; @@ -155,13 +155,29 @@ void DirectFeedFetcher::DownloadFeed(GURL url, std::string publisher_id, DownloadFeedCallback callback) { if (url.SchemeIs(url::kHttpScheme)) { - content::GetUIThreadTaskRunner({})->PostTaskAndReplyWithResult( + content::GetUIThreadTaskRunner({})->PostTask( FROM_HERE, - base::BindOnce(&Delegate::ShouldUpgradeToHttps, - base::Unretained(delegate_), url), - base::BindOnce(&DirectFeedFetcher::DownloadFeedHelper, - weak_ptr_factory_.GetWeakPtr(), url, publisher_id, - std::move(callback))); + base::BindOnce( + [](base::WeakPtr delegate, + scoped_refptr source_task_runner, + base::OnceCallback callback, GURL url) { + if (delegate.WasInvalidated()) { + return; + } + bool should_upgrade = delegate->ShouldUpgradeToHttps(url); + source_task_runner->PostTask( + FROM_HERE, + base::BindOnce( + [](base::OnceCallback callback, bool result) { + std::move(callback).Run(result); + }, + std::move(callback), should_upgrade)); + }, + delegate_, base::SingleThreadTaskRunner::GetCurrentDefault(), + base::BindOnce(&DirectFeedFetcher::DownloadFeedHelper, + weak_ptr_factory_.GetWeakPtr(), url, publisher_id, + std::move(callback)), + url)); return; } DownloadFeedHelper(url, publisher_id, std::move(callback), false); diff --git a/components/brave_news/browser/direct_feed_fetcher.h b/components/brave_news/browser/direct_feed_fetcher.h index a8a89b042876..5b24e7620f51 100644 --- a/components/brave_news/browser/direct_feed_fetcher.h +++ b/components/brave_news/browser/direct_feed_fetcher.h @@ -72,12 +72,12 @@ class DirectFeedFetcher { virtual ~Delegate() = default; virtual bool ShouldUpgradeToHttps(const GURL& url) = 0; - virtual void Shutdown(); + virtual base::WeakPtr AsWeakPtr() = 0; }; explicit DirectFeedFetcher( scoped_refptr url_loader_factory, - Delegate* delegate); + base::WeakPtr delegate); DirectFeedFetcher(const DirectFeedFetcher&) = delete; DirectFeedFetcher& operator=(const DirectFeedFetcher&) = delete; ~DirectFeedFetcher(); @@ -110,7 +110,7 @@ class DirectFeedFetcher { scoped_refptr url_loader_factory_; - raw_ptr delegate_; + base::WeakPtr delegate_; base::WeakPtrFactory weak_ptr_factory_{this}; }; diff --git a/components/brave_news/browser/feed_controller.cc b/components/brave_news/browser/feed_controller.cc index 29c7fc282688..56d1b662a6c8 100644 --- a/components/brave_news/browser/feed_controller.cc +++ b/components/brave_news/browser/feed_controller.cc @@ -35,7 +35,7 @@ FeedController::FeedController( PublishersController* publishers_controller, BackgroundHistoryQuerier& history_querier, scoped_refptr url_loader_factory, - DirectFeedFetcher::Delegate* direct_feed_fetcher_delegate) + base::WeakPtr direct_feed_fetcher_delegate) : publishers_controller_(publishers_controller), history_querier_(history_querier), feed_fetcher_(*publishers_controller, diff --git a/components/brave_news/browser/feed_controller.h b/components/brave_news/browser/feed_controller.h index 983411578b0a..5322912f60c2 100644 --- a/components/brave_news/browser/feed_controller.h +++ b/components/brave_news/browser/feed_controller.h @@ -38,7 +38,7 @@ class FeedController { PublishersController* publishers_controller, BackgroundHistoryQuerier& history_querier, scoped_refptr url_loader_factory, - DirectFeedFetcher::Delegate* direct_feed_fetcher_delegate); + base::WeakPtr direct_feed_fetcher_delegate); ~FeedController(); FeedController(const FeedController&) = delete; FeedController& operator=(const FeedController&) = delete; diff --git a/components/brave_news/browser/feed_fetcher.cc b/components/brave_news/browser/feed_fetcher.cc index fa702a990c35..d63795360e0a 100644 --- a/components/brave_news/browser/feed_fetcher.cc +++ b/components/brave_news/browser/feed_fetcher.cc @@ -106,7 +106,7 @@ std::tuple FeedFetcher::CombineFeedSourceResults( FeedFetcher::FeedFetcher( PublishersController& publishers_controller, scoped_refptr url_loader_factory, - DirectFeedFetcher::Delegate* direct_feed_fetcher_delegate) + base::WeakPtr direct_feed_fetcher_delegate) : publishers_controller_(publishers_controller), api_request_helper_(GetNetworkTrafficAnnotationTag(), url_loader_factory), direct_feed_fetcher_(url_loader_factory, direct_feed_fetcher_delegate) {} diff --git a/components/brave_news/browser/feed_fetcher.h b/components/brave_news/browser/feed_fetcher.h index e4a566e6b66c..fc9a41abc29b 100644 --- a/components/brave_news/browser/feed_fetcher.h +++ b/components/brave_news/browser/feed_fetcher.h @@ -30,7 +30,7 @@ class FeedFetcher { public: FeedFetcher(PublishersController& publishers_controller, scoped_refptr url_loader_factory, - DirectFeedFetcher::Delegate* delegate); + base::WeakPtr delegate); ~FeedFetcher(); FeedFetcher(const FeedFetcher&) = delete; FeedFetcher& operator=(const FeedFetcher&) = delete; diff --git a/components/brave_news/browser/feed_v2_builder.cc b/components/brave_news/browser/feed_v2_builder.cc index 1a99e2d29d13..cb5883a2ee2f 100644 --- a/components/brave_news/browser/feed_v2_builder.cc +++ b/components/brave_news/browser/feed_v2_builder.cc @@ -614,7 +614,7 @@ FeedV2Builder::FeedV2Builder( SuggestionsController& suggestions_controller, BackgroundHistoryQuerier& history_querier, scoped_refptr url_loader_factory, - DirectFeedFetcher::Delegate* direct_feed_fetcher_delegate) + base::WeakPtr direct_feed_fetcher_delegate) : publishers_controller_(publishers_controller), channels_controller_(channels_controller), suggestions_controller_(suggestions_controller), diff --git a/components/brave_news/browser/feed_v2_builder.h b/components/brave_news/browser/feed_v2_builder.h index 64b0aab54b2b..4770e4d4143e 100644 --- a/components/brave_news/browser/feed_v2_builder.h +++ b/components/brave_news/browser/feed_v2_builder.h @@ -45,7 +45,7 @@ class FeedV2Builder { SuggestionsController& suggestions_controller, BackgroundHistoryQuerier& history_querier, scoped_refptr url_loader_factory, - DirectFeedFetcher::Delegate* direct_feed_fetcher_delegate); + base::WeakPtr direct_feed_fetcher_delegate); FeedV2Builder(const FeedV2Builder&) = delete; FeedV2Builder& operator=(const FeedV2Builder&) = delete; ~FeedV2Builder();