Review fixes

This commit is contained in:
wchen342
2023-09-01 18:38:58 +03:00
parent 52fc4aaa52
commit f8640535a9
14 changed files with 61 additions and 44 deletions
@@ -23,7 +23,7 @@ import org.chromium.components.browser_ui.settings.SettingsUtils;
/* Class for Media section of main preferences */
public class MediaPreferences
extends BravePreferenceFragment implements Preference.OnPreferenceChangeListener {
public static final String PREF_ENABLE_WIDEVINE = "enable_widevine";
public static final String PREF_WIDEVINE_OPTED_IN = "widevine_opted_in";
public static final String PREF_BACKGROUND_VIDEO_PLAYBACK = "background_video_playback";
@Override
@@ -41,7 +41,7 @@ public class MediaPreferences
super.onActivityCreated(savedInstanceState);
ChromeSwitchPreference enableWidevinePref =
(ChromeSwitchPreference) findPreference(PREF_ENABLE_WIDEVINE);
(ChromeSwitchPreference) findPreference(PREF_WIDEVINE_OPTED_IN);
if (enableWidevinePref != null) {
enableWidevinePref.setChecked(
BraveLocalState.get().getBoolean(BravePref.WIDEVINE_OPTED_IN));
@@ -63,9 +63,9 @@ public class MediaPreferences
public boolean onPreferenceChange(Preference preference, Object newValue) {
String key = preference.getKey();
boolean shouldRelaunch = false;
if (PREF_ENABLE_WIDEVINE.equals(key)) {
if (PREF_WIDEVINE_OPTED_IN.equals(key)) {
ChromeSwitchPreference enableWidevinePref =
(ChromeSwitchPreference) findPreference(PREF_ENABLE_WIDEVINE);
(ChromeSwitchPreference) findPreference(PREF_WIDEVINE_OPTED_IN);
BraveLocalState.get().setBoolean(BravePref.WIDEVINE_OPTED_IN,
!BraveLocalState.get().getBoolean(BravePref.WIDEVINE_OPTED_IN));
shouldRelaunch = true;
+1 -1
View File
@@ -231,7 +231,7 @@
</message>
</if>
<if expr="is_android">
<message name="IDS_WIDEVINE_PERMISSION_REQUEST_TEXT_FRAGMENT_ENABLE_SYSTEM" desc="Text fragment for Widevine permission request. 'Widevine' is the name of a plugin and should not be translated.">
<message name="IDS_WIDEVINE_PERMISSION_REQUEST_TEXT_FRAGMENT_ANDROID" desc="Text fragment for Widevine permission request. 'Widevine' is the name of a plugin and should not be translated.">
<ph name="URL">$1<ex>https://www.youtube.com</ex></ph> wants to play protected Google Widevine content, which requires you to allow Android system support for Widevine. By allowing, you'll agree to Google's terms of use.
</message>
</if>
+33 -11
View File
@@ -43,10 +43,10 @@ source_set("widevine") {
]
}
source_set("unittest") {
source_set("widevine_cdm_component_installer_unittest") {
testonly = true
sources = []
sources = [ "widevine_cdm_component_installer_unittest.cc" ]
deps = [
"//base",
"//brave/common",
@@ -60,19 +60,41 @@ source_set("unittest") {
"//testing/gtest",
"//third_party/widevine/cdm:buildflags",
]
}
source_set("widevine_permission_android_unittest") {
testonly = true
sources = [ "widevine_permission_android_unittest.cc" ]
deps = [
":widevine",
"//base",
"//brave/common",
"//chrome/browser:browser",
"//chrome/browser/profiles:profile",
"//chrome/test:test_support",
"//components/permissions",
"//components/pref_registry",
"//components/prefs",
"//content/public/browser",
"//content/test:test_support",
"//testing/gmock",
"//testing/gtest",
"//third_party/widevine/cdm:buildflags",
]
}
source_set("unittests") {
testonly = true
sources = []
deps = []
if (enable_widevine_cdm_component) {
sources += [ "widevine_cdm_component_installer_unittest.cc" ]
deps += [ ":widevine_cdm_component_installer_unittest" ]
}
if (is_android) {
sources += [ "widevine_permission_android_unittest.cc" ]
deps += [
":widevine",
"//chrome/browser:browser",
"//chrome/test:test_support",
"//components/prefs:test_support",
"//content/test:test_support",
]
deps += [ ":widevine_permission_android_unittest" ]
}
}
@@ -90,10 +90,11 @@ class WidevinePermissionAndroidTest : public ChromeRenderViewHostTestHarness {
private:
std::unique_ptr<TestingProfileManager> profile_manager_;
raw_ptr<TestingProfile> profile_;
raw_ptr<TestingProfile> profile_ = nullptr;
std::unique_ptr<content::WebContents> web_contents_;
raw_ptr<BraveDrmTabHelper> tab_helper_;
raw_ptr<permissions::PermissionRequestManager> permission_request_manager_;
raw_ptr<BraveDrmTabHelper> tab_helper_ = nullptr;
raw_ptr<permissions::PermissionRequestManager> permission_request_manager_ =
nullptr;
};
TEST_F(WidevinePermissionAndroidTest, BraveDrmTabHelperTest) {
@@ -176,9 +177,9 @@ TEST_F(WidevinePermissionAndroidTest, WidevinePermissionRequestTest) {
TEST_F(WidevinePermissionAndroidTest, PermissionWidevineUtilsTest) {
SanityCheck();
permissions::DontAskWidevineInstall(profile()->GetPrefs(), true);
permissions::AskWidevineInstall(profile()->GetPrefs(), false);
EXPECT_FALSE(profile()->GetPrefs()->GetBoolean(kAskWidevineInstall));
permissions::DontAskWidevineInstall(profile()->GetPrefs(), false);
permissions::AskWidevineInstall(profile()->GetPrefs(), true);
EXPECT_TRUE(profile()->GetPrefs()->GetBoolean(kAskWidevineInstall));
std::vector<permissions::PermissionRequest*> requests;
@@ -76,7 +76,7 @@ void WidevinePermissionRequest::PermissionDecided(ContentSetting result,
} else if (result == ContentSetting::CONTENT_SETTING_BLOCK) {
Profile* profile =
static_cast<Profile*>(web_contents_->GetBrowserContext());
permissions::DontAskWidevineInstall(profile->GetPrefs(), GetDontAskAgain());
permissions::AskWidevineInstall(profile->GetPrefs(), !get_dont_ask_again());
// Cancelled
} else {
DCHECK(result == CONTENT_SETTING_DEFAULT);
@@ -109,10 +109,10 @@ IN_PROC_BROWSER_TEST_F(WidevinePermissionRequestBrowserTest, VisibilityTest) {
// Check permission bubble is not visible when user turns it off.
observer.bubble_added_ = false;
permissions::DontAskWidevineInstall(
permissions::AskWidevineInstall(
static_cast<Profile*>(GetActiveWebContents()->GetBrowserContext())
->GetPrefs(),
true);
false);
EXPECT_TRUE(content::NavigateToURL(GetActiveWebContents(),
GURL("chrome://newtab/")));
drm_tab_helper->OnWidevineKeySystemAccessRequest();
@@ -121,10 +121,10 @@ IN_PROC_BROWSER_TEST_F(WidevinePermissionRequestBrowserTest, VisibilityTest) {
// Check permission bubble is visible when user turns it on.
observer.bubble_added_ = false;
permissions::DontAskWidevineInstall(
permissions::AskWidevineInstall(
static_cast<Profile*>(GetActiveWebContents()->GetBrowserContext())
->GetPrefs(),
false);
true);
EXPECT_TRUE(content::NavigateToURL(GetActiveWebContents(),
GURL("chrome://newtab/")));
drm_tab_helper->OnWidevineKeySystemAccessRequest();
+1 -1
View File
@@ -110,7 +110,7 @@ int GetWidevinePermissionRequestTextFrangmentResourceId(bool for_restart) {
? IDS_WIDEVINE_PERMISSION_REQUEST_TEXT_FRAGMENT_RESTART_BROWSER
: IDS_WIDEVINE_PERMISSION_REQUEST_TEXT_FRAGMENT_INSTALL;
#elif BUILDFLAG(IS_ANDROID)
return IDS_WIDEVINE_PERMISSION_REQUEST_TEXT_FRAGMENT_ENABLE_SYSTEM;
return IDS_WIDEVINE_PERMISSION_REQUEST_TEXT_FRAGMENT_ANDROID;
#else
return IDS_WIDEVINE_PERMISSION_REQUEST_TEXT_FRAGMENT;
#endif
@@ -66,7 +66,7 @@ DontAskAgainCheckbox::DontAskAgainCheckbox(WidevinePermissionRequest* request)
request_(request) {}
void DontAskAgainCheckbox::ButtonPressed() {
request_->SetDontAskAgain(GetChecked());
request_->set_dont_ask_again(GetChecked());
}
void AddAdditionalWidevineViewControlsIfNeeded(
@@ -76,10 +76,10 @@ void ApplyDontAskAgainOption(JNIEnv* env,
return;
}
const bool dontAskAgain =
const bool dont_ask_again =
Java_BravePermissionDialogDelegate_getDontAskAgain(env, obj);
PermissionRequest* request = permission_prompt->delegate()->Requests()[0];
request->SetDontAskAgain(dontAskAgain);
request->set_dont_ask_again(dont_ask_again);
}
} // namespace
@@ -122,14 +122,6 @@ const absl::optional<base::TimeDelta>& PermissionRequest::GetLifetime() const {
return lifetime_;
}
void PermissionRequest::SetDontAskAgain(bool dont_ask_again) {
dont_ask_again_ = dont_ask_again;
}
bool PermissionRequest::GetDontAskAgain() const {
return dont_ask_again_;
}
bool PermissionRequest::IsDuplicateOf(PermissionRequest* other_request) const {
return PermissionRequest_ChromiumImpl::IsDuplicateOf_ChromiumImpl(
other_request);
@@ -34,8 +34,10 @@ class PermissionRequest : public PermissionRequest_ChromiumImpl {
void SetLifetime(absl::optional<base::TimeDelta> lifetime);
const absl::optional<base::TimeDelta>& GetLifetime() const;
void SetDontAskAgain(bool dont_ask_again);
bool GetDontAskAgain() const;
void set_dont_ask_again(bool dont_ask_again) {
dont_ask_again_ = dont_ask_again;
}
bool get_dont_ask_again() const { return dont_ask_again_; }
// We rename upstream's IsDuplicateOf() via a define above and re-declare it
// here to workaround the fact that the PermissionRequest_ChromiumImpl rename
@@ -12,8 +12,8 @@
namespace permissions {
void DontAskWidevineInstall(PrefService* prefs, bool dont_ask) {
prefs->SetBoolean(kAskWidevineInstall, !dont_ask);
void AskWidevineInstall(PrefService* prefs, bool ask) {
prefs->SetBoolean(kAskWidevineInstall, ask);
}
bool HasWidevinePermissionRequest(
@@ -13,7 +13,7 @@ class PrefService;
namespace permissions {
class PermissionRequest;
void DontAskWidevineInstall(PrefService* prefs, bool dont_ask);
void AskWidevineInstall(PrefService* prefs, bool ask);
bool HasWidevinePermissionRequest(
const std::vector<permissions::PermissionRequest*>& requests);
+1 -1
View File
@@ -326,7 +326,7 @@ test("brave_unit_tests") {
}
if (enable_widevine) {
deps += [ "//brave/browser/widevine:unittest" ]
deps += [ "//brave/browser/widevine:unittests" ]
}
if (enable_brave_vpn) {