Skip to content

Commit

Permalink
Decouple AdsServiceImpl full screen mode business logic
Browse files Browse the repository at this point in the history
  • Loading branch information
tmancey committed Mar 17, 2021
1 parent c00cb7f commit d6ecdf8
Show file tree
Hide file tree
Showing 26 changed files with 221 additions and 23 deletions.
2 changes: 1 addition & 1 deletion components/brave_ads/browser/DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ specific_include_rules = {
"+brave/browser/notifications/brave_notification_platform_bridge_helper_android.h",
"+chrome/common/chrome_paths.h",
"+chrome/browser/browser_process.h",
"+chrome/browser/fullscreen.h",
"+chrome/browser/notifications/notification_display_service.h",
"+chrome/browser/profiles/profile_manager.h",
"+chrome/browser/profiles/profile.h",
Expand Down Expand Up @@ -77,7 +78,6 @@ specific_include_rules = {
"+brave/common/brave_channel_info.h",
"+chrome/android/chrome_jni_headers/NotificationSystemStatusUtil_jni.h",
"+chrome/browser/notifications/notification_channels_provider_android.h",
"+chrome/browser/fullscreen.h",
"+chrome/common/chrome_features.h",
"+chrome/install_static/install_util.h",
"+chrome/installer/util/install_util.h",
Expand Down
5 changes: 5 additions & 0 deletions components/brave_ads/browser/ads_service_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
#include "brave/components/services/bat_ads/public/interfaces/bat_ads.mojom.h"
#include "brave/grit/brave_generated_resources.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/fullscreen.h"
#include "chrome/browser/notifications/notification_display_service.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/profiles/profile_manager.h"
Expand Down Expand Up @@ -1720,6 +1721,10 @@ bool AdsServiceImpl::IsForeground() const {
return BackgroundHelper::GetInstance()->IsForeground();
}

bool AdsServiceImpl::IsFullScreen() const {
return IsFullScreenMode();
}

std::string AdsServiceImpl::GetLocale() const {
return brave_l10n::LocaleHelper::GetInstance()->GetLocale();
}
Expand Down
2 changes: 2 additions & 0 deletions components/brave_ads/browser/ads_service_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,8 @@ class AdsServiceImpl : public AdsService,

bool IsForeground() const override;

bool IsFullScreen() const override;

bool ShouldShowNotifications() override;

bool CanShowBackgroundNotifications() const override;
Expand Down
6 changes: 0 additions & 6 deletions components/brave_ads/browser/notification_helper_linux.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

#include "base/logging.h"
#include "brave/common/brave_channel_info.h"
#include "chrome/browser/fullscreen.h"

namespace brave_ads {

Expand All @@ -16,11 +15,6 @@ NotificationHelperLinux::NotificationHelperLinux() = default;
NotificationHelperLinux::~NotificationHelperLinux() = default;

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

if (brave::IsNightlyOrDeveloperBuild()) {
return true;
}
Expand Down
9 changes: 1 addition & 8 deletions components/brave_ads/browser/notification_helper_mac.mm
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,8 @@
#include "base/logging.h"
#include "base/mac/mac_util.h"
#include "brave/common/brave_channel_info.h"
#include "chrome/common/chrome_features.h"

#include "brave/components/brave_ads/browser/notification_helper_mac.h"
#include "chrome/browser/fullscreen.h"
#include "chrome/common/chrome_features.h"

namespace brave_ads {

Expand All @@ -26,11 +24,6 @@
NotificationHelperMac::~NotificationHelperMac() = default;

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

if (brave::IsNightlyOrDeveloperBuild()) {
return true;
}
Expand Down
10 changes: 2 additions & 8 deletions components/brave_ads/browser/notification_helper_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,16 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

#include <Windows.h>

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

#include <Windows.h>

#include "base/feature_list.h"
#include "base/logging.h"
#include "base/win/core_winrt_util.h"
#include "base/win/scoped_hstring.h"
#include "base/win/windows_version.h"
#include "brave/common/brave_channel_info.h"
#include "chrome/browser/fullscreen.h"
#include "chrome/common/chrome_features.h"
#include "chrome/install_static/install_util.h"
#include "chrome/installer/util/install_util.h"
Expand Down Expand Up @@ -66,11 +65,6 @@ NotificationHelperWin::NotificationHelperWin() = default;
NotificationHelperWin::~NotificationHelperWin() = default;

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

if (brave::IsNightlyOrDeveloperBuild()) {
return true;
}
Expand Down
1 change: 1 addition & 0 deletions components/brave_ads/test/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ source_set("brave_ads_unit_tests") {
"//brave/vendor/bat-native-ads/src/bat/ads/internal/frequency_capping/permission_rules/browser_is_active_frequency_cap_unittest.cc",
"//brave/vendor/bat-native-ads/src/bat/ads/internal/frequency_capping/permission_rules/catalog_frequency_cap_unittest.cc",
"//brave/vendor/bat-native-ads/src/bat/ads/internal/frequency_capping/permission_rules/do_not_disturb_frequency_cap_unittest.cc",
"//brave/vendor/bat-native-ads/src/bat/ads/internal/frequency_capping/permission_rules/full_screen_mode_frequency_cap_unittest.cc",
"//brave/vendor/bat-native-ads/src/bat/ads/internal/frequency_capping/permission_rules/media_frequency_cap_unittest.cc",
"//brave/vendor/bat-native-ads/src/bat/ads/internal/frequency_capping/permission_rules/minimum_wait_time_frequency_cap_unittest.cc",
"//brave/vendor/bat-native-ads/src/bat/ads/internal/frequency_capping/permission_rules/network_connection_frequency_cap_unittest.cc",
Expand Down
10 changes: 10 additions & 0 deletions components/services/bat_ads/bat_ads_client_mojo_bridge.cc
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,16 @@ bool BatAdsClientMojoBridge::IsForeground() const {
return is_foreground;
}

bool BatAdsClientMojoBridge::IsFullScreen() const {
if (!connected()) {
return false;
}

bool is_full_screen;
bat_ads_client_->IsFullScreen(&is_full_screen);
return is_full_screen;
}

void BatAdsClientMojoBridge::ShowNotification(
const ads::AdNotificationInfo& ad_notification) {
if (!connected()) {
Expand Down
2 changes: 2 additions & 0 deletions components/services/bat_ads/bat_ads_client_mojo_bridge.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ class BatAdsClientMojoBridge

bool IsForeground() const override;

bool IsFullScreen() const override;

void ShowNotification(
const ads::AdNotificationInfo& ad_notification) override;
bool ShouldShowNotifications() override;
Expand Down
10 changes: 10 additions & 0 deletions components/services/bat_ads/public/cpp/ads_client_mojo_bridge.cc
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,16 @@ void AdsClientMojoBridge::IsForeground(
std::move(callback).Run(ads_client_->IsForeground());
}

bool AdsClientMojoBridge::IsFullScreen(bool* out_is_full_screen) {
DCHECK(out_is_full_screen);
*out_is_full_screen = ads_client_->IsFullScreen();
return true;
}

void AdsClientMojoBridge::IsFullScreen(IsFullScreenCallback callback) {
std::move(callback).Run(ads_client_->IsFullScreen());
}

bool AdsClientMojoBridge::CanShowBackgroundNotifications(
bool* out_can_show) {
DCHECK(out_can_show);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ class AdsClientMojoBridge
bool* out_is_foreground) override;
void IsForeground(
IsForegroundCallback callback) override;
bool IsFullScreen(bool* out_is_full_screen) override;
void IsFullScreen(IsFullScreenCallback callback) override;
bool IsNetworkConnectionAvailable(
bool* out_available) override;
void IsNetworkConnectionAvailable(
Expand Down
2 changes: 2 additions & 0 deletions components/services/bat_ads/public/interfaces/bat_ads.mojom
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ interface BatAdsClient {
[Sync]
IsForeground() => (bool is_foreground);
[Sync]
IsFullScreen() => (bool is_full_screen);
[Sync]
ShouldShowNotifications() => (bool should_show);
[Sync]
CanShowBackgroundNotifications() => (bool can_show);
Expand Down
2 changes: 2 additions & 0 deletions vendor/bat-native-ads/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,8 @@ source_set("ads") {
"src/bat/ads/internal/frequency_capping/permission_rules/catalog_frequency_cap.h",
"src/bat/ads/internal/frequency_capping/permission_rules/do_not_disturb_frequency_cap.cc",
"src/bat/ads/internal/frequency_capping/permission_rules/do_not_disturb_frequency_cap.h",
"src/bat/ads/internal/frequency_capping/permission_rules/full_screen_mode_frequency_cap.cc",
"src/bat/ads/internal/frequency_capping/permission_rules/full_screen_mode_frequency_cap.h",
"src/bat/ads/internal/frequency_capping/permission_rules/media_frequency_cap.cc",
"src/bat/ads/internal/frequency_capping/permission_rules/media_frequency_cap.h",
"src/bat/ads/internal/frequency_capping/permission_rules/minimum_wait_time_frequency_cap.cc",
Expand Down
3 changes: 3 additions & 0 deletions vendor/bat-native-ads/include/bat/ads/ads_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ class ADS_EXPORT AdsClient {
// false
virtual bool IsForeground() const = 0;

// Return true if the browser is full screen otherwise return false
virtual bool IsFullScreen() const = 0;

// Return true if notifications should be displayed otherwise return false
virtual bool ShouldShowNotifications() = 0;

Expand Down
2 changes: 2 additions & 0 deletions vendor/bat-native-ads/src/bat/ads/internal/ads_client_mock.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ class AdsClientMock : public AdsClient {

MOCK_CONST_METHOD0(IsForeground, bool());

MOCK_CONST_METHOD0(IsFullScreen, bool());

MOCK_METHOD0(ShouldShowNotifications, bool());

MOCK_CONST_METHOD0(CanShowBackgroundNotifications, bool());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include "bat/ads/internal/frequency_capping/permission_rules/browser_is_active_frequency_cap.h"
#include "bat/ads/internal/frequency_capping/permission_rules/catalog_frequency_cap.h"
#include "bat/ads/internal/frequency_capping/permission_rules/do_not_disturb_frequency_cap.h"
#include "bat/ads/internal/frequency_capping/permission_rules/full_screen_mode_frequency_cap.h"
#include "bat/ads/internal/frequency_capping/permission_rules/media_frequency_cap.h"
#include "bat/ads/internal/frequency_capping/permission_rules/minimum_wait_time_frequency_cap.h"
#include "bat/ads/internal/frequency_capping/permission_rules/network_connection_frequency_cap.h"
Expand Down Expand Up @@ -58,6 +59,11 @@ bool FrequencyCapping::IsAdAllowed() {
return false;
}

FullScreenModeFrequencyCap full_screen_mode_frequency_cap;
if (!ShouldAllow(&full_screen_mode_frequency_cap)) {
return false;
}

BrowserIsActiveFrequencyCap browser_is_active_frequency_cap;
if (!ShouldAllow(&browser_is_active_frequency_cap)) {
return false;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/* Copyright (c) 2020 The Brave Authors. All rights reserved.
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

#include "bat/ads/internal/frequency_capping/permission_rules/full_screen_mode_frequency_cap.h"

#include "bat/ads/internal/ads_client_helper.h"
#include "bat/ads/internal/frequency_capping/frequency_capping_util.h"
#include "bat/ads/internal/platform/platform_helper.h"

namespace ads {

FullScreenModeFrequencyCap::FullScreenModeFrequencyCap() = default;

FullScreenModeFrequencyCap::~FullScreenModeFrequencyCap() = default;

bool FullScreenModeFrequencyCap::ShouldAllow() {
if (!DoesRespectCap()) {
last_message_ = "Full screen mode";
return false;
}

return true;
}

std::string FullScreenModeFrequencyCap::get_last_message() const {
return last_message_;
}

bool FullScreenModeFrequencyCap::DoesRespectCap() {
if (PlatformHelper::GetInstance()->IsMobile()) {
return true;
}

return !AdsClientHelper::Get()->IsFullScreen();
}

} // namespace ads
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/* Copyright (c) 2020 The Brave Authors. All rights reserved.
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

#ifndef BRAVE_VENDOR_BAT_NATIVE_ADS_SRC_BAT_ADS_INTERNAL_FREQUENCY_CAPPING_PERMISSION_RULES_FULL_SCREEN_MODE_FREQUENCY_CAP_H_
#define BRAVE_VENDOR_BAT_NATIVE_ADS_SRC_BAT_ADS_INTERNAL_FREQUENCY_CAPPING_PERMISSION_RULES_FULL_SCREEN_MODE_FREQUENCY_CAP_H_

#include <string>

#include "bat/ads/internal/frequency_capping/permission_rules/permission_rule.h"

namespace ads {

class FullScreenModeFrequencyCap : public PermissionRule {
public:
FullScreenModeFrequencyCap();

~FullScreenModeFrequencyCap() override;

FullScreenModeFrequencyCap(const FullScreenModeFrequencyCap&) = delete;
FullScreenModeFrequencyCap& operator=(const FullScreenModeFrequencyCap&) =
delete;

bool ShouldAllow() override;

std::string get_last_message() const override;

private:
std::string last_message_;

bool DoesRespectCap();
};

} // namespace ads

#endif // BRAVE_VENDOR_BAT_NATIVE_ADS_SRC_BAT_ADS_INTERNAL_FREQUENCY_CAPPING_PERMISSION_RULES_FULL_SCREEN_MODE_FREQUENCY_CAP_H_
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
/* Copyright (c) 2020 The Brave Authors. All rights reserved.
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

#include "bat/ads/internal/frequency_capping/permission_rules/full_screen_mode_frequency_cap.h"

#include "bat/ads/internal/unittest_base.h"
#include "bat/ads/internal/unittest_util.h"

// npm run test -- brave_unit_tests --filter=BatAds*

namespace ads {

class BatAdsFullScreenModeFrequencyCapTest : public UnitTestBase {
protected:
BatAdsFullScreenModeFrequencyCapTest() = default;

~BatAdsFullScreenModeFrequencyCapTest() override = default;
};

TEST_F(BatAdsFullScreenModeFrequencyCapTest, AllowAd) {
// Arrange
MockIsFullScreen(ads_client_mock_, false);

// Act
FullScreenModeFrequencyCap frequency_cap;
const bool is_allowed = frequency_cap.ShouldAllow();

// Assert
EXPECT_TRUE(is_allowed);
}

TEST_F(BatAdsFullScreenModeFrequencyCapTest, AlwaysAllowAdForAndroid) {
// Arrange
MockPlatformHelper(platform_helper_mock_, PlatformType::kAndroid);

MockIsFullScreen(ads_client_mock_, true);

// Act
FullScreenModeFrequencyCap frequency_cap;
const bool is_allowed = frequency_cap.ShouldAllow();

// Assert
EXPECT_TRUE(is_allowed);
}

TEST_F(BatAdsFullScreenModeFrequencyCapTest, AlwaysAllowAdForIOS) {
// Arrange
MockPlatformHelper(platform_helper_mock_, PlatformType::kIOS);

MockIsFullScreen(ads_client_mock_, true);

// Act
FullScreenModeFrequencyCap frequency_cap;
const bool is_allowed = frequency_cap.ShouldAllow();

// Assert
EXPECT_TRUE(is_allowed);
}

TEST_F(BatAdsFullScreenModeFrequencyCapTest, DoNotAllowAd) {
// Arrange
MockIsFullScreen(ads_client_mock_, true);

// Act
FullScreenModeFrequencyCap frequency_cap;
const bool is_allowed = frequency_cap.ShouldAllow();

// Assert
EXPECT_FALSE(is_allowed);
}

} // namespace ads
2 changes: 2 additions & 0 deletions vendor/bat-native-ads/src/bat/ads/internal/unittest_base.cc
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,8 @@ void UnitTestBase::Initialize() {

MockIsForeground(ads_client_mock_, true);

MockIsFullScreen(ads_client_mock_, false);

MockShouldShowNotifications(ads_client_mock_, true);
MockShowNotification(ads_client_mock_);
MockCloseNotification(ads_client_mock_);
Expand Down
Loading

0 comments on commit d6ecdf8

Please sign in to comment.