Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix user earns while native Windows and macOS notifications are suppressed #3084

Merged
merged 1 commit into from
Nov 6, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion components/brave_ads/browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ source_set("browser") {
"notification_helper_android.h",
"notification_helper_linux.cc",
"notification_helper_linux.h",
"notification_helper_mac.cc",
"notification_helper_mac.mm",
"notification_helper_mac.h",
"notification_helper_win.cc",
"notification_helper_win.h",
Expand All @@ -88,6 +88,12 @@ source_set("browser") {
"//ui/message_center/public/cpp",
]

if (is_mac) {
libs = [
"UserNotifications.framework",
]
}

if (is_win) {
deps += [
"//ui/views",
Expand Down
3 changes: 2 additions & 1 deletion components/brave_ads/browser/ads_service_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
#include "brave/components/brave_rewards/common/pref_names.h"
#include "brave/components/services/bat_ads/public/cpp/ads_client_mojo_bridge.h"
#include "brave/components/services/bat_ads/public/interfaces/bat_ads.mojom.h"
#include "brave/components/brave_ads/browser/notification_helper.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/notifications/notification_display_service.h"
#include "chrome/browser/notifications/notification_display_service_impl.h"
Expand Down Expand Up @@ -1922,7 +1923,7 @@ void AdsServiceImpl::ShowNotification(
#endif
}

bool AdsServiceImpl::ShouldShowNotifications() const {
bool AdsServiceImpl::ShouldShowNotifications() {
masparrow marked this conversation as resolved.
Show resolved Hide resolved
return NotificationHelper::GetInstance()->ShouldShowNotifications();
}

Expand Down
2 changes: 1 addition & 1 deletion components/brave_ads/browser/ads_service_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@ class AdsServiceImpl : public AdsService,

void ShowNotification(
std::unique_ptr<ads::NotificationInfo> info) override;
bool ShouldShowNotifications() const override;
bool ShouldShowNotifications() override;
void CloseNotification(
const std::string& id) override;

Expand Down
23 changes: 19 additions & 4 deletions components/brave_ads/browser/notification_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,25 +7,40 @@

namespace brave_ads {

NotificationHelper* g_notification_helper_for_testing = nullptr;

NotificationHelper::NotificationHelper() = default;

NotificationHelper::~NotificationHelper() = default;

bool NotificationHelper::ShouldShowNotifications() const {
void NotificationHelper::set_for_testing(
NotificationHelper* notification_helper) {
g_notification_helper_for_testing = notification_helper;
}

bool NotificationHelper::ShouldShowNotifications() {
return true;
}

bool NotificationHelper::ShowMyFirstAdNotification() const {
bool NotificationHelper::ShowMyFirstAdNotification() {
return false;
}

bool NotificationHelper::CanShowBackgroundNotifications() const {
return true;
}

#if !defined(OS_MACOSX) && !defined(OS_WIN) && !defined(OS_LINUX) && !defined(OS_ANDROID) // NOLINT
NotificationHelper* NotificationHelper::GetInstance() {
// just return a dummy notification helper for all other platforms
if (g_notification_helper_for_testing) {
return g_notification_helper_for_testing;
}

return GetInstanceImpl();
}

#if !defined(OS_MACOSX) && !defined(OS_WIN) && !defined(OS_LINUX) && !defined(OS_ANDROID) // NOLINT
NotificationHelper* NotificationHelper::GetInstanceImpl() {
// Return a default notification helper for unsupported platforms
return base::Singleton<NotificationHelper>::get();
}
#endif
Expand Down
11 changes: 7 additions & 4 deletions components/brave_ads/browser/notification_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@
#ifndef BRAVE_COMPONENTS_BRAVE_ADS_BROWSER_NOTIFICATION_HELPER_H_
#define BRAVE_COMPONENTS_BRAVE_ADS_BROWSER_NOTIFICATION_HELPER_H_

#include <string>

#include "base/macros.h"
#include "base/memory/singleton.h"
#include "build/build_config.h"
Expand All @@ -18,16 +16,21 @@ class NotificationHelper {
public:
static NotificationHelper* GetInstance();

virtual bool ShouldShowNotifications() const;
void set_for_testing(
NotificationHelper* notification_helper);

virtual bool ShouldShowNotifications();

virtual bool ShowMyFirstAdNotification() const;
virtual bool ShowMyFirstAdNotification();

virtual bool CanShowBackgroundNotifications() const;

protected:
NotificationHelper();
virtual ~NotificationHelper();

static NotificationHelper* GetInstanceImpl();

private:
friend struct base::DefaultSingletonTraits<NotificationHelper>;
DISALLOW_COPY_AND_ASSIGN(NotificationHelper);
Expand Down
27 changes: 14 additions & 13 deletions components/brave_ads/browser/notification_helper_android.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ NotificationHelperAndroid::NotificationHelperAndroid() = default;

NotificationHelperAndroid::~NotificationHelperAndroid() = default;

bool NotificationHelperAndroid::ShouldShowNotifications() const {
bool NotificationHelperAndroid::ShouldShowNotifications() {
JNIEnv* env = base::android::AttachCurrentThread();
int status = Java_NotificationSystemStatusUtil_getAppNotificationStatus(env);
bool is_notifications_enabled = (status == kAppNotificationsStatusEnabled ||
Expand All @@ -40,12 +40,11 @@ bool NotificationHelperAndroid::ShouldShowNotifications() const {
auto should_show_notifications =
CanShowBackgroundNotifications() || is_foreground;

return is_notifications_enabled &&
is_notification_channel_enabled &&
should_show_notifications;
return is_notifications_enabled && is_notification_channel_enabled &&
should_show_notifications;
}

bool NotificationHelperAndroid::ShowMyFirstAdNotification() const {
bool NotificationHelperAndroid::ShowMyFirstAdNotification() {
if (!ShouldShowNotifications()) {
return false;
}
Expand All @@ -61,6 +60,16 @@ bool NotificationHelperAndroid::CanShowBackgroundNotifications() const {
return Java_BraveAdsSignupDialog_showAdsInBackground(env);
}

NotificationHelperAndroid* NotificationHelperAndroid::GetInstanceImpl() {
return base::Singleton<NotificationHelperAndroid>::get();
}

NotificationHelper* NotificationHelper::GetInstanceImpl() {
return NotificationHelperAndroid::GetInstanceImpl();
}

///////////////////////////////////////////////////////////////////////////////

bool NotificationHelperAndroid::IsBraveAdsNotificationChannelEnabled() const {
if (GetOperatingSystemVersion() < kMinimumVersionForNotificationChannels) {
return true;
Expand All @@ -86,12 +95,4 @@ int NotificationHelperAndroid::GetOperatingSystemVersion() const {
return major_version;
}

NotificationHelperAndroid* NotificationHelperAndroid::GetInstance() {
return base::Singleton<NotificationHelperAndroid>::get();
}

NotificationHelper* NotificationHelper::GetInstance() {
return NotificationHelperAndroid::GetInstance();
}

} // namespace brave_ads
18 changes: 8 additions & 10 deletions components/brave_ads/browser/notification_helper_android.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,25 +8,23 @@

#include <memory>

#include "base/macros.h"
#include "base/memory/singleton.h"
#include "base/memory/weak_ptr.h"

#include "brave/components/brave_ads/browser/notification_helper.h"
#include "chrome/browser/notifications/notification_channels_provider_android.h"

namespace brave_ads {

class NotificationHelperAndroid :
public NotificationHelper,
public base::SupportsWeakPtr<NotificationHelperAndroid> {
class NotificationHelperAndroid
: public NotificationHelper,
public base::SupportsWeakPtr<NotificationHelperAndroid> {
public:
static NotificationHelperAndroid* GetInstanceImpl();

private:
NotificationHelperAndroid();
~NotificationHelperAndroid() override;

static NotificationHelperAndroid* GetInstance();

private:
bool IsBraveAdsNotificationChannelEnabled() const;

std::unique_ptr<NotificationChannelsProviderAndroid> channels_provider_ =
Expand All @@ -35,9 +33,9 @@ class NotificationHelperAndroid :
int GetOperatingSystemVersion() const;

// NotificationHelper impl
bool ShouldShowNotifications() const override;
bool ShouldShowNotifications() override;

bool ShowMyFirstAdNotification() const override;
bool ShowMyFirstAdNotification() override;

bool CanShowBackgroundNotifications() const override;

Expand Down
22 changes: 15 additions & 7 deletions components/brave_ads/browser/notification_helper_linux.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
* You can obtain one at http://mozilla.org/MPL/2.0/. */

#include "brave/components/brave_ads/browser/notification_helper_linux.h"

#include "chrome/browser/fullscreen.h"

namespace brave_ads {
Expand All @@ -13,24 +12,33 @@ NotificationHelperLinux::NotificationHelperLinux() = default;

NotificationHelperLinux::~NotificationHelperLinux() = default;

bool NotificationHelperLinux::ShouldShowNotifications() const {
return !IsFullScreenMode();
bool NotificationHelperLinux::ShouldShowNotifications() {
if (IsFullScreenMode()) {
LOG(WARNING) << "Notification not made: Full screen mode";
return false;
}

// TODO(https://github.com/brave/brave-browser/issues/5542): Investigate how
// we can detect if notifications are enabled within the Linux operating
// system

return true;
}

bool NotificationHelperLinux::ShowMyFirstAdNotification() const {
bool NotificationHelperLinux::ShowMyFirstAdNotification() {
return false;
}

bool NotificationHelperLinux::CanShowBackgroundNotifications() const {
return true;
}

NotificationHelperLinux* NotificationHelperLinux::GetInstance() {
NotificationHelperLinux* NotificationHelperLinux::GetInstanceImpl() {
return base::Singleton<NotificationHelperLinux>::get();
}

NotificationHelper* NotificationHelper::GetInstance() {
return NotificationHelperLinux::GetInstance();
NotificationHelper* NotificationHelper::GetInstanceImpl() {
return NotificationHelperLinux::GetInstanceImpl();
}

} // namespace brave_ads
18 changes: 8 additions & 10 deletions components/brave_ads/browser/notification_helper_linux.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,28 +6,26 @@
#ifndef BRAVE_COMPONENTS_BRAVE_ADS_BROWSER_NOTIFICATION_HELPER_LINUX_H_
#define BRAVE_COMPONENTS_BRAVE_ADS_BROWSER_NOTIFICATION_HELPER_LINUX_H_

#include "base/macros.h"
#include "base/memory/singleton.h"
#include "base/memory/weak_ptr.h"

#include "brave/components/brave_ads/browser/notification_helper.h"

namespace brave_ads {

class NotificationHelperLinux :
public NotificationHelper,
public base::SupportsWeakPtr<NotificationHelperLinux> {
class NotificationHelperLinux
: public NotificationHelper,
public base::SupportsWeakPtr<NotificationHelperLinux> {
public:
static NotificationHelperLinux* GetInstanceImpl();

private:
NotificationHelperLinux();
~NotificationHelperLinux() override;

static NotificationHelperLinux* GetInstance();

private:
// NotificationHelper impl
bool ShouldShowNotifications() const override;
bool ShouldShowNotifications() override;

bool ShowMyFirstAdNotification() const override;
bool ShowMyFirstAdNotification() override;

bool CanShowBackgroundNotifications() const override;

Expand Down
36 changes: 0 additions & 36 deletions components/brave_ads/browser/notification_helper_mac.cc

This file was deleted.

18 changes: 9 additions & 9 deletions components/brave_ads/browser/notification_helper_mac.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,28 +6,28 @@
#ifndef BRAVE_COMPONENTS_BRAVE_ADS_BROWSER_NOTIFICATION_HELPER_MAC_H_
#define BRAVE_COMPONENTS_BRAVE_ADS_BROWSER_NOTIFICATION_HELPER_MAC_H_

#include "base/macros.h"
#include "base/memory/singleton.h"
#include "base/memory/weak_ptr.h"

#include "brave/components/brave_ads/browser/notification_helper.h"

namespace brave_ads {

class NotificationHelperMac :
public NotificationHelper,
public base::SupportsWeakPtr<NotificationHelperMac> {
class NotificationHelperMac
: public NotificationHelper,
public base::SupportsWeakPtr<NotificationHelperMac> {
public:
static NotificationHelperMac* GetInstanceImpl();

private:
NotificationHelperMac();
~NotificationHelperMac() override;

static NotificationHelperMac* GetInstance();
bool IsNotificationsEnabled() const;

private:
// NotificationHelper impl
bool ShouldShowNotifications() const override;
bool ShouldShowNotifications() override;

bool ShowMyFirstAdNotification() const override;
bool ShowMyFirstAdNotification() override;

bool CanShowBackgroundNotifications() const override;

Expand Down
Loading