Skip to content

Commit

Permalink
[CodeHealth][rewards] Use base::ScopedObservation pt.2
Browse files Browse the repository at this point in the history
This PR changes the use of manual `raw_ptr` for observations, to have
then managed by `base::ScopedObservation`.

Resolves brave/brave-browser#41607
  • Loading branch information
cdesouza-chromium committed Oct 14, 2024
1 parent d2e7bfb commit 7faaf9a
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 49 deletions.
55 changes: 18 additions & 37 deletions browser/brave_rewards/android/brave_rewards_native_worker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -42,19 +42,18 @@ namespace android {
BraveRewardsNativeWorker::BraveRewardsNativeWorker(JNIEnv* env,
const base::android::JavaRef<jobject>& obj):
weak_java_brave_rewards_native_worker_(env, obj),
brave_rewards_service_(nullptr),
weak_factory_(this) {
Java_BraveRewardsNativeWorker_setNativePtr(env, obj,
reinterpret_cast<intptr_t>(this));

brave_rewards_service_ = brave_rewards::RewardsServiceFactory::GetForProfile(
ProfileManager::GetActiveUserProfile()->GetOriginalProfile());
if (brave_rewards_service_) {
brave_rewards_service_->AddObserver(this);
brave_rewards::RewardsNotificationService* notification_service =
brave_rewards_service_->GetNotificationService();
if (notification_service) {
notification_service->AddObserver(this);
rewards_service_observation_.Observe(brave_rewards_service_);

if (auto* notification_service =
brave_rewards_service_->GetNotificationService()) {
rewards_notification_service_observation_.Observe(notification_service);
}
}
}
Expand All @@ -63,14 +62,6 @@ BraveRewardsNativeWorker::~BraveRewardsNativeWorker() {
}

void BraveRewardsNativeWorker::Destroy(JNIEnv* env) {
if (brave_rewards_service_) {
brave_rewards_service_->RemoveObserver(this);
brave_rewards::RewardsNotificationService* notification_service =
brave_rewards_service_->GetNotificationService();
if (notification_service) {
notification_service->RemoveObserver(this);
}
}
delete this;
}

Expand Down Expand Up @@ -251,8 +242,8 @@ void BraveRewardsNativeWorker::GetPublisherInfo(
int tabId,
const base::android::JavaParamRef<jstring>& host) {
if (brave_rewards_service_) {
brave_rewards_service_->GetPublisherActivityFromUrl(tabId,
base::android::ConvertJavaStringToUTF8(env, host), "", "");
brave_rewards_service_->GetPublisherActivityFromUrl(
tabId, base::android::ConvertJavaStringToUTF8(env, host), "", "");
}
}

Expand Down Expand Up @@ -578,26 +569,16 @@ void BraveRewardsNativeWorker::OnSendContribution(bool result) {
}

void BraveRewardsNativeWorker::GetAllNotifications(JNIEnv* env) {
if (!brave_rewards_service_) {
return;
}
brave_rewards::RewardsNotificationService* notification_service =
brave_rewards_service_->GetNotificationService();
if (notification_service) {
notification_service->GetNotifications();
if (rewards_notification_service_observation_.IsObserving()) {
rewards_notification_service_observation_.GetSource()->GetNotifications();
}
}

void BraveRewardsNativeWorker::DeleteNotification(JNIEnv* env,
const base::android::JavaParamRef<jstring>& notification_id) {
if (!brave_rewards_service_) {
return;
}
brave_rewards::RewardsNotificationService* notification_service =
brave_rewards_service_->GetNotificationService();
if (notification_service) {
notification_service->DeleteNotification(
base::android::ConvertJavaStringToUTF8(env, notification_id));
if (rewards_notification_service_observation_.IsObserving()) {
rewards_notification_service_observation_.GetSource()->DeleteNotification(
base::android::ConvertJavaStringToUTF8(env, notification_id));
}
}

Expand Down Expand Up @@ -704,14 +685,14 @@ double BraveRewardsNativeWorker::GetPublisherRecurrentDonationAmount(
void BraveRewardsNativeWorker::RemoveRecurring(JNIEnv* env,
const base::android::JavaParamRef<jstring>& publisher) {
if (brave_rewards_service_) {
brave_rewards_service_->RemoveRecurringTip(
brave_rewards_service_->RemoveRecurringTip(
base::android::ConvertJavaStringToUTF8(env, publisher));
auto it = map_recurrent_publishers_.find(
base::android::ConvertJavaStringToUTF8(env, publisher));
auto it = map_recurrent_publishers_.find(
base::android::ConvertJavaStringToUTF8(env, publisher));

if (it != map_recurrent_publishers_.end()) {
map_recurrent_publishers_.erase(it);
}
if (it != map_recurrent_publishers_.end()) {
map_recurrent_publishers_.erase(it);
}
}
}

Expand Down
8 changes: 8 additions & 0 deletions browser/brave_rewards/android/brave_rewards_native_worker.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#define BRAVE_BROWSER_BRAVE_REWARDS_ANDROID_BRAVE_REWARDS_NATIVE_WORKER_H_

#include <jni.h>

#include <map>
#include <memory>
#include <string>
Expand All @@ -16,6 +17,7 @@
#include "base/containers/flat_map.h"
#include "base/memory/raw_ptr.h"
#include "base/memory/weak_ptr.h"
#include "base/scoped_observation.h"
#include "brave/components/brave_ads/core/mojom/brave_ads.mojom.h"
#include "brave/components/brave_rewards/browser/rewards_notification_service_observer.h"
#include "brave/components/brave_rewards/browser/rewards_service_observer.h"
Expand Down Expand Up @@ -273,6 +275,12 @@ class BraveRewardsNativeWorker
std::map<std::string, brave_rewards::mojom::PublisherInfoPtr>
map_recurrent_publishers_;
std::map<std::string, std::string> addresses_;
base::ScopedObservation<brave_rewards::RewardsService,
brave_rewards::RewardsServiceObserver>
rewards_service_observation_{this};
base::ScopedObservation<brave_rewards::RewardsNotificationService,
brave_rewards::RewardsNotificationServiceObserver>
rewards_notification_service_observation_{this};
base::WeakPtrFactory<BraveRewardsNativeWorker> weak_factory_;
};

Expand Down
16 changes: 5 additions & 11 deletions browser/brave_rewards/rewards_tab_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,9 @@ RewardsTabHelper::RewardsTabHelper(content::WebContents* web_contents)
if (tab_id_.is_valid()) {
rewards_service_ = RewardsServiceFactory::GetForProfile(
Profile::FromBrowserContext(GetWebContents().GetBrowserContext()));
}

if (rewards_service_) {
rewards_service_->AddObserver(this);
if (rewards_service_) {
rewards_service_observation_.Observe(rewards_service_);
}
}

#if !BUILDFLAG(IS_ANDROID)
Expand All @@ -67,9 +66,6 @@ RewardsTabHelper::RewardsTabHelper(content::WebContents* web_contents)
}

RewardsTabHelper::~RewardsTabHelper() {
if (rewards_service_) {
rewards_service_->RemoveObserver(this);
}
#if !BUILDFLAG(IS_ANDROID)
BrowserList::RemoveObserver(browser_list_observer_.get());
#endif
Expand Down Expand Up @@ -186,10 +182,8 @@ void RewardsTabHelper::OnRewardsInitialized(RewardsService* rewards_service) {
// When Rewards is initialized for the current profile, we need to inform the
// utility process about the currently active tab so that it can start
// measuring auto-contribute correctly.
if (rewards_service_) {
rewards_service_->OnShow(tab_id_);
rewards_service_->OnLoad(tab_id_, GetWebContents().GetLastCommittedURL());
}
rewards_service->OnShow(tab_id_);
rewards_service->OnLoad(tab_id_, GetWebContents().GetLastCommittedURL());
}

void RewardsTabHelper::MaybeSavePublisherInfo() {
Expand Down
5 changes: 4 additions & 1 deletion browser/brave_rewards/rewards_tab_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,10 @@ class RewardsTabHelper : public content::WebContentsUserData<RewardsTabHelper>,
std::unique_ptr<BrowserListObserver> browser_list_observer_;
#endif
SessionID tab_id_;
raw_ptr<RewardsService> rewards_service_ = nullptr; // NOT OWNED
base::ScopedObservation<brave_rewards::RewardsService,
brave_rewards::RewardsServiceObserver>
rewards_service_observation_{this};
raw_ptr<RewardsService> rewards_service_ = nullptr;
base::ObserverList<Observer> observer_list_;
std::string publisher_id_;

Expand Down

0 comments on commit 7faaf9a

Please sign in to comment.