Commit 4dbcd04c authored by Evan Stade's avatar Evan Stade Committed by Commit Bot

Remove notification related dependencies from WebsitePreferenceBridge.

WebLayer won't implement notifications for now, so notifications code
is getting left in //chrome. These dependencies are easy to remove
and replace with generic permissions/prefs code, which will ease the
componentization of WebsitePreferenceBridge.

1. Use PrefServiceBridge to directly set the quiet notification
   preference instead of adding a specific jni interface in
   WebsitePreferenceBridge.
2. Use HostContentSettingsMap to set the notification preference by
   ContentSettingsType rather than using the (thin) wrapper provided
   by NotificationPermissionContext

Bug: 1058600
Change-Id: I450946b545408dbeb1abec6508b3e13f8effb9bc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2122914
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: default avatarNatalie Chouinard <chouinard@chromium.org>
Reviewed-by: default avatarBalazs Engedy <engedy@chromium.org>
Reviewed-by: default avatarFinnur Thorarinsson <finnur@chromium.org>
Cr-Commit-Position: refs/heads/master@{#756021}
parent 73213a96
...@@ -548,7 +548,15 @@ public class SingleCategorySettings extends SiteSettingsPreferenceFragment ...@@ -548,7 +548,15 @@ public class SingleCategorySettings extends SiteSettingsPreferenceFragment
PrefServiceBridge.getInstance().setBoolean( PrefServiceBridge.getInstance().setBoolean(
Pref.NOTIFICATIONS_VIBRATE_ENABLED, (boolean) newValue); Pref.NOTIFICATIONS_VIBRATE_ENABLED, (boolean) newValue);
} else if (NOTIFICATIONS_QUIET_UI_TOGGLE_KEY.equals(preference.getKey())) { } else if (NOTIFICATIONS_QUIET_UI_TOGGLE_KEY.equals(preference.getKey())) {
WebsitePreferenceBridge.setQuietNotificationsUiEnabled((boolean) newValue); boolean boolValue = (boolean) newValue;
if (boolValue) {
PrefServiceBridge.getInstance().setBoolean(
Pref.ENABLE_QUIET_NOTIFICATION_PERMISSION_UI, true);
} else {
// Clear the pref so if the default changes later the user will get the new default.
PrefServiceBridge.getInstance().clearPref(
Pref.ENABLE_QUIET_NOTIFICATION_PERMISSION_UI);
}
} }
return true; return true;
} }
...@@ -1108,7 +1116,8 @@ public class SingleCategorySettings extends SiteSettingsPreferenceFragment ...@@ -1108,7 +1116,8 @@ public class SingleCategorySettings extends SiteSettingsPreferenceFragment
quiet_ui_pref = (ChromeBaseCheckBoxPreference) getPreferenceScreen().findPreference( quiet_ui_pref = (ChromeBaseCheckBoxPreference) getPreferenceScreen().findPreference(
NOTIFICATIONS_QUIET_UI_TOGGLE_KEY); NOTIFICATIONS_QUIET_UI_TOGGLE_KEY);
} }
quiet_ui_pref.setChecked(WebsitePreferenceBridge.isQuietNotificationsUiEnabled()); quiet_ui_pref.setChecked(PrefServiceBridge.getInstance().getBoolean(
Pref.ENABLE_QUIET_NOTIFICATION_PERMISSION_UI));
} else if (quiet_ui_pref != null) { } else if (quiet_ui_pref != null) {
// Save a reference to allow re-adding it to the screen. // Save a reference to allow re-adding it to the screen.
mNotificationsQuietUiPref = quiet_ui_pref; mNotificationsQuietUiPref = quiet_ui_pref;
......
...@@ -541,18 +541,6 @@ public class WebsitePreferenceBridge { ...@@ -541,18 +541,6 @@ public class WebsitePreferenceBridge {
contentSettingType, primaryPattern, secondaryPattern, setting); contentSettingType, primaryPattern, secondaryPattern, setting);
} }
public static boolean isQuietNotificationsUiEnabled() {
return WebsitePreferenceBridgeJni.get().getQuietNotificationsUiEnabled(getProfile());
}
public static void setQuietNotificationsUiEnabled(boolean enabled) {
WebsitePreferenceBridgeJni.get().setQuietNotificationsUiEnabled(getProfile(), enabled);
}
private static Profile getProfile() {
return Profile.getLastUsedRegularProfile();
}
@VisibleForTesting @VisibleForTesting
@NativeMethods @NativeMethods
public interface Natives { public interface Natives {
...@@ -656,7 +644,5 @@ public class WebsitePreferenceBridge { ...@@ -656,7 +644,5 @@ public class WebsitePreferenceBridge {
void setSensorsEnabled(boolean enabled); void setSensorsEnabled(boolean enabled);
void setSoundEnabled(boolean enabled); void setSoundEnabled(boolean enabled);
void setVrEnabled(boolean enabled); void setVrEnabled(boolean enabled);
boolean getQuietNotificationsUiEnabled(Profile profile);
void setQuietNotificationsUiEnabled(Profile profile, boolean enabled);
} }
} }
...@@ -35,6 +35,12 @@ const char* PrefServiceBridge::GetPrefNameExposedToJava(int pref_index) { ...@@ -35,6 +35,12 @@ const char* PrefServiceBridge::GetPrefNameExposedToJava(int pref_index) {
// Native JNI methods // Native JNI methods
// ---------------------------------------------------------------------------- // ----------------------------------------------------------------------------
static void JNI_PrefServiceBridge_ClearPref(JNIEnv* env,
const jint j_pref_index) {
GetPrefService()->ClearPref(
PrefServiceBridge::GetPrefNameExposedToJava(j_pref_index));
}
static jboolean JNI_PrefServiceBridge_GetBoolean( static jboolean JNI_PrefServiceBridge_GetBoolean(
JNIEnv* env, JNIEnv* env,
const jint j_pref_index) { const jint j_pref_index) {
......
...@@ -66,6 +66,7 @@ enum Pref { ...@@ -66,6 +66,7 @@ enum Pref {
WEBKIT_FORCE_DARK_MODE_ENABLED, WEBKIT_FORCE_DARK_MODE_ENABLED,
HOME_PAGE, HOME_PAGE,
AUTOFILL_CREDIT_CARD_FIDO_AUTH_ENABLED, AUTOFILL_CREDIT_CARD_FIDO_AUTH_ENABLED,
ENABLE_QUIET_NOTIFICATION_PERMISSION_UI,
// PREF_NUM_PREFS must be the last entry. // PREF_NUM_PREFS must be the last entry.
PREF_NUM_PREFS PREF_NUM_PREFS
}; };
...@@ -113,6 +114,7 @@ const char* const kPrefsExposedToJava[] = { ...@@ -113,6 +114,7 @@ const char* const kPrefsExposedToJava[] = {
prefs::kWebKitForceDarkModeEnabled, prefs::kWebKitForceDarkModeEnabled,
prefs::kHomePage, prefs::kHomePage,
autofill::prefs::kAutofillCreditCardFidoAuthEnabled, autofill::prefs::kAutofillCreditCardFidoAuthEnabled,
prefs::kEnableQuietNotificationPermissionUi,
}; };
#endif // CHROME_BROWSER_ANDROID_PREFERENCES_PREFS_H_ #endif // CHROME_BROWSER_ANDROID_PREFERENCES_PREFS_H_
...@@ -100,6 +100,8 @@ TEST_F(PrefsTest, TestIndex) { ...@@ -100,6 +100,8 @@ TEST_F(PrefsTest, TestIndex) {
EXPECT_EQ(prefs::kHomePage, GetPrefName(HOME_PAGE)); EXPECT_EQ(prefs::kHomePage, GetPrefName(HOME_PAGE));
EXPECT_EQ(autofill::prefs::kAutofillCreditCardFidoAuthEnabled, EXPECT_EQ(autofill::prefs::kAutofillCreditCardFidoAuthEnabled,
GetPrefName(AUTOFILL_CREDIT_CARD_FIDO_AUTH_ENABLED)); GetPrefName(AUTOFILL_CREDIT_CARD_FIDO_AUTH_ENABLED));
EXPECT_EQ(prefs::kEnableQuietNotificationPermissionUi,
GetPrefName(ENABLE_QUIET_NOTIFICATION_PERMISSION_UI));
// If this check fails, a pref is missing a test case above. // If this check fails, a pref is missing a test case above.
EXPECT_EQ(Pref::PREF_NUM_PREFS, pref_count_); EXPECT_EQ(Pref::PREF_NUM_PREFS, pref_count_);
......
...@@ -20,12 +20,9 @@ ...@@ -20,12 +20,9 @@
#include "base/metrics/user_metrics.h" #include "base/metrics/user_metrics.h"
#include "base/stl_util.h" #include "base/stl_util.h"
#include "chrome/android/chrome_jni_headers/WebsitePreferenceBridge_jni.h" #include "chrome/android/chrome_jni_headers/WebsitePreferenceBridge_jni.h"
#include "chrome/browser/notifications/notification_permission_context.h"
#include "chrome/browser/permissions/quiet_notification_permission_ui_state.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/browser/profiles/profile_android.h" #include "chrome/browser/profiles/profile_android.h"
#include "chrome/browser/profiles/profile_manager.h" #include "chrome/browser/profiles/profile_manager.h"
#include "chrome/common/pref_names.h"
#include "components/browser_ui/site_settings/android/storage_info_fetcher.h" #include "components/browser_ui/site_settings/android/storage_info_fetcher.h"
#include "components/browsing_data/content/local_storage_helper.h" #include "components/browsing_data/content/local_storage_helper.h"
#include "components/cdm/browser/media_drm_storage_impl.h" #include "components/cdm/browser/media_drm_storage_impl.h"
...@@ -485,7 +482,10 @@ static void JNI_WebsitePreferenceBridge_SetNotificationSettingForOrigin( ...@@ -485,7 +482,10 @@ static void JNI_WebsitePreferenceBridge_SetNotificationSettingForOrigin(
profile, url, GURL(), ContentSettingsType::NOTIFICATIONS, profile, url, GURL(), ContentSettingsType::NOTIFICATIONS,
permissions::PermissionSourceUI::SITE_SETTINGS); permissions::PermissionSourceUI::SITE_SETTINGS);
NotificationPermissionContext::UpdatePermission(profile, url, setting); GetHostContentSettingsMap(is_incognito)
->SetContentSettingDefaultScope(
url, GURL(), ContentSettingsType::NOTIFICATIONS,
content_settings::ResourceIdentifier(), setting);
content_settings::LogWebSiteSettingsPermissionChange( content_settings::LogWebSiteSettingsPermissionChange(
ContentSettingsType::NOTIFICATIONS, setting); ContentSettingsType::NOTIFICATIONS, setting);
} }
...@@ -1265,23 +1265,3 @@ static jboolean JNI_WebsitePreferenceBridge_GetMicManagedByCustodian( ...@@ -1265,23 +1265,3 @@ static jboolean JNI_WebsitePreferenceBridge_GetMicManagedByCustodian(
return IsContentSettingManagedByCustodian( return IsContentSettingManagedByCustodian(
ContentSettingsType::MEDIASTREAM_MIC); ContentSettingsType::MEDIASTREAM_MIC);
} }
static jboolean JNI_WebsitePreferenceBridge_GetQuietNotificationsUiEnabled(
JNIEnv* env,
const JavaParamRef<jobject>& jprofile) {
return QuietNotificationPermissionUiState::IsQuietUiEnabledInPrefs(
ProfileAndroid::FromProfileAndroid(jprofile));
}
static void JNI_WebsitePreferenceBridge_SetQuietNotificationsUiEnabled(
JNIEnv* env,
const JavaParamRef<jobject>& jprofile,
jboolean enabled) {
if (enabled) {
QuietNotificationPermissionUiState::EnableQuietUiInPrefs(
ProfileAndroid::FromProfileAndroid(jprofile));
} else {
QuietNotificationPermissionUiState::DisableQuietUiInPrefs(
ProfileAndroid::FromProfileAndroid(jprofile));
}
}
...@@ -346,8 +346,8 @@ TEST_F(ChromePermissionRequestManagerTest, ...@@ -346,8 +346,8 @@ TEST_F(ChromePermissionRequestManagerTest,
auto* permission_ui_enabler = auto* permission_ui_enabler =
AdaptiveQuietNotificationPermissionUiEnabler::GetForProfile(profile()); AdaptiveQuietNotificationPermissionUiEnabler::GetForProfile(profile());
EXPECT_FALSE( EXPECT_FALSE(profile()->GetPrefs()->GetBoolean(
QuietNotificationPermissionUiState::IsQuietUiEnabledInPrefs(profile())); prefs::kEnableQuietNotificationPermissionUi));
// TODO(hkamila): Collapse the below blocks into a single for statement. // TODO(hkamila): Collapse the below blocks into a single for statement.
GURL notification1("http://www.notification1.com/"); GURL notification1("http://www.notification1.com/");
NavigateAndCommit(notification1); NavigateAndCommit(notification1);
...@@ -437,8 +437,8 @@ TEST_F(ChromePermissionRequestManagerTest, ...@@ -437,8 +437,8 @@ TEST_F(ChromePermissionRequestManagerTest,
manager_->AddRequest(&notification7_request); manager_->AddRequest(&notification7_request);
WaitForBubbleToBeShown(); WaitForBubbleToBeShown();
EXPECT_TRUE(manager_->ShouldCurrentRequestUseQuietUI()); EXPECT_TRUE(manager_->ShouldCurrentRequestUseQuietUI());
EXPECT_TRUE( EXPECT_TRUE(profile()->GetPrefs()->GetBoolean(
QuietNotificationPermissionUiState::IsQuietUiEnabledInPrefs(profile())); prefs::kEnableQuietNotificationPermissionUi));
Accept(); Accept();
base::SimpleTestClock clock_; base::SimpleTestClock clock_;
...@@ -462,7 +462,7 @@ TEST_F(ChromePermissionRequestManagerTest, ...@@ -462,7 +462,7 @@ TEST_F(ChromePermissionRequestManagerTest,
// not change the state of the currently showing quiet UI. // not change the state of the currently showing quiet UI.
permission_ui_enabler->ClearInteractionHistory(base::Time(), permission_ui_enabler->ClearInteractionHistory(base::Time(),
base::Time::Max()); base::Time::Max());
QuietNotificationPermissionUiState::DisableQuietUiInPrefs(profile()); profile()->GetPrefs()->ClearPref(prefs::kEnableQuietNotificationPermissionUi);
EXPECT_TRUE(manager_->ShouldCurrentRequestUseQuietUI()); EXPECT_TRUE(manager_->ShouldCurrentRequestUseQuietUI());
Deny(); Deny();
...@@ -574,8 +574,10 @@ TEST_F(ChromePermissionRequestManagerTest, TestCrowdDenyHoldbackChance) { ...@@ -574,8 +574,10 @@ TEST_F(ChromePermissionRequestManagerTest, TestCrowdDenyHoldbackChance) {
{QuietNotificationPermissionUiConfig::kCrowdDenyHoldBackChance, {QuietNotificationPermissionUiConfig::kCrowdDenyHoldBackChance,
test.holdback_chance}}); test.holdback_chance}});
if (test.enabled_in_prefs) if (test.enabled_in_prefs) {
QuietNotificationPermissionUiState::EnableQuietUiInPrefs(profile()); profile()->GetPrefs()->SetBoolean(
prefs::kEnableQuietNotificationPermissionUi, true);
}
permissions::MockPermissionRequest request( permissions::MockPermissionRequest request(
"request", permissions::PermissionRequestType::PERMISSION_NOTIFICATIONS, "request", permissions::PermissionRequestType::PERMISSION_NOTIFICATIONS,
......
...@@ -17,9 +17,12 @@ ...@@ -17,9 +17,12 @@
#include "chrome/browser/permissions/crowd_deny_preload_data.h" #include "chrome/browser/permissions/crowd_deny_preload_data.h"
#include "chrome/browser/permissions/quiet_notification_permission_ui_config.h" #include "chrome/browser/permissions/quiet_notification_permission_ui_config.h"
#include "chrome/browser/permissions/quiet_notification_permission_ui_state.h" #include "chrome/browser/permissions/quiet_notification_permission_ui_state.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/safe_browsing/safe_browsing_service.h" #include "chrome/browser/safe_browsing/safe_browsing_service.h"
#include "chrome/common/chrome_features.h" #include "chrome/common/chrome_features.h"
#include "chrome/common/pref_names.h"
#include "components/permissions/permission_request.h" #include "components/permissions/permission_request.h"
#include "components/prefs/pref_service.h"
#include "components/safe_browsing/core/db/database_manager.h" #include "components/safe_browsing/core/db/database_manager.h"
namespace { namespace {
...@@ -172,7 +175,8 @@ void ContextualNotificationPermissionUiSelector::OnCrowdDenyTriggerEvaluated( ...@@ -172,7 +175,8 @@ void ContextualNotificationPermissionUiSelector::OnCrowdDenyTriggerEvaluated(
// Still show the quiet UI if it is enabled for all sites, even if crowd deny // Still show the quiet UI if it is enabled for all sites, even if crowd deny
// did not trigger showing the quiet UI on this origin. // did not trigger showing the quiet UI on this origin.
if (QuietNotificationPermissionUiState::IsQuietUiEnabledInPrefs(profile_)) { if (profile_->GetPrefs()->GetBoolean(
prefs::kEnableQuietNotificationPermissionUi)) {
Notify(UiToUse::kQuietUi, QuietUiReason::kEnabledInPrefs); Notify(UiToUse::kQuietUi, QuietUiReason::kEnabledInPrefs);
return; return;
} }
......
...@@ -5,11 +5,12 @@ ...@@ -5,11 +5,12 @@
#include "base/run_loop.h" #include "base/run_loop.h"
#include "base/test/scoped_feature_list.h" #include "base/test/scoped_feature_list.h"
#include "chrome/browser/infobars/mock_infobar_service.h" #include "chrome/browser/infobars/mock_infobar_service.h"
#include "chrome/browser/permissions/quiet_notification_permission_ui_state.h"
#include "chrome/common/chrome_features.h" #include "chrome/common/chrome_features.h"
#include "chrome/common/pref_names.h"
#include "chrome/test/base/chrome_render_view_host_test_harness.h" #include "chrome/test/base/chrome_render_view_host_test_harness.h"
#include "components/permissions/permission_request_manager.h" #include "components/permissions/permission_request_manager.h"
#include "components/permissions/test/mock_permission_request.h" #include "components/permissions/test/mock_permission_request.h"
#include "components/prefs/pref_service.h"
class PermissionPromptAndroidTest : public ChromeRenderViewHostTestHarness { class PermissionPromptAndroidTest : public ChromeRenderViewHostTestHarness {
public: public:
...@@ -23,7 +24,8 @@ class PermissionPromptAndroidTest : public ChromeRenderViewHostTestHarness { ...@@ -23,7 +24,8 @@ class PermissionPromptAndroidTest : public ChromeRenderViewHostTestHarness {
// Ensure that the test uses the mini-infobar variant. // Ensure that the test uses the mini-infobar variant.
scoped_feature_list_.InitAndEnableFeature( scoped_feature_list_.InitAndEnableFeature(
features::kQuietNotificationPrompts); features::kQuietNotificationPrompts);
QuietNotificationPermissionUiState::EnableQuietUiInPrefs(profile()); profile()->GetPrefs()->SetBoolean(
prefs::kEnableQuietNotificationPermissionUi, true);
NavigateAndCommit(GURL("http://example.com")); NavigateAndCommit(GURL("http://example.com"));
......
...@@ -25,26 +25,6 @@ void QuietNotificationPermissionUiState::RegisterProfilePrefs( ...@@ -25,26 +25,6 @@ void QuietNotificationPermissionUiState::RegisterProfilePrefs(
false /* default_value */); false /* default_value */);
} }
// static
bool QuietNotificationPermissionUiState::IsQuietUiEnabledInPrefs(
Profile* profile) {
return profile->GetPrefs()->GetBoolean(
prefs::kEnableQuietNotificationPermissionUi);
}
// static
void QuietNotificationPermissionUiState::EnableQuietUiInPrefs(
Profile* profile) {
profile->GetPrefs()->SetBoolean(prefs::kEnableQuietNotificationPermissionUi,
true /* value */);
}
// static
void QuietNotificationPermissionUiState::DisableQuietUiInPrefs(
Profile* profile) {
profile->GetPrefs()->ClearPref(prefs::kEnableQuietNotificationPermissionUi);
}
// static // static
bool QuietNotificationPermissionUiState::ShouldShowPromo(Profile* profile) { bool QuietNotificationPermissionUiState::ShouldShowPromo(Profile* profile) {
return profile->GetPrefs()->GetBoolean( return profile->GetPrefs()->GetBoolean(
......
...@@ -16,15 +16,6 @@ class QuietNotificationPermissionUiState { ...@@ -16,15 +16,6 @@ class QuietNotificationPermissionUiState {
// Register Profile-keyed preferences used for permission UI selection. // Register Profile-keyed preferences used for permission UI selection.
static void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry); static void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry);
// Whether the quiet UI is enabled in settings.
static bool IsQuietUiEnabledInPrefs(Profile* profile);
// Turns on the quiet UI in settings.
static void EnableQuietUiInPrefs(Profile* profile);
// Turns off the quiet UI in settings, as per user request.
static void DisableQuietUiInPrefs(Profile* profile);
// Whether to show a promo for the prompt indicator. // Whether to show a promo for the prompt indicator.
static bool ShouldShowPromo(Profile* profile); static bool ShouldShowPromo(Profile* profile);
......
...@@ -33,6 +33,13 @@ public class PrefServiceBridge { ...@@ -33,6 +33,13 @@ public class PrefServiceBridge {
return sInstance; return sInstance;
} }
/**
* @param preference The name of the preference.
*/
public void clearPref(@Pref int preference) {
PrefServiceBridgeJni.get().clearPref(preference);
}
/** /**
* @param preference The name of the preference. * @param preference The name of the preference.
* @return Whether the specified preference is enabled. * @return Whether the specified preference is enabled.
...@@ -97,6 +104,7 @@ public class PrefServiceBridge { ...@@ -97,6 +104,7 @@ public class PrefServiceBridge {
@NativeMethods @NativeMethods
interface Natives { interface Natives {
void clearPref(int preference);
boolean getBoolean(int preference); boolean getBoolean(int preference);
void setBoolean(int preference, boolean value); void setBoolean(int preference, boolean value);
int getInteger(int preference); int getInteger(int preference);
......
...@@ -21,6 +21,7 @@ ...@@ -21,6 +21,7 @@
#include "chrome/browser/ui/tabs/tab_strip_model.h" #include "chrome/browser/ui/tabs/tab_strip_model.h"
#include "chrome/common/chrome_features.h" #include "chrome/common/chrome_features.h"
#include "chrome/common/chrome_switches.h" #include "chrome/common/chrome_switches.h"
#include "chrome/common/pref_names.h"
#include "chrome/grit/generated_resources.h" #include "chrome/grit/generated_resources.h"
#include "chrome/test/base/browser_with_test_window_test.h" #include "chrome/test/base/browser_with_test_window_test.h"
#include "chrome/test/base/chrome_render_view_host_test_harness.h" #include "chrome/test/base/chrome_render_view_host_test_harness.h"
...@@ -397,7 +398,8 @@ TEST_F(ContentSettingImageModelTest, NotificationsPrompt) { ...@@ -397,7 +398,8 @@ TEST_F(ContentSettingImageModelTest, NotificationsPrompt) {
auto* profile = auto* profile =
Profile::FromBrowserContext(web_contents()->GetBrowserContext()); Profile::FromBrowserContext(web_contents()->GetBrowserContext());
QuietNotificationPermissionUiState::EnableQuietUiInPrefs(profile); profile->GetPrefs()->SetBoolean(prefs::kEnableQuietNotificationPermissionUi,
true);
auto content_setting_image_model = auto content_setting_image_model =
ContentSettingImageModel::CreateForContentType( ContentSettingImageModel::CreateForContentType(
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment