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

Don't show widevine install prompt when user doesn't want #2980

Merged
merged 1 commit into from
Aug 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
6 changes: 6 additions & 0 deletions app/brave_generated_resources.grd
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,12 @@
<message name="IDS_WALLET_PERMISSION_REQUEST_TEXT_FRAGMENT" desc="Text fragment for wallet installation permission request.">
Install Crypto Wallets
</message>
<message name="IDS_WIDEVINE_DONT_ASK_AGAIN_CHECKBOX" desc="Checkbox for prevent showing widevine install prompt">
Don't ask again
</message>
<message name="IDS_SETTINGS_ASK_WIDEVINE_INSTALL_DESC" desc="Text fragment for asking widevine install or not">
Ask when a site wants to install Widevine on your computer.
</message>
<if expr="is_linux">
<message name="IDS_WIDEVINE_PERMISSION_REQUEST_TEXT_FRAGMENT_INSTALL" desc="Text fragment for Widevine permission request. 'Widevine' is the name of a plugin and should not be translated.">
Install Widevine
Expand Down
6 changes: 4 additions & 2 deletions browser/brave_drm_tab_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,10 @@ BraveDrmTabHelper::~BraveDrmTabHelper() {}

bool BraveDrmTabHelper::ShouldShowWidevineOptIn() const {
// If the user already opted in, don't offer it.
PrefService* prefs = ProfileManager::GetActiveUserProfile()->GetPrefs();
if (prefs->GetBoolean(kWidevineOptedIn)) {
PrefService* prefs =
static_cast<Profile*>(web_contents()->GetBrowserContext())->GetPrefs();
if (prefs->GetBoolean(kWidevineOptedIn) ||
!prefs->GetBoolean(kAskWidevineInstall)) {
return false;
}

Expand Down
1 change: 1 addition & 0 deletions browser/brave_profile_prefs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry) {
#endif

registry->RegisterBooleanPref(kWidevineOptedIn, false);
registry->RegisterBooleanPref(kAskWidevineInstall, true);
#if BUILDFLAG(BUNDLE_WIDEVINE_CDM)
BraveWidevineBundleManager::RegisterProfilePrefs(registry);
#endif
Expand Down
2 changes: 2 additions & 0 deletions browser/extensions/api/settings_private/brave_prefs_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ const PrefsUtil::TypedPrefMap& BravePrefsUtil::GetWhitelistedKeys() {
settings_api::PrefType::PREF_TYPE_BOOLEAN;
(*s_brave_whitelist)[kBraveThemeType] =
settings_api::PrefType::PREF_TYPE_NUMBER;
(*s_brave_whitelist)[kAskWidevineInstall] =
settings_api::PrefType::PREF_TYPE_BOOLEAN;
// Clear browsing data on exit prefs.
(*s_brave_whitelist)[browsing_data::prefs::kDeleteBrowsingHistoryOnExit] =
settings_api::PrefType::PREF_TYPE_BOOLEAN;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,11 @@
sub-label="$i18n{ipfsCompanionEnabledDesc}"
on-settings-boolean-control-change="onIPFSCompanionEnabledChange_">
</settings-toggle-button>
<settings-toggle-button
pref="{{prefs.brave.ask_widevine_install}}"
label="Widevine"
sub-label="$i18n{appearanceSettingsAskWidevineInstallDesc}">
</settings-toggle-button>
<settings-toggle-button id="mediaRouterEnabled"
pref="{{prefs.brave.enable_media_router}}"
label="Media Router"
Expand Down
2 changes: 1 addition & 1 deletion browser/widevine/widevine_permission_request.cc
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ void WidevinePermissionRequest::PermissionGranted() {
}

void WidevinePermissionRequest::PermissionDenied() {
// Do nothing.
DontAskWidevineInstall(web_contents_, dont_ask_widevine_install_);
}

void WidevinePermissionRequest::Cancelled() {
Expand Down
5 changes: 5 additions & 0 deletions browser/widevine/widevine_permission_request.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ class WidevinePermissionRequest : public PermissionRequest {
~WidevinePermissionRequest() override;

base::string16 GetExplanatoryMessageText() const;
void set_dont_ask_widevine_install(bool dont_ask) {
dont_ask_widevine_install_ = dont_ask;
}

private:
// PermissionRequest overrides:
Expand All @@ -36,6 +39,8 @@ class WidevinePermissionRequest : public PermissionRequest {
// by PermissionManager that is tied with this |web_contents_|.
content::WebContents* web_contents_;

bool dont_ask_widevine_install_ = false;

DISALLOW_COPY_AND_ASSIGN(WidevinePermissionRequest);
};

Expand Down
23 changes: 21 additions & 2 deletions browser/widevine/widevine_permission_request_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,24 @@ IN_PROC_BROWSER_TEST_F(WidevinePermissionRequestBrowserTest, VisibilityTest) {
drm_tab_helper->OnWidevineKeySystemAccessRequest();
content::RunAllTasksUntilIdle();
EXPECT_TRUE(observer.bubble_added_);

// Check permission bubble is not visible when user turns it off.
observer.bubble_added_ = false;
DontAskWidevineInstall(GetActiveWebContents(), true);
EXPECT_TRUE(content::NavigateToURL(GetActiveWebContents(),
GURL("chrome://newtab/")));
drm_tab_helper->OnWidevineKeySystemAccessRequest();
content::RunAllTasksUntilIdle();
EXPECT_FALSE(observer.bubble_added_);

// Check permission bubble is visible when user turns it on.
observer.bubble_added_ = false;
DontAskWidevineInstall(GetActiveWebContents(), false);
EXPECT_TRUE(content::NavigateToURL(GetActiveWebContents(),
GURL("chrome://newtab/")));
drm_tab_helper->OnWidevineKeySystemAccessRequest();
content::RunAllTasksUntilIdle();
EXPECT_TRUE(observer.bubble_added_);
}

// Check extra text is added.
Expand All @@ -122,8 +140,9 @@ IN_PROC_BROWSER_TEST_F(WidevinePermissionRequestBrowserTest, BubbleTest) {
DCHECK(delegate_view);
// Original PermissionsBubbleDialogDelegateView has one child.
// It's label that includes icon and fragment test.
// For widevine permission requests, one more label is added.
EXPECT_EQ(2ull, delegate_view->children().size());
// For widevine permission requests, two more child views are added.
// one for extra label and the other one is do not ask checkbox.
EXPECT_EQ(3ul, delegate_view->children().size());
}

// OptedInPref of bundling tests are done by
Expand Down
13 changes: 10 additions & 3 deletions browser/widevine/widevine_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,12 @@

#include "brave/browser/brave_browser_process_impl.h"
#include "brave/browser/widevine/widevine_permission_request.h"
#include "brave/common/pref_names.h"
#include "brave/grit/brave_generated_resources.h"
#include "chrome/browser/permissions/permission_request_manager.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/subresource_filter/chrome_subresource_filter_client.h"
#include "components/prefs/pref_service.h"

#if BUILDFLAG(BUNDLE_WIDEVINE_CDM)
#include <string>
Expand All @@ -24,10 +27,8 @@
#endif

#if BUILDFLAG(ENABLE_WIDEVINE_CDM_COMPONENT)
#include "brave/common/pref_names.h"
#include "chrome/browser/component_updater/widevine_cdm_component_installer.h"
#include "chrome/browser/profiles/profile_manager.h"
#include "components/prefs/pref_service.h"
#endif

#if BUILDFLAG(BUNDLE_WIDEVINE_CDM)
Expand Down Expand Up @@ -88,7 +89,8 @@ void RequestWidevinePermission(content::WebContents* web_contents) {
void EnableWidevineCdmComponent(content::WebContents* web_contents) {
DCHECK(web_contents);

PrefService* prefs = ProfileManager::GetActiveUserProfile()->GetPrefs();
PrefService* prefs =
static_cast<Profile*>(web_contents->GetBrowserContext())->GetPrefs();
if (prefs->GetBoolean(kWidevineOptedIn))
return;

Expand Down Expand Up @@ -120,3 +122,8 @@ void InstallBundleOrRestartBrowser() {
}
}
#endif

void DontAskWidevineInstall(content::WebContents* web_contents, bool dont_ask) {
Profile* profile = static_cast<Profile*>(web_contents->GetBrowserContext());
profile->GetPrefs()->SetBoolean(kAskWidevineInstall, !dont_ask);
}
2 changes: 2 additions & 0 deletions browser/widevine/widevine_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,6 @@ void InstallBundleOrRestartBrowser();
void EnableWidevineCdmComponent(content::WebContents* web_contents);
#endif

void DontAskWidevineInstall(content::WebContents* web_contents, bool dont_ask);

#endif // BRAVE_BROWSER_WIDEVINE_WIDEVINE_UTILS_H_
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,43 @@
#include <vector>

#include "brave/browser/widevine/widevine_permission_request.h"
#include "brave/grit/brave_generated_resources.h"
#include "chrome/browser/permissions/permission_request.h"
#include "chrome/browser/ui/views/chrome_layout_provider.h"
#include "chrome/browser/ui/views/chrome_typography.h"
#include "ui/gfx/text_constants.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/views/controls/button/checkbox.h"
#include "ui/views/controls/label.h"
#include "ui/views/window/dialog_delegate.h"

namespace {

class DontAskAgainCheckbox : public views::Checkbox,
public views::ButtonListener {
public:
explicit DontAskAgainCheckbox(WidevinePermissionRequest* request);

private:
// views::ButtonListener overrides:
void ButtonPressed(Button* sender, const ui::Event& event) override;

WidevinePermissionRequest* request_;

DISALLOW_COPY_AND_ASSIGN(DontAskAgainCheckbox);
};

DontAskAgainCheckbox::DontAskAgainCheckbox(WidevinePermissionRequest* request)
: Checkbox(l10n_util::GetStringUTF16(IDS_WIDEVINE_DONT_ASK_AGAIN_CHECKBOX),
this),
request_(request) {
}

void DontAskAgainCheckbox::ButtonPressed(Button* sender,
const ui::Event& event) {
request_->set_dont_ask_widevine_install(GetChecked());
}

bool HasWidevinePermissionRequest(
const std::vector<PermissionRequest*>& requests) {
// When widevine permission is requested, |requests| only includes Widevine
Expand All @@ -27,7 +55,7 @@ bool HasWidevinePermissionRequest(
return false;
}

void AddWidevineExplanatoryMessageTextIfNeeded(
void AddAdditionalWidevineViewControlsIfNeeded(
views::DialogDelegateView* dialog_delegate,
const std::vector<PermissionRequest*>& requests) {
if (!HasWidevinePermissionRequest(requests))
Expand All @@ -47,6 +75,7 @@ void AddWidevineExplanatoryMessageTextIfNeeded(
// Resize width. Then, it's height deduced.
text->SizeToFit(preferred_dialog_width - dialog_delegate->margins().width());
dialog_delegate->AddChildView(text);
dialog_delegate->AddChildView(new DontAskAgainCheckbox(widevine_request));
}

} // namespace
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ void BraveAddCommonStrings(content::WebUIDataSource* html_source,
IDS_SETTINGS_BRAVE_GET_STARTED_TITLE},
{"braveAdditionalSettingsTitle",
IDS_SETTINGS_BRAVE_ADDITIONAL_SETTINGS},
{"appearanceSettingsAskWidevineInstallDesc",
IDS_SETTINGS_ASK_WIDEVINE_INSTALL_DESC},
{"appearanceSettingsBraveTheme",
IDS_SETTINGS_APPEARANCE_SETTINGS_BRAVE_THEMES},
{"appearanceSettingsLocationBarIsWide",
Expand Down
1 change: 1 addition & 0 deletions common/pref_names.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ const char kAdBlockCustomFilters[] = "brave.ad_block.custom_filters";
const char kAdBlockRegionalFilters[] = "brave.ad_block.regional_filters";
const char kWidevineOptedIn[] = "brave.widevine_opted_in";
const char kWidevineInstalledVersion[] = "brave.widevine_installed_version";
const char kAskWidevineInstall[] = "brave.ask_widevine_install";
const char kUseAlternativeSearchEngineProvider[] =
"brave.use_alternate_private_search_engine";
const char kAlternativeSearchEngineProviderInTor[] =
Expand Down
1 change: 1 addition & 0 deletions common/pref_names.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ extern const char kAdBlockCustomFilters[];
extern const char kAdBlockRegionalFilters[];
extern const char kWidevineOptedIn[];
extern const char kWidevineInstalledVersion[];
extern const char kAskWidevineInstall[];
extern const char kUseAlternativeSearchEngineProvider[];
extern const char kAlternativeSearchEngineProviderInTor[];
extern const char kBraveThemeType[];
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,12 @@
diff --git a/chrome/browser/ui/views/permission_bubble/permission_prompt_impl.cc b/chrome/browser/ui/views/permission_bubble/permission_prompt_impl.cc
index 489db17efe96db01666c57e01f6be0c2d37be873..8564f7450243c3a3e12aa96c338d71e4c072e4b3 100644
index 489db17efe96db01666c57e01f6be0c2d37be873..84d5a95b37b2219a4abbb3c28554be0168b12562 100644
--- a/chrome/browser/ui/views/permission_bubble/permission_prompt_impl.cc
+++ b/chrome/browser/ui/views/permission_bubble/permission_prompt_impl.cc
@@ -148,6 +148,10 @@ PermissionsBubbleDialogDelegateView::PermissionsBubbleDialogDelegateView(
@@ -148,6 +148,7 @@ PermissionsBubbleDialogDelegateView::PermissionsBubbleDialogDelegateView(
AddChildView(label_container);
}

+#if defined(BRAVE_CHROMIUM_BUILD)
+ AddWidevineExplanatoryMessageTextIfNeeded(this, requests);
+#endif
+
+ AddAdditionalWidevineViewControlsIfNeeded(this, requests);
chrome::RecordDialogCreation(chrome::DialogIdentifier::PERMISSIONS);
}