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 widevine install prompt is shown after using new profiles #3959

Merged
merged 4 commits into from
Nov 19, 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
3 changes: 3 additions & 0 deletions .github/CODEOWNERS
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ vendor/bat-native-ledger/src/bat/ledger/internal/static_values.h @gdregalo
# Network
browser/net/ @iefremov

# Widevine
browser/widevine @simonhong

# Licensing of third-party components
common/licenses/ @fmarier
components/brave_new_tab_ui/data/backgrounds.ts @fmarier
Expand Down
12 changes: 6 additions & 6 deletions browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,6 @@ source_set("browser_process") {
"brave_browser_process_impl.h",
"brave_content_browser_client.cc",
"brave_content_browser_client.h",
"brave_drm_tab_helper.cc",
"brave_drm_tab_helper.h",
"brave_local_state_prefs.cc",
"brave_local_state_prefs.h",
"brave_profile_prefs.cc",
Expand Down Expand Up @@ -161,6 +159,7 @@ source_set("browser_process") {
"themes",
"//services/network/public/cpp",
"//services/service_manager/embedder",
"//third_party/widevine/cdm:buildflags",
"//ui/base",
]

Expand Down Expand Up @@ -253,11 +252,12 @@ source_set("browser_process") {
]
}

if (bundle_widevine_cdm || enable_widevine_cdm_component) {
deps += [
"//brave/browser/widevine",
"//third_party/widevine/cdm:buildflags",
if (enable_widevine) {
sources += [
"brave_drm_tab_helper.cc",
"brave_drm_tab_helper.h",
]
deps += [ "//brave/browser/widevine" ]
}

if (is_win && is_official_build) {
Expand Down
12 changes: 2 additions & 10 deletions browser/brave_drm_tab_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,13 @@

#include "brave/browser/brave_drm_tab_helper.h"

#include "brave/browser/widevine/widevine_utils.h"
#include "brave/common/pref_names.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/profiles/profile_manager.h"
#include "components/prefs/pref_service.h"
#include "content/public/browser/navigation_handle.h"

#if BUILDFLAG(ENABLE_WIDEVINE_CDM_COMPONENT) || BUILDFLAG(BUNDLE_WIDEVINE_CDM)
#include "brave/browser/widevine/widevine_utils.h"
#endif

BraveDrmTabHelper::BraveDrmTabHelper(content::WebContents* contents)
: WebContentsObserver(contents), bindings_(contents, this) {}

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

Expand All @@ -39,20 +35,16 @@ void BraveDrmTabHelper::DidStartNavigation(
return;
}
is_widevine_requested_ = false;
#if BUILDFLAG(ENABLE_WIDEVINE_CDM_COMPONENT) || BUILDFLAG(BUNDLE_WIDEVINE_CDM)
is_permission_requested_ = false;
#endif
}

void BraveDrmTabHelper::OnWidevineKeySystemAccessRequest() {
is_widevine_requested_ = true;

#if BUILDFLAG(ENABLE_WIDEVINE_CDM_COMPONENT) || BUILDFLAG(BUNDLE_WIDEVINE_CDM)
if (ShouldShowWidevineOptIn() && !is_permission_requested_) {
is_permission_requested_ = true;
RequestWidevinePermission(web_contents());
}
#endif
}

WEB_CONTENTS_USER_DATA_KEY_IMPL(BraveDrmTabHelper)
3 changes: 0 additions & 3 deletions browser/brave_drm_tab_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
#include "content/public/browser/web_contents_binding_set.h"
#include "content/public/browser/web_contents_observer.h"
#include "content/public/browser/web_contents_user_data.h"
#include "third_party/widevine/cdm/buildflags.h"

// Reacts to DRM content detected on the renderer side.
class BraveDrmTabHelper final
Expand All @@ -35,12 +34,10 @@ class BraveDrmTabHelper final
private:
content::WebContentsFrameBindingSet<brave_drm::mojom::BraveDRM> bindings_;

#if BUILDFLAG(ENABLE_WIDEVINE_CDM_COMPONENT) || BUILDFLAG(BUNDLE_WIDEVINE_CDM)
// Permission request is done only once during the navigation. If user
// chooses dismiss/deny, additional request is added again only when new
// main frame navigation is started.
bool is_permission_requested_ = false;
#endif

// True if we are notified that a page requested widevine availability.
bool is_widevine_requested_ = false;
Expand Down
14 changes: 11 additions & 3 deletions browser/brave_local_state_prefs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "chrome/browser/first_run/first_run.h"
#include "components/metrics/metrics_pref_names.h"
#include "components/prefs/pref_registry_simple.h"
#include "third_party/widevine/cdm/buildflags.h"

#if BUILDFLAG(ENABLE_BRAVE_REFERRALS)
#include "brave/components/brave_referrals/browser/brave_referrals_service.h"
Expand All @@ -31,6 +32,10 @@
#include "brave/components/p3a/p3a_core_metrics.h"
#endif // !defined(OS_ANDROID)

#if BUILDFLAG(ENABLE_WIDEVINE)
#include "brave/browser/widevine/widevine_utils.h"
#endif

namespace brave {

void RegisterLocalStatePrefs(PrefRegistrySimple* registry) {
Expand All @@ -46,9 +51,6 @@ void RegisterLocalStatePrefs(PrefRegistrySimple* registry) {
#endif
#if BUILDFLAG(ENABLE_TOR)
tor::TorProfileService::RegisterLocalStatePrefs(registry);
#endif
#if !defined(OS_ANDROID)
RegisterPrefsForMuonMigration(registry);
#endif
registry->SetDefaultPrefValue(
metrics::prefs::kMetricsReportingEnabled,
Expand All @@ -60,9 +62,15 @@ void RegisterLocalStatePrefs(PrefRegistrySimple* registry) {

brave_shields::RegisterShieldsP3APrefs(registry);
#if !defined(OS_ANDROID)
RegisterPrefsForMuonMigration(registry);

BraveWindowTracker::RegisterPrefs(registry);
BraveUptimeTracker::RegisterPrefs(registry);
#endif

#if BUILDFLAG(ENABLE_WIDEVINE)
RegisterWidevineLocalstatePrefs(registry);
#endif
}

} // namespace brave
25 changes: 17 additions & 8 deletions browser/brave_profile_prefs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,6 @@
#include "extensions/common/feature_switch.h"
#include "third_party/widevine/cdm/buildflags.h"

#if BUILDFLAG(BUNDLE_WIDEVINE_CDM)
#include "brave/browser/widevine/brave_widevine_bundle_manager.h"
#endif

#if BUILDFLAG(ENABLE_BRAVE_WEBTORRENT)
#include "brave/components/brave_webtorrent/browser/webtorrent_util.h"
#endif
Expand All @@ -38,10 +34,21 @@
#include "components/gcm_driver/gcm_channel_status_syncer.h"
#endif

#if BUILDFLAG(ENABLE_WIDEVINE)
#include "brave/browser/widevine/widevine_utils.h"
#endif

using extensions::FeatureSwitch;

namespace brave {

void RegisterProfilePrefsForMigration(
user_prefs::PrefRegistrySyncable* registry) {
#if BUILDFLAG(ENABLE_WIDEVINE)
RegisterWidevineProfilePrefsForMigration(registry);
#endif
}

void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry) {
brave_shields::BraveShieldsWebContentsObserver::RegisterProfilePrefs(
registry);
Expand All @@ -52,11 +59,11 @@ void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry) {

brave_sync::prefs::Prefs::RegisterProfilePrefs(registry);

registry->RegisterBooleanPref(kWidevineOptedIn, false);
// TODO(shong): Migrate this to local state also and guard in ENABLE_WIDEVINE.
// We don't need to display "don't ask widevine prompt option" in settings
// if widevine is disabled.
// F/u issue: https://github.com/brave/brave-browser/issues/7000
registry->RegisterBooleanPref(kAskWidevineInstall, true);
#if BUILDFLAG(BUNDLE_WIDEVINE_CDM)
BraveWidevineBundleManager::RegisterProfilePrefs(registry);
#endif

// Default Brave shields
registry->RegisterBooleanPref(kHTTPSEVerywhereControlType, true);
Expand Down Expand Up @@ -169,6 +176,8 @@ void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry) {
registry->RegisterStringPref(kBraveWalletAES256GCMSivNonce, "");
registry->RegisterStringPref(kBraveWalletEncryptedSeed, "");
registry->RegisterBooleanPref(kBraveWalletEnabled, true);

RegisterProfilePrefsForMigration(registry);
}

} // namespace brave
2 changes: 0 additions & 2 deletions browser/brave_profile_prefs_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@ IN_PROC_BROWSER_TEST_F(BraveProfilePrefsBrowserTest, DownloadPromptDefault) {
}

IN_PROC_BROWSER_TEST_F(BraveProfilePrefsBrowserTest, MiscBravePrefs) {
EXPECT_FALSE(
browser()->profile()->GetPrefs()->GetBoolean(kWidevineOptedIn));
EXPECT_TRUE(browser()->profile()->GetPrefs()->GetBoolean(
kHTTPSEVerywhereControlType));
EXPECT_FALSE(
Expand Down
11 changes: 9 additions & 2 deletions browser/brave_tab_helpers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@

#include "brave/browser/brave_tab_helpers.h"

#include "brave/browser/brave_drm_tab_helper.h"
#include "brave/browser/ui/bookmark/brave_bookmark_tab_helper.h"
#include "brave/components/brave_ads/browser/ads_tab_helper.h"
#include "brave/components/brave_rewards/browser/buildflags/buildflags.h"
#include "brave/components/brave_shields/browser/brave_shields_web_contents_observer.h"
#include "brave/components/brave_shields/browser/buildflags/buildflags.h" // For STP
#include "brave/components/greaselion/browser/buildflags/buildflags.h"
#include "content/public/browser/web_contents.h"
#include "third_party/widevine/cdm/buildflags.h"

#if BUILDFLAG(ENABLE_GREASELION)
#include "brave/browser/greaselion/greaselion_tab_helper.h"
Expand All @@ -32,6 +32,10 @@
#include "brave/components/brave_shields/browser/tracking_protection_service.h"
#endif

#if BUILDFLAG(ENABLE_WIDEVINE)
#include "brave/browser/brave_drm_tab_helper.h"
#endif

namespace brave {

void AttachTabHelpers(content::WebContents* web_contents) {
Expand All @@ -46,7 +50,6 @@ void AttachTabHelpers(content::WebContents* web_contents) {
BackgroundVideoPlaybackTabHelper::CreateForWebContents(web_contents);
#else
// Add tab helpers here unless they are intended for android too
BraveDrmTabHelper::CreateForWebContents(web_contents);
BraveBookmarkTabHelper::CreateForWebContents(web_contents);
#endif

Expand All @@ -60,6 +63,10 @@ void AttachTabHelpers(content::WebContents* web_contents) {
}
#endif

#if BUILDFLAG(ENABLE_WIDEVINE)
BraveDrmTabHelper::CreateForWebContents(web_contents);
#endif

brave_ads::AdsTabHelper::CreateForWebContents(web_contents);
}

Expand Down
1 change: 1 addition & 0 deletions browser/widevine/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ source_set("widevine") {
"//components/prefs",
"//components/pref_registry",
"//components/services/unzip/public/cpp",
"//content/public/browser",
"//services/network/public/cpp",
"//services/service_manager/public/cpp",
"//third_party/widevine/cdm:headers",
Expand Down
39 changes: 8 additions & 31 deletions browser/widevine/brave_widevine_bundle_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,13 @@
#include "base/strings/string_piece.h"
#include "base/task/post_task.h"
#include "brave/browser/widevine/brave_widevine_bundle_unzipper.h"
#include "brave/common/pref_names.h"
#include "brave/browser/widevine/widevine_utils.h"
#include "brave/common/brave_switches.h"
#include "brave/grit/brave_generated_resources.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/net/system_network_context_manager.h"
#include "chrome/browser/profiles/profile_manager.h"
#include "chrome/common/chrome_paths.h"
#include "components/prefs/pref_service.h"
#include "components/pref_registry/pref_registry_syncable.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/common/cdm_info.h"
#include "content/public/browser/cdm_registry.h"
Expand Down Expand Up @@ -61,38 +59,23 @@ base::Optional<base::FilePath> GetTargetWidevineBundleDir() {
void ResetWidevinePrefs() {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);

Profile* profile = ProfileManager::GetActiveUserProfile();
DCHECK(profile);

profile->GetPrefs()->SetBoolean(kWidevineOptedIn, false);
profile->GetPrefs()->SetString(
kWidevineInstalledVersion,
SetWidevineOptedIn(false);
SetWidevineInstalledVersion(
BraveWidevineBundleManager::kWidevineInvalidVersion);
}

void SetWidevinePrefsAsInstalledState() {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);

Profile* profile = ProfileManager::GetActiveUserProfile();
DCHECK(profile);

profile->GetPrefs()->SetBoolean(kWidevineOptedIn, true);
profile->GetPrefs()->SetString(kWidevineInstalledVersion,
WIDEVINE_CDM_VERSION_STRING);
SetWidevineOptedIn(true);
SetWidevineInstalledVersion(WIDEVINE_CDM_VERSION_STRING);
}

} // namespace

// static
char BraveWidevineBundleManager::kWidevineInvalidVersion[] = "";

// static
void BraveWidevineBundleManager::RegisterProfilePrefs(
user_prefs::PrefRegistrySyncable* registry) {
registry->RegisterStringPref(kWidevineInstalledVersion,
kWidevineInvalidVersion);
}

BraveWidevineBundleManager::BraveWidevineBundleManager() : weak_factory_(this) {
}

Expand Down Expand Up @@ -267,19 +250,15 @@ void BraveWidevineBundleManager::StartupCheck() {
return;
}

Profile* profile = ProfileManager::GetActiveUserProfile();
DCHECK(profile);

// Although this case would be very rare, this might be happen because
// bundle unzipping and prefs setting is done asynchronously.
if (!profile->GetPrefs()->GetBoolean(kWidevineOptedIn)) {
if (!IsWidevineOptedIn()) {
DVLOG(1) << __func__ << ": recover invalid widevine prefs state";
SetWidevinePrefsAsInstalledState();
return;
}

const std::string installed_version =
profile->GetPrefs()->GetString(kWidevineInstalledVersion);
const std::string installed_version = GetWidevineInstalledVersion();

DVLOG(1) << __func__ << ": widevine prefs state looks fine";
DVLOG(1) << __func__ << ": installed widevine version: " << installed_version;
Expand Down Expand Up @@ -327,9 +306,7 @@ void BraveWidevineBundleManager::OnBackgroundUpdateFinished(
void BraveWidevineBundleManager::DoDelayedBackgroundUpdate() {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);

Profile* profile = ProfileManager::GetActiveUserProfile();
const std::string installed_version =
profile->GetPrefs()->GetString(kWidevineInstalledVersion);
const std::string installed_version = GetWidevineInstalledVersion();

DVLOG(1) << __func__ << ": update widevine"
<< " from " << installed_version
Expand Down
5 changes: 0 additions & 5 deletions browser/widevine/brave_widevine_bundle_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,9 @@ class FilePath;
class SequencedTaskRunner;
}

namespace user_prefs {
class PrefRegistrySyncable;
}

class BraveWidevineBundleManager {
public:
static char kWidevineInvalidVersion[];
static void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry);

// Called when install process is finished.
using DoneCallback = base::OnceCallback<void(const std::string& error)>;
Expand Down
Loading