From 0b748d9c128687a89f37f593c9d1bbbbd34d2313 Mon Sep 17 00:00:00 2001 From: Simon Hong Date: Mon, 11 Nov 2019 10:54:41 +0900 Subject: [PATCH 1/4] Fix widevine install prompt is shown after using new profiles Install prompt is displayed if widevine is opt opted in by user and user doesn't choose don't ask option of it. Widevine cdm lib is loaded into cdm utility process after it installed. And it is browser widely used not just for specific profile. However, Brave has prompted widevine permission prompt when user loads contents site(ex, netflix) in newly created profile. If Widevine cdm lib is already installed by any profile, Brave should not display it again because cdm lib is already used. To fix this, two kWidevineOptedIn/kWidevineInstalledVersion are migrated from profile prefs to local state. When migration is needed, firstly active profile is used to get existing pref values. There is one more Widevine related pref. It's kAskWidevineInstall. It would be good to migrate to local prefs also. but left as-is in profile prefs also seems fine. After Widevine is installed that prefs doesn't have much meaning and if not just click once to dont ask is sufficient. Also, it is convinient to implement settings option with profile prefs. --- .github/CODEOWNERS | 3 + browser/BUILD.gn | 2 +- browser/brave_drm_tab_helper.cc | 16 ++- browser/brave_drm_tab_helper.h | 2 +- browser/brave_local_state_prefs.cc | 13 +- browser/brave_profile_prefs.cc | 21 +-- browser/brave_profile_prefs_browsertest.cc | 2 - browser/widevine/BUILD.gn | 1 + .../widevine/brave_widevine_bundle_manager.cc | 39 ++--- .../widevine/brave_widevine_bundle_manager.h | 5 - .../brave_widevine_bundle_manager_unittest.cc | 60 ++++---- ...widevine_permission_request_browsertest.cc | 7 +- browser/widevine/widevine_utils.cc | 135 ++++++++++++++---- browser/widevine/widevine_utils.h | 22 ++- .../widevine_cdm_component_installer.cc | 16 +-- .../chrome/browser/prefs/browser_prefs.cc | 19 ++- 16 files changed, 225 insertions(+), 138 deletions(-) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index d620bd0c2f2e..a90de450c42c 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -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 diff --git a/browser/BUILD.gn b/browser/BUILD.gn index b4e37d68b42b..fe4b5da224b8 100644 --- a/browser/BUILD.gn +++ b/browser/BUILD.gn @@ -161,6 +161,7 @@ source_set("browser_process") { "themes", "//services/network/public/cpp", "//services/service_manager/embedder", + "//third_party/widevine/cdm:buildflags", "//ui/base", ] @@ -256,7 +257,6 @@ source_set("browser_process") { if (bundle_widevine_cdm || enable_widevine_cdm_component) { deps += [ "//brave/browser/widevine", - "//third_party/widevine/cdm:buildflags", ] } diff --git a/browser/brave_drm_tab_helper.cc b/browser/brave_drm_tab_helper.cc index ed6c727b6bb4..22e0e1d9f60a 100644 --- a/browser/brave_drm_tab_helper.cc +++ b/browser/brave_drm_tab_helper.cc @@ -5,14 +5,13 @@ #include "brave/browser/brave_drm_tab_helper.h" +#if BUILDFLAG(ENABLE_WIDEVINE_CDM_COMPONENT) || BUILDFLAG(BUNDLE_WIDEVINE_CDM) +#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) @@ -21,33 +20,36 @@ BraveDrmTabHelper::BraveDrmTabHelper(content::WebContents* contents) BraveDrmTabHelper::~BraveDrmTabHelper() {} bool BraveDrmTabHelper::ShouldShowWidevineOptIn() const { +#if BUILDFLAG(ENABLE_WIDEVINE_CDM_COMPONENT) || BUILDFLAG(BUNDLE_WIDEVINE_CDM) // If the user already opted in, don't offer it. PrefService* prefs = static_cast(web_contents()->GetBrowserContext())->GetPrefs(); - if (prefs->GetBoolean(kWidevineOptedIn) || - !prefs->GetBoolean(kAskWidevineInstall)) { + if (IsWidevineOptedIn() || !prefs->GetBoolean(kAskWidevineInstall)) { return false; } return is_widevine_requested_; +#else + return false; +#endif } void BraveDrmTabHelper::DidStartNavigation( content::NavigationHandle* navigation_handle) { +#if BUILDFLAG(ENABLE_WIDEVINE_CDM_COMPONENT) || BUILDFLAG(BUNDLE_WIDEVINE_CDM) if (!navigation_handle->IsInMainFrame() || navigation_handle->IsSameDocument()) { return; } is_widevine_requested_ = false; -#if BUILDFLAG(ENABLE_WIDEVINE_CDM_COMPONENT) || BUILDFLAG(BUNDLE_WIDEVINE_CDM) is_permission_requested_ = false; #endif } void BraveDrmTabHelper::OnWidevineKeySystemAccessRequest() { +#if BUILDFLAG(ENABLE_WIDEVINE_CDM_COMPONENT) || BUILDFLAG(BUNDLE_WIDEVINE_CDM) 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()); diff --git a/browser/brave_drm_tab_helper.h b/browser/brave_drm_tab_helper.h index c66d7a9f6e11..c77ee26d176f 100644 --- a/browser/brave_drm_tab_helper.h +++ b/browser/brave_drm_tab_helper.h @@ -40,10 +40,10 @@ class BraveDrmTabHelper final // 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; +#endif }; #endif // BRAVE_BROWSER_BRAVE_DRM_TAB_HELPER_H_ diff --git a/browser/brave_local_state_prefs.cc b/browser/brave_local_state_prefs.cc index 19bda7768ec5..44d0d2a4b04d 100644 --- a/browser/brave_local_state_prefs.cc +++ b/browser/brave_local_state_prefs.cc @@ -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" @@ -31,6 +32,10 @@ #include "brave/components/p3a/p3a_core_metrics.h" #endif // !defined(OS_ANDROID) +#if BUILDFLAG(ENABLE_WIDEVINE_CDM_COMPONENT) || BUILDFLAG(BUNDLE_WIDEVINE_CDM) +#include "brave/browser/widevine/widevine_utils.h" +#endif + namespace brave { void RegisterLocalStatePrefs(PrefRegistrySimple* registry) { @@ -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, @@ -60,6 +62,11 @@ void RegisterLocalStatePrefs(PrefRegistrySimple* registry) { brave_shields::RegisterShieldsP3APrefs(registry); #if !defined(OS_ANDROID) + RegisterPrefsForMuonMigration(registry); +#if BUILDFLAG(ENABLE_WIDEVINE_CDM_COMPONENT) || BUILDFLAG(BUNDLE_WIDEVINE_CDM) + RegisterWidevineLocalstatePrefs(registry); +#endif + BraveWindowTracker::RegisterPrefs(registry); BraveUptimeTracker::RegisterPrefs(registry); #endif diff --git a/browser/brave_profile_prefs.cc b/browser/brave_profile_prefs.cc index 7e3fa0da6262..92c999f9c499 100644 --- a/browser/brave_profile_prefs.cc +++ b/browser/brave_profile_prefs.cc @@ -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 @@ -38,10 +34,21 @@ #include "components/gcm_driver/gcm_channel_status_syncer.h" #endif +#if BUILDFLAG(ENABLE_WIDEVINE_CDM_COMPONENT) || BUILDFLAG(BUNDLE_WIDEVINE_CDM) +#include "brave/browser/widevine/widevine_utils.h" +#endif + using extensions::FeatureSwitch; namespace brave { +void RegisterProfilePrefsForMigration( + user_prefs::PrefRegistrySyncable* registry) { +#if BUILDFLAG(ENABLE_WIDEVINE_CDM_COMPONENT) || BUILDFLAG(BUNDLE_WIDEVINE_CDM) + RegisterWidevineProfilePrefsForMigration(registry); +#endif +} + void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry) { brave_shields::BraveShieldsWebContentsObserver::RegisterProfilePrefs( registry); @@ -52,11 +59,7 @@ void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry) { brave_sync::prefs::Prefs::RegisterProfilePrefs(registry); - registry->RegisterBooleanPref(kWidevineOptedIn, false); registry->RegisterBooleanPref(kAskWidevineInstall, true); -#if BUILDFLAG(BUNDLE_WIDEVINE_CDM) - BraveWidevineBundleManager::RegisterProfilePrefs(registry); -#endif // Default Brave shields registry->RegisterBooleanPref(kHTTPSEVerywhereControlType, true); @@ -169,6 +172,8 @@ void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry) { registry->RegisterStringPref(kBraveWalletAES256GCMSivNonce, ""); registry->RegisterStringPref(kBraveWalletEncryptedSeed, ""); registry->RegisterBooleanPref(kBraveWalletEnabled, true); + + RegisterProfilePrefsForMigration(registry); } } // namespace brave diff --git a/browser/brave_profile_prefs_browsertest.cc b/browser/brave_profile_prefs_browsertest.cc index b5bc12c1cecf..32bb2efa05bb 100644 --- a/browser/brave_profile_prefs_browsertest.cc +++ b/browser/brave_profile_prefs_browsertest.cc @@ -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( diff --git a/browser/widevine/BUILD.gn b/browser/widevine/BUILD.gn index 17d780d7d492..9f0864e284d4 100644 --- a/browser/widevine/BUILD.gn +++ b/browser/widevine/BUILD.gn @@ -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", diff --git a/browser/widevine/brave_widevine_bundle_manager.cc b/browser/widevine/brave_widevine_bundle_manager.cc index dbd378f3616f..a3df0e0fc8ab 100644 --- a/browser/widevine/brave_widevine_bundle_manager.cc +++ b/browser/widevine/brave_widevine_bundle_manager.cc @@ -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" @@ -61,24 +59,16 @@ base::Optional 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 @@ -86,13 +76,6 @@ void SetWidevinePrefsAsInstalledState() { // static char BraveWidevineBundleManager::kWidevineInvalidVersion[] = ""; -// static -void BraveWidevineBundleManager::RegisterProfilePrefs( - user_prefs::PrefRegistrySyncable* registry) { - registry->RegisterStringPref(kWidevineInstalledVersion, - kWidevineInvalidVersion); -} - BraveWidevineBundleManager::BraveWidevineBundleManager() : weak_factory_(this) { } @@ -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; @@ -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 diff --git a/browser/widevine/brave_widevine_bundle_manager.h b/browser/widevine/brave_widevine_bundle_manager.h index de216af73d2e..2eb1bf349e6e 100644 --- a/browser/widevine/brave_widevine_bundle_manager.h +++ b/browser/widevine/brave_widevine_bundle_manager.h @@ -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; diff --git a/browser/widevine/brave_widevine_bundle_manager_unittest.cc b/browser/widevine/brave_widevine_bundle_manager_unittest.cc index 4d3512fb7032..5c8e367c188f 100644 --- a/browser/widevine/brave_widevine_bundle_manager_unittest.cc +++ b/browser/widevine/brave_widevine_bundle_manager_unittest.cc @@ -7,14 +7,13 @@ #include -#include "base/files/scoped_temp_dir.h" -#include "brave/common/pref_names.h" +#include "brave/browser/brave_local_state_prefs.h" +#include "brave/browser/widevine/widevine_utils.h" #include "brave/grit/brave_generated_resources.h" +#include "chrome/browser/prefs/browser_prefs.h" #include "chrome/browser/profiles/profile_manager.h" #include "chrome/test/base/testing_browser_process.h" -#include "chrome/test/base/testing_profile.h" -#include "chrome/test/base/testing_profile_manager.h" -#include "components/user_prefs/user_prefs.h" +#include "components/prefs/testing_pref_service.h" #include "content/public/browser/cdm_registry.h" #include "content/public/common/cdm_info.h" #include "content/public/test/browser_task_environment.h" @@ -54,16 +53,18 @@ class TestClient : public content::TestContentClient { class BraveWidevineBundleManagerTest : public testing::Test { public: - BraveWidevineBundleManagerTest() - : testing_profile_manager_(TestingBrowserProcess::GetGlobal()) { - } + BraveWidevineBundleManagerTest() {} ~BraveWidevineBundleManagerTest() override {} protected: void SetUp() override { - ASSERT_TRUE(temp_dir_.CreateUniqueTempDir()); - ASSERT_TRUE(testing_profile_manager_.SetUp(temp_dir_.GetPath())); manager_.is_test_ = true; + RegisterLocalState(local_state_.registry()); + TestingBrowserProcess::GetGlobal()->SetLocalState(&local_state_); + } + + void TearDown() override { + TestingBrowserProcess::GetGlobal()->SetLocalState(nullptr); } void PrepareCdmRegistry(bool empty_cdms) { @@ -76,31 +77,23 @@ class BraveWidevineBundleManagerTest : public testing::Test { PrepareCdmRegistry(empty_cdms); } - PrefService* pref_service() { - return ProfileManager::GetActiveUserProfile()->GetPrefs(); - } - void CheckPrefsStatesAreInitialState() { - EXPECT_EQ(initial_opted_in_value_, - pref_service()->GetBoolean(kWidevineOptedIn)); - EXPECT_EQ(initial_version_string_, - pref_service()->GetString(kWidevineInstalledVersion)); + EXPECT_EQ(initial_opted_in_value_, IsWidevineOptedIn()); + EXPECT_EQ(initial_version_string_, GetWidevineInstalledVersion()); } void CheckPrefsStatesAreInstalledState() { - EXPECT_EQ(true, pref_service()->GetBoolean(kWidevineOptedIn)); - EXPECT_EQ(WIDEVINE_CDM_VERSION_STRING, - pref_service()->GetString(kWidevineInstalledVersion)); + EXPECT_EQ(true, IsWidevineOptedIn()); + EXPECT_EQ(WIDEVINE_CDM_VERSION_STRING, GetWidevineInstalledVersion()); } content::TestBrowserThreadBundle threads_; BraveWidevineBundleManager manager_; - TestingProfileManager testing_profile_manager_; - base::ScopedTempDir temp_dir_; TestClient client_; bool initial_opted_in_value_ = false; std::string initial_version_string_ = BraveWidevineBundleManager::kWidevineInvalidVersion; + TestingPrefServiceSimple local_state_; }; TEST_F(BraveWidevineBundleManagerTest, InitialPrefsTest) { @@ -123,9 +116,8 @@ TEST_F(BraveWidevineBundleManagerTest, PrefsResetTestWithEmptyCdmRegistry) { PrepareTest(true); // When only prefs are set w/o cdm library, reset prefs to initial state. - pref_service()->SetBoolean(kWidevineOptedIn, true); - pref_service()->SetString(kWidevineInstalledVersion, - WIDEVINE_CDM_VERSION_STRING); + SetWidevineOptedIn(true); + SetWidevineInstalledVersion(WIDEVINE_CDM_VERSION_STRING); manager_.StartupCheck(); CheckPrefsStatesAreInitialState(); @@ -211,8 +203,8 @@ TEST_F(BraveWidevineBundleManagerTest, UpdateTriggerTest) { PrepareTest(false); // Set installed state with different version to trigger update. - pref_service()->SetBoolean(kWidevineOptedIn, true); - pref_service()->SetString(kWidevineInstalledVersion, "1.0.0.0"); + SetWidevineOptedIn(true); + SetWidevineInstalledVersion("1.0.0.0"); EXPECT_FALSE(manager_.update_requested_); @@ -233,8 +225,8 @@ TEST_F(BraveWidevineBundleManagerTest, UpdateFailTest) { initial_version_string_ = "1.0.0.0"; // Set installed state with different version to trigger update. - pref_service()->SetBoolean(kWidevineOptedIn, initial_opted_in_value_); - pref_service()->SetString(kWidevineInstalledVersion, initial_version_string_); + SetWidevineOptedIn(initial_opted_in_value_); + SetWidevineInstalledVersion(initial_version_string_); manager_.StartupCheck(); manager_.DoDelayedBackgroundUpdate(); @@ -251,8 +243,8 @@ TEST_F(BraveWidevineBundleManagerTest, UpdateRetryAndFinallyFailedTest) { initial_version_string_ = "1.0.0.0"; // Set installed state with different version to trigger update. - pref_service()->SetBoolean(kWidevineOptedIn, initial_opted_in_value_); - pref_service()->SetString(kWidevineInstalledVersion, initial_version_string_); + SetWidevineOptedIn(initial_opted_in_value_); + SetWidevineInstalledVersion(initial_version_string_); manager_.StartupCheck(); @@ -291,8 +283,8 @@ TEST_F(BraveWidevineBundleManagerTest, UpdateRetryAndFinallySuccessTest) { initial_version_string_ = "1.0.0.0"; // Set installed state with different version to trigger update. - pref_service()->SetBoolean(kWidevineOptedIn, initial_opted_in_value_); - pref_service()->SetString(kWidevineInstalledVersion, initial_version_string_); + SetWidevineOptedIn(initial_opted_in_value_); + SetWidevineInstalledVersion(initial_version_string_); manager_.StartupCheck(); diff --git a/browser/widevine/widevine_permission_request_browsertest.cc b/browser/widevine/widevine_permission_request_browsertest.cc index e9f96f6bfffb..5e51e94e5ce5 100644 --- a/browser/widevine/widevine_permission_request_browsertest.cc +++ b/browser/widevine/widevine_permission_request_browsertest.cc @@ -9,8 +9,6 @@ #include "brave/common/brave_paths.h" #include "brave/common/pref_names.h" #include "chrome/browser/permissions/permission_request_manager.h" -#include "chrome/browser/profiles/profile.h" -#include "chrome/browser/profiles/profile_manager.h" #include "chrome/browser/ssl/cert_verifier_browser_test.h" #include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/browser_commands.h" @@ -150,9 +148,8 @@ IN_PROC_BROWSER_TEST_F(WidevinePermissionRequestBrowserTest, BubbleTest) { #if BUILDFLAG(ENABLE_WIDEVINE_CDM_COMPONENT) IN_PROC_BROWSER_TEST_F(WidevinePermissionRequestBrowserTest, CheckOptedInPrefStateForComponent) { - PrefService* prefs = ProfileManager::GetActiveUserProfile()->GetPrefs(); // Before we allow, opted in should be false - EXPECT_FALSE(prefs->GetBoolean(kWidevineOptedIn)); + EXPECT_FALSE(IsWidevineOptedIn()); GetPermissionRequestManager()->set_auto_response_for_test( PermissionRequestManager::ACCEPT_ALL); @@ -161,7 +158,7 @@ IN_PROC_BROWSER_TEST_F(WidevinePermissionRequestBrowserTest, content::RunAllTasksUntilIdle(); // After we allow, opted in pref should be true - EXPECT_TRUE(prefs->GetBoolean(kWidevineOptedIn)); + EXPECT_TRUE(IsWidevineOptedIn()); EXPECT_TRUE(observer.bubble_added_); // Reset observer and check permission bubble isn't created again. diff --git a/browser/widevine/widevine_utils.cc b/browser/widevine/widevine_utils.cc index 4683d176357e..35b15108af2e 100644 --- a/browser/widevine/widevine_utils.cc +++ b/browser/widevine/widevine_utils.cc @@ -12,7 +12,10 @@ #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_registry_simple.h" #include "components/prefs/pref_service.h" +#include "components/pref_registry/pref_registry_syncable.h" +#include "content/public/browser/browser_thread.h" #if BUILDFLAG(BUNDLE_WIDEVINE_CDM) #include @@ -22,17 +25,34 @@ #include "brave/browser/widevine/brave_widevine_bundle_manager.h" #include "chrome/browser/lifetime/application_lifetime.h" #include "chrome/browser/ui/browser_finder.h" -#include "chrome/browser/ui/browser_window.h" #include "chrome/browser/ui/tabs/tab_strip_model.h" #endif #if BUILDFLAG(ENABLE_WIDEVINE_CDM_COMPONENT) #include "chrome/browser/component_updater/widevine_cdm_component_installer.h" -#include "chrome/browser/profiles/profile_manager.h" #endif -#if BUILDFLAG(BUNDLE_WIDEVINE_CDM) +using content::BrowserThread; + namespace { + +#if BUILDFLAG(ENABLE_WIDEVINE_CDM_COMPONENT) || BUILDFLAG(BUNDLE_WIDEVINE_CDM) +PrefService* GetLocalState() { + DCHECK(g_browser_process); + return g_browser_process->local_state(); +} + +void ClearWidevinePrefs(Profile* profile) { + PrefService* prefs = profile->GetPrefs(); + prefs->ClearPref(kWidevineOptedIn); + +#if BUILDFLAG(BUNDLE_WIDEVINE_CDM) + prefs->ClearPref(kWidevineInstalledVersion); +#endif +} +#endif + +#if BUILDFLAG(BUNDLE_WIDEVINE_CDM) content::WebContents* GetActiveWebContents() { if (Browser* browser = chrome::FindLastActive()) return browser->tab_strip_model()->GetActiveWebContents(); @@ -61,40 +81,18 @@ void OnWidevineInstallDone(const std::string& error) { if (IsActiveTabRequestedWidevine()) RequestWidevinePermission(GetActiveWebContents()); } -} // namespace -#endif - -int GetWidevinePermissionRequestTextFrangmentResourceId() { - int message_id = -1; - -#if BUILDFLAG(ENABLE_WIDEVINE_CDM_COMPONENT) - message_id = IDS_WIDEVINE_PERMISSION_REQUEST_TEXT_FRAGMENT; #endif -#if BUILDFLAG(BUNDLE_WIDEVINE_CDM) - auto* manager = g_brave_browser_process->brave_widevine_bundle_manager(); - message_id = manager->GetWidevinePermissionRequestTextFragment(); -#endif - - DCHECK_NE(message_id, -1); - return message_id; -} - -void RequestWidevinePermission(content::WebContents* web_contents) { - PermissionRequestManager::FromWebContents(web_contents)->AddRequest( - new WidevinePermissionRequest(web_contents)); -} +} // namespace #if BUILDFLAG(ENABLE_WIDEVINE_CDM_COMPONENT) void EnableWidevineCdmComponent(content::WebContents* web_contents) { DCHECK(web_contents); - PrefService* prefs = - static_cast(web_contents->GetBrowserContext())->GetPrefs(); - if (prefs->GetBoolean(kWidevineOptedIn)) + if (IsWidevineOptedIn()) return; - prefs->SetBoolean(kWidevineOptedIn, true); + SetWidevineOptedIn(true); RegisterWidevineCdmComponent(g_brave_browser_process->component_updater()); ChromeSubresourceFilterClient::FromWebContents(web_contents) ->OnReloadRequested(); @@ -121,9 +119,90 @@ void InstallBundleOrRestartBrowser() { true); } } + +void SetWidevineInstalledVersion(const std::string& version) { + GetLocalState()->SetString(kWidevineInstalledVersion, version); +} + +std::string GetWidevineInstalledVersion() { + return GetLocalState()->GetString(kWidevineInstalledVersion); +} +#endif + +#if BUILDFLAG(ENABLE_WIDEVINE_CDM_COMPONENT) || BUILDFLAG(BUNDLE_WIDEVINE_CDM) +int GetWidevinePermissionRequestTextFrangmentResourceId() { + int message_id = -1; + +#if BUILDFLAG(ENABLE_WIDEVINE_CDM_COMPONENT) + message_id = IDS_WIDEVINE_PERMISSION_REQUEST_TEXT_FRAGMENT; #endif +#if BUILDFLAG(BUNDLE_WIDEVINE_CDM) + auto* manager = g_brave_browser_process->brave_widevine_bundle_manager(); + message_id = manager->GetWidevinePermissionRequestTextFragment(); +#endif + + DCHECK_NE(message_id, -1); + return message_id; +} + +void RequestWidevinePermission(content::WebContents* web_contents) { + PermissionRequestManager::FromWebContents(web_contents)->AddRequest( + new WidevinePermissionRequest(web_contents)); +} void DontAskWidevineInstall(content::WebContents* web_contents, bool dont_ask) { Profile* profile = static_cast(web_contents->GetBrowserContext()); profile->GetPrefs()->SetBoolean(kAskWidevineInstall, !dont_ask); } + +void RegisterWidevineProfilePrefsForMigration( + user_prefs::PrefRegistrySyncable* registry) { + registry->RegisterBooleanPref(kWidevineOptedIn, false); + +#if BUILDFLAG(BUNDLE_WIDEVINE_CDM) + registry->RegisterStringPref( + kWidevineInstalledVersion, + BraveWidevineBundleManager::kWidevineInvalidVersion); +#endif +} + +void RegisterWidevineLocalstatePrefs(PrefRegistrySimple* registry) { + registry->RegisterBooleanPref(kWidevineOptedIn, false); + +#if BUILDFLAG(BUNDLE_WIDEVINE_CDM) + registry->RegisterStringPref( + kWidevineInstalledVersion, + BraveWidevineBundleManager::kWidevineInvalidVersion); +#endif +} + +bool IsWidevineOptedIn() { + return GetLocalState()->GetBoolean(kWidevineOptedIn); +} + +void SetWidevineOptedIn(bool opted_in) { + GetLocalState()->SetBoolean(kWidevineOptedIn, opted_in); +} + +void MigrateWidevinePrefs(Profile* profile) { + DCHECK_CURRENTLY_ON(BrowserThread::UI); + + // If migration is done, local state doesn't have default value because + // they were explicitly set by primary prefs' value. After that, we don't + // need to try migration again and prefs from profiles are already cleared. + if (GetLocalState()->FindPreference(kWidevineOptedIn)->IsDefaultValue()) { + PrefService* prefs = profile->GetPrefs(); + GetLocalState()->SetBoolean(kWidevineOptedIn, + prefs->GetBoolean(kWidevineOptedIn)); + +#if BUILDFLAG(BUNDLE_WIDEVINE_CDM) + GetLocalState()->SetString(kWidevineInstalledVersion, + prefs->GetString(kWidevineInstalledVersion)); +#endif + } + + // Clear deprecated prefs. + ClearWidevinePrefs(profile); +} + +#endif diff --git a/browser/widevine/widevine_utils.h b/browser/widevine/widevine_utils.h index dfef84e25eb7..8d60e30e8bd2 100644 --- a/browser/widevine/widevine_utils.h +++ b/browser/widevine/widevine_utils.h @@ -6,23 +6,41 @@ #ifndef BRAVE_BROWSER_WIDEVINE_WIDEVINE_UTILS_H_ #define BRAVE_BROWSER_WIDEVINE_WIDEVINE_UTILS_H_ +#include + #include "third_party/widevine/cdm/buildflags.h" namespace content { class WebContents; +} // namespace content + +namespace user_prefs { +class PrefRegistrySyncable; } -int GetWidevinePermissionRequestTextFrangmentResourceId(); -void RequestWidevinePermission(content::WebContents* web_contents); +class PrefRegistrySimple; +class Profile; #if BUILDFLAG(BUNDLE_WIDEVINE_CDM) void InstallBundleOrRestartBrowser(); +void SetWidevineInstalledVersion(const std::string& version); +std::string GetWidevineInstalledVersion(); #endif #if BUILDFLAG(ENABLE_WIDEVINE_CDM_COMPONENT) void EnableWidevineCdmComponent(content::WebContents* web_contents); #endif +#if BUILDFLAG(ENABLE_WIDEVINE_CDM_COMPONENT) || BUILDFLAG(BUNDLE_WIDEVINE_CDM) +void RegisterWidevineProfilePrefsForMigration( + user_prefs::PrefRegistrySyncable* registry); +int GetWidevinePermissionRequestTextFrangmentResourceId(); +void RequestWidevinePermission(content::WebContents* web_contents); +void RegisterWidevineLocalstatePrefs(PrefRegistrySimple* registry); void DontAskWidevineInstall(content::WebContents* web_contents, bool dont_ask); +bool IsWidevineOptedIn(); +void SetWidevineOptedIn(bool opted_in); +void MigrateWidevinePrefs(Profile* profile); +#endif #endif // BRAVE_BROWSER_WIDEVINE_WIDEVINE_UTILS_H_ diff --git a/chromium_src/chrome/browser/component_updater/widevine_cdm_component_installer.cc b/chromium_src/chrome/browser/component_updater/widevine_cdm_component_installer.cc index aa66fd39d5b9..46d25604d9f3 100644 --- a/chromium_src/chrome/browser/component_updater/widevine_cdm_component_installer.cc +++ b/chromium_src/chrome/browser/component_updater/widevine_cdm_component_installer.cc @@ -8,19 +8,19 @@ #undef RegisterWidevineCdmComponent #include "brave/browser/brave_browser_process_impl.h" -#include "brave/common/pref_names.h" -#include "chrome/browser/profiles/profile_manager.h" -#include "chrome/browser/ui/webui/components_ui.h" #include "brave/common/extensions/extension_constants.h" +#include "chrome/browser/ui/webui/components_ui.h" #include "components/component_updater/component_updater_service.h" -#include "components/prefs/pref_service.h" #include "third_party/widevine/cdm/buildflags.h" +#if BUILDFLAG(ENABLE_WIDEVINE_CDM_COMPONENT) +#include "brave/browser/widevine/widevine_utils.h" +#endif + namespace component_updater { #if BUILDFLAG(ENABLE_WIDEVINE_CDM_COMPONENT) - void OnWidevineRegistered() { ComponentsUI::OnDemandUpdate(widevine_extension_id); } @@ -41,12 +41,8 @@ void RegisterAndInstallWidevine() { void RegisterWidevineCdmComponent(ComponentUpdateService* cus) { #if BUILDFLAG(ENABLE_WIDEVINE_CDM_COMPONENT) DCHECK_CURRENTLY_ON(BrowserThread::UI); - PrefService* prefs = ProfileManager::GetActiveUserProfile()->GetPrefs(); - bool widevine_opted_in = - prefs->GetBoolean(kWidevineOptedIn); - if (widevine_opted_in) { + if (IsWidevineOptedIn()) RegisterAndInstallWidevine(); - } #endif // defined(ENABLE_WIDEVINE_CDM_COMPONENT) } diff --git a/chromium_src/chrome/browser/prefs/browser_prefs.cc b/chromium_src/chrome/browser/prefs/browser_prefs.cc index d8b472bcc2e9..4ce9096ed915 100644 --- a/chromium_src/chrome/browser/prefs/browser_prefs.cc +++ b/chromium_src/chrome/browser/prefs/browser_prefs.cc @@ -5,4 +5,21 @@ #include "brave/browser/brave_profile_prefs.h" #include "brave/browser/brave_local_state_prefs.h" -#include "../../../../chrome/browser/prefs/browser_prefs.cc" +#include "third_party/widevine/cdm/buildflags.h" + +#if BUILDFLAG(ENABLE_WIDEVINE_CDM_COMPONENT) || BUILDFLAG(BUNDLE_WIDEVINE_CDM) +#include "brave/browser/widevine/widevine_utils.h" +#endif + +#define MigrateObsoleteProfilePrefs MigrateObsoleteProfilePrefs_ChromiumImpl +#include "../../../../chrome/browser/prefs/browser_prefs.cc" // NOLINT +#undef MigrateObsoleteProfilePrefs + +void MigrateObsoleteProfilePrefs(Profile* profile) { + MigrateObsoleteProfilePrefs_ChromiumImpl(profile); + +#if BUILDFLAG(ENABLE_WIDEVINE_CDM_COMPONENT) || BUILDFLAG(BUNDLE_WIDEVINE_CDM) + MigrateWidevinePrefs(profile); +#endif +} + From dec8edf543f838b01f959dba99c5bb69d7b02c7c Mon Sep 17 00:00:00 2001 From: Simon Hong Date: Mon, 18 Nov 2019 09:27:06 +0900 Subject: [PATCH 2/4] Cleanup Widevine build flag usages and add some migration tests Add WidevinePrefsMigrationTest.PrefMigrationTest for prefs migration test. --- browser/BUILD.gn | 10 +++---- browser/brave_drm_tab_helper.cc | 10 ------- browser/brave_drm_tab_helper.h | 3 -- browser/brave_local_state_prefs.cc | 9 +++--- browser/brave_profile_prefs.cc | 7 +++-- browser/brave_tab_helpers.cc | 11 +++++-- .../widevine/widevine_permission_request.cc | 1 + .../widevine_prefs_migration_browsertest.cc | 23 ++++++++++++++ browser/widevine/widevine_utils.cc | 30 +++++++------------ browser/widevine/widevine_utils.h | 2 -- .../widevine_cdm_component_installer.cc | 12 +------- .../chrome/browser/prefs/browser_prefs.cc | 6 ++-- .../permission_prompt_impl.cc | 16 ++++++++-- .../renderer/media/chrome_key_systems.cc | 25 ++++++++++++++++ test/BUILD.gn | 24 +++++++-------- 15 files changed, 114 insertions(+), 75 deletions(-) create mode 100644 browser/widevine/widevine_prefs_migration_browsertest.cc create mode 100644 chromium_src/chrome/renderer/media/chrome_key_systems.cc diff --git a/browser/BUILD.gn b/browser/BUILD.gn index fe4b5da224b8..b14758e60644 100644 --- a/browser/BUILD.gn +++ b/browser/BUILD.gn @@ -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", @@ -254,10 +252,12 @@ source_set("browser_process") { ] } - if (bundle_widevine_cdm || enable_widevine_cdm_component) { - deps += [ - "//brave/browser/widevine", + if (enable_widevine) { + sources += [ + "brave_drm_tab_helper.cc", + "brave_drm_tab_helper.h", ] + deps += [ "//brave/browser/widevine" ] } if (is_win && is_official_build) { diff --git a/browser/brave_drm_tab_helper.cc b/browser/brave_drm_tab_helper.cc index 22e0e1d9f60a..8bdd2acf2836 100644 --- a/browser/brave_drm_tab_helper.cc +++ b/browser/brave_drm_tab_helper.cc @@ -5,14 +5,12 @@ #include "brave/browser/brave_drm_tab_helper.h" -#if BUILDFLAG(ENABLE_WIDEVINE_CDM_COMPONENT) || BUILDFLAG(BUNDLE_WIDEVINE_CDM) #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" -#endif BraveDrmTabHelper::BraveDrmTabHelper(content::WebContents* contents) : WebContentsObserver(contents), bindings_(contents, this) {} @@ -20,7 +18,6 @@ BraveDrmTabHelper::BraveDrmTabHelper(content::WebContents* contents) BraveDrmTabHelper::~BraveDrmTabHelper() {} bool BraveDrmTabHelper::ShouldShowWidevineOptIn() const { -#if BUILDFLAG(ENABLE_WIDEVINE_CDM_COMPONENT) || BUILDFLAG(BUNDLE_WIDEVINE_CDM) // If the user already opted in, don't offer it. PrefService* prefs = static_cast(web_contents()->GetBrowserContext())->GetPrefs(); @@ -29,32 +26,25 @@ bool BraveDrmTabHelper::ShouldShowWidevineOptIn() const { } return is_widevine_requested_; -#else - return false; -#endif } void BraveDrmTabHelper::DidStartNavigation( content::NavigationHandle* navigation_handle) { -#if BUILDFLAG(ENABLE_WIDEVINE_CDM_COMPONENT) || BUILDFLAG(BUNDLE_WIDEVINE_CDM) if (!navigation_handle->IsInMainFrame() || navigation_handle->IsSameDocument()) { return; } is_widevine_requested_ = false; is_permission_requested_ = false; -#endif } void BraveDrmTabHelper::OnWidevineKeySystemAccessRequest() { -#if BUILDFLAG(ENABLE_WIDEVINE_CDM_COMPONENT) || BUILDFLAG(BUNDLE_WIDEVINE_CDM) is_widevine_requested_ = true; if (ShouldShowWidevineOptIn() && !is_permission_requested_) { is_permission_requested_ = true; RequestWidevinePermission(web_contents()); } -#endif } WEB_CONTENTS_USER_DATA_KEY_IMPL(BraveDrmTabHelper) diff --git a/browser/brave_drm_tab_helper.h b/browser/brave_drm_tab_helper.h index c77ee26d176f..21cf47ea6b3c 100644 --- a/browser/brave_drm_tab_helper.h +++ b/browser/brave_drm_tab_helper.h @@ -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 @@ -35,7 +34,6 @@ class BraveDrmTabHelper final private: content::WebContentsFrameBindingSet 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. @@ -43,7 +41,6 @@ class BraveDrmTabHelper final // True if we are notified that a page requested widevine availability. bool is_widevine_requested_ = false; -#endif }; #endif // BRAVE_BROWSER_BRAVE_DRM_TAB_HELPER_H_ diff --git a/browser/brave_local_state_prefs.cc b/browser/brave_local_state_prefs.cc index 44d0d2a4b04d..440adb6b79dd 100644 --- a/browser/brave_local_state_prefs.cc +++ b/browser/brave_local_state_prefs.cc @@ -32,7 +32,7 @@ #include "brave/components/p3a/p3a_core_metrics.h" #endif // !defined(OS_ANDROID) -#if BUILDFLAG(ENABLE_WIDEVINE_CDM_COMPONENT) || BUILDFLAG(BUNDLE_WIDEVINE_CDM) +#if BUILDFLAG(ENABLE_WIDEVINE) #include "brave/browser/widevine/widevine_utils.h" #endif @@ -63,13 +63,14 @@ void RegisterLocalStatePrefs(PrefRegistrySimple* registry) { brave_shields::RegisterShieldsP3APrefs(registry); #if !defined(OS_ANDROID) RegisterPrefsForMuonMigration(registry); -#if BUILDFLAG(ENABLE_WIDEVINE_CDM_COMPONENT) || BUILDFLAG(BUNDLE_WIDEVINE_CDM) - RegisterWidevineLocalstatePrefs(registry); -#endif BraveWindowTracker::RegisterPrefs(registry); BraveUptimeTracker::RegisterPrefs(registry); #endif + +#if BUILDFLAG(ENABLE_WIDEVINE) + RegisterWidevineLocalstatePrefs(registry); +#endif } } // namespace brave diff --git a/browser/brave_profile_prefs.cc b/browser/brave_profile_prefs.cc index 92c999f9c499..f5d57246b33a 100644 --- a/browser/brave_profile_prefs.cc +++ b/browser/brave_profile_prefs.cc @@ -34,7 +34,7 @@ #include "components/gcm_driver/gcm_channel_status_syncer.h" #endif -#if BUILDFLAG(ENABLE_WIDEVINE_CDM_COMPONENT) || BUILDFLAG(BUNDLE_WIDEVINE_CDM) +#if BUILDFLAG(ENABLE_WIDEVINE) #include "brave/browser/widevine/widevine_utils.h" #endif @@ -44,7 +44,7 @@ namespace brave { void RegisterProfilePrefsForMigration( user_prefs::PrefRegistrySyncable* registry) { -#if BUILDFLAG(ENABLE_WIDEVINE_CDM_COMPONENT) || BUILDFLAG(BUNDLE_WIDEVINE_CDM) +#if BUILDFLAG(ENABLE_WIDEVINE) RegisterWidevineProfilePrefsForMigration(registry); #endif } @@ -59,6 +59,9 @@ void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry) { brave_sync::prefs::Prefs::RegisterProfilePrefs(registry); + // TODO(shong): Move this pref to ENABLE_WIDEVINE. + // We don't need to display "don't ask widevine prompt option" in settings + // if widevine is disabled. registry->RegisterBooleanPref(kAskWidevineInstall, true); // Default Brave shields diff --git a/browser/brave_tab_helpers.cc b/browser/brave_tab_helpers.cc index 16ae25350d16..cd7b808e16ad 100644 --- a/browser/brave_tab_helpers.cc +++ b/browser/brave_tab_helpers.cc @@ -5,7 +5,6 @@ #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" @@ -13,6 +12,7 @@ #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" @@ -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) { @@ -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 @@ -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); } diff --git a/browser/widevine/widevine_permission_request.cc b/browser/widevine/widevine_permission_request.cc index cf081ad1849f..f5b0a6da6542 100644 --- a/browser/widevine/widevine_permission_request.cc +++ b/browser/widevine/widevine_permission_request.cc @@ -10,6 +10,7 @@ #include "chrome/app/vector_icons/vector_icons.h" #include "content/public/browser/web_contents.h" #include "ui/base/l10n/l10n_util.h" +#include "third_party/widevine/cdm/buildflags.h" WidevinePermissionRequest::WidevinePermissionRequest( content::WebContents* web_contents) diff --git a/browser/widevine/widevine_prefs_migration_browsertest.cc b/browser/widevine/widevine_prefs_migration_browsertest.cc new file mode 100644 index 000000000000..ad137eb39627 --- /dev/null +++ b/browser/widevine/widevine_prefs_migration_browsertest.cc @@ -0,0 +1,23 @@ +/* Copyright (c) 2019 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 "brave/common/pref_names.h" +#include "chrome/browser/browser_process.h" +#include "chrome/test/base/in_process_browser_test.h" +#include "components/prefs/pref_service.h" +#include "third_party/widevine/cdm/buildflags.h" + +using WidevinePrefsMigrationTest = InProcessBrowserTest; + +IN_PROC_BROWSER_TEST_F(WidevinePrefsMigrationTest, PrefMigrationTest) { + // After migration, local state doesn't have default value. + EXPECT_FALSE(g_browser_process->local_state()-> + FindPreference(kWidevineOptedIn)->IsDefaultValue()); + +#if BUILDFLAG(BUNDLE_WIDEVINE_CDM) + EXPECT_FALSE(g_browser_process->local_state()-> + FindPreference(kWidevineInstalledVersion)->IsDefaultValue()); +#endif +} diff --git a/browser/widevine/widevine_utils.cc b/browser/widevine/widevine_utils.cc index 35b15108af2e..d115749d13e5 100644 --- a/browser/widevine/widevine_utils.cc +++ b/browser/widevine/widevine_utils.cc @@ -36,12 +36,6 @@ using content::BrowserThread; namespace { -#if BUILDFLAG(ENABLE_WIDEVINE_CDM_COMPONENT) || BUILDFLAG(BUNDLE_WIDEVINE_CDM) -PrefService* GetLocalState() { - DCHECK(g_browser_process); - return g_browser_process->local_state(); -} - void ClearWidevinePrefs(Profile* profile) { PrefService* prefs = profile->GetPrefs(); prefs->ClearPref(kWidevineOptedIn); @@ -50,7 +44,6 @@ void ClearWidevinePrefs(Profile* profile) { prefs->ClearPref(kWidevineInstalledVersion); #endif } -#endif #if BUILDFLAG(BUNDLE_WIDEVINE_CDM) content::WebContents* GetActiveWebContents() { @@ -121,15 +114,15 @@ void InstallBundleOrRestartBrowser() { } void SetWidevineInstalledVersion(const std::string& version) { - GetLocalState()->SetString(kWidevineInstalledVersion, version); + g_browser_process->local_state()->SetString(kWidevineInstalledVersion, + version); } std::string GetWidevineInstalledVersion() { - return GetLocalState()->GetString(kWidevineInstalledVersion); + return g_browser_process->local_state()->GetString(kWidevineInstalledVersion); } #endif -#if BUILDFLAG(ENABLE_WIDEVINE_CDM_COMPONENT) || BUILDFLAG(BUNDLE_WIDEVINE_CDM) int GetWidevinePermissionRequestTextFrangmentResourceId() { int message_id = -1; @@ -177,32 +170,31 @@ void RegisterWidevineLocalstatePrefs(PrefRegistrySimple* registry) { } bool IsWidevineOptedIn() { - return GetLocalState()->GetBoolean(kWidevineOptedIn); + return g_browser_process->local_state()->GetBoolean(kWidevineOptedIn); } void SetWidevineOptedIn(bool opted_in) { - GetLocalState()->SetBoolean(kWidevineOptedIn, opted_in); + g_browser_process->local_state()->SetBoolean(kWidevineOptedIn, opted_in); } void MigrateWidevinePrefs(Profile* profile) { DCHECK_CURRENTLY_ON(BrowserThread::UI); + auto* local_state = g_browser_process->local_state(); // If migration is done, local state doesn't have default value because // they were explicitly set by primary prefs' value. After that, we don't // need to try migration again and prefs from profiles are already cleared. - if (GetLocalState()->FindPreference(kWidevineOptedIn)->IsDefaultValue()) { + if (local_state->FindPreference(kWidevineOptedIn)->IsDefaultValue()) { PrefService* prefs = profile->GetPrefs(); - GetLocalState()->SetBoolean(kWidevineOptedIn, - prefs->GetBoolean(kWidevineOptedIn)); + local_state->SetBoolean(kWidevineOptedIn, + prefs->GetBoolean(kWidevineOptedIn)); #if BUILDFLAG(BUNDLE_WIDEVINE_CDM) - GetLocalState()->SetString(kWidevineInstalledVersion, - prefs->GetString(kWidevineInstalledVersion)); + local_state->SetString(kWidevineInstalledVersion, + prefs->GetString(kWidevineInstalledVersion)); #endif } // Clear deprecated prefs. ClearWidevinePrefs(profile); } - -#endif diff --git a/browser/widevine/widevine_utils.h b/browser/widevine/widevine_utils.h index 8d60e30e8bd2..02ae9ef68602 100644 --- a/browser/widevine/widevine_utils.h +++ b/browser/widevine/widevine_utils.h @@ -31,7 +31,6 @@ std::string GetWidevineInstalledVersion(); void EnableWidevineCdmComponent(content::WebContents* web_contents); #endif -#if BUILDFLAG(ENABLE_WIDEVINE_CDM_COMPONENT) || BUILDFLAG(BUNDLE_WIDEVINE_CDM) void RegisterWidevineProfilePrefsForMigration( user_prefs::PrefRegistrySyncable* registry); int GetWidevinePermissionRequestTextFrangmentResourceId(); @@ -41,6 +40,5 @@ void DontAskWidevineInstall(content::WebContents* web_contents, bool dont_ask); bool IsWidevineOptedIn(); void SetWidevineOptedIn(bool opted_in); void MigrateWidevinePrefs(Profile* profile); -#endif #endif // BRAVE_BROWSER_WIDEVINE_WIDEVINE_UTILS_H_ diff --git a/chromium_src/chrome/browser/component_updater/widevine_cdm_component_installer.cc b/chromium_src/chrome/browser/component_updater/widevine_cdm_component_installer.cc index 46d25604d9f3..6d7fa2fdba7f 100644 --- a/chromium_src/chrome/browser/component_updater/widevine_cdm_component_installer.cc +++ b/chromium_src/chrome/browser/component_updater/widevine_cdm_component_installer.cc @@ -8,19 +8,13 @@ #undef RegisterWidevineCdmComponent #include "brave/browser/brave_browser_process_impl.h" +#include "brave/browser/widevine/widevine_utils.h" #include "brave/common/extensions/extension_constants.h" #include "chrome/browser/ui/webui/components_ui.h" #include "components/component_updater/component_updater_service.h" -#include "third_party/widevine/cdm/buildflags.h" - -#if BUILDFLAG(ENABLE_WIDEVINE_CDM_COMPONENT) -#include "brave/browser/widevine/widevine_utils.h" -#endif namespace component_updater { -#if BUILDFLAG(ENABLE_WIDEVINE_CDM_COMPONENT) - void OnWidevineRegistered() { ComponentsUI::OnDemandUpdate(widevine_extension_id); } @@ -35,15 +29,11 @@ void RegisterAndInstallWidevine() { base::Bind(&OnWidevineRegistered)); } -#endif - // Do nothing unless the user opts in! void RegisterWidevineCdmComponent(ComponentUpdateService* cus) { -#if BUILDFLAG(ENABLE_WIDEVINE_CDM_COMPONENT) DCHECK_CURRENTLY_ON(BrowserThread::UI); if (IsWidevineOptedIn()) RegisterAndInstallWidevine(); -#endif // defined(ENABLE_WIDEVINE_CDM_COMPONENT) } } // namespace component_updater diff --git a/chromium_src/chrome/browser/prefs/browser_prefs.cc b/chromium_src/chrome/browser/prefs/browser_prefs.cc index 4ce9096ed915..6f02bd691c22 100644 --- a/chromium_src/chrome/browser/prefs/browser_prefs.cc +++ b/chromium_src/chrome/browser/prefs/browser_prefs.cc @@ -7,7 +7,7 @@ #include "brave/browser/brave_local_state_prefs.h" #include "third_party/widevine/cdm/buildflags.h" -#if BUILDFLAG(ENABLE_WIDEVINE_CDM_COMPONENT) || BUILDFLAG(BUNDLE_WIDEVINE_CDM) +#if BUILDFLAG(ENABLE_WIDEVINE) #include "brave/browser/widevine/widevine_utils.h" #endif @@ -15,10 +15,12 @@ #include "../../../../chrome/browser/prefs/browser_prefs.cc" // NOLINT #undef MigrateObsoleteProfilePrefs +// This method should be periodically pruned of year+ old migrations. void MigrateObsoleteProfilePrefs(Profile* profile) { MigrateObsoleteProfilePrefs_ChromiumImpl(profile); -#if BUILDFLAG(ENABLE_WIDEVINE_CDM_COMPONENT) || BUILDFLAG(BUNDLE_WIDEVINE_CDM) +#if BUILDFLAG(ENABLE_WIDEVINE) + // Added 11/2019. MigrateWidevinePrefs(profile); #endif } diff --git a/chromium_src/chrome/browser/ui/views/permission_bubble/permission_prompt_impl.cc b/chromium_src/chrome/browser/ui/views/permission_bubble/permission_prompt_impl.cc index 27efb6923b1e..5942ca17a3c1 100644 --- a/chromium_src/chrome/browser/ui/views/permission_bubble/permission_prompt_impl.cc +++ b/chromium_src/chrome/browser/ui/views/permission_bubble/permission_prompt_impl.cc @@ -5,19 +5,24 @@ #include +#include "chrome/browser/permissions/permission_request.h" +#include "third_party/widevine/cdm/buildflags.h" +#include "ui/views/window/dialog_delegate.h" + +#if BUILDFLAG(ENABLE_WIDEVINE) #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 "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/style/typography.h" -#include "ui/views/window/dialog_delegate.h" +#endif namespace { +#if BUILDFLAG(ENABLE_WIDEVINE) class DontAskAgainCheckbox : public views::Checkbox, public views::ButtonListener { public: @@ -76,7 +81,12 @@ void AddAdditionalWidevineViewControlsIfNeeded( dialog_delegate->AddChildView(text); dialog_delegate->AddChildView(new DontAskAgainCheckbox(widevine_request)); } - +#else +void AddAdditionalWidevineViewControlsIfNeeded( + views::DialogDelegateView* dialog_delegate, + const std::vector& requests) { +} +#endif } // namespace #include "../../../../../../../chrome/browser/ui/views/permission_bubble/permission_prompt_impl.cc" // NOLINT diff --git a/chromium_src/chrome/renderer/media/chrome_key_systems.cc b/chromium_src/chrome/renderer/media/chrome_key_systems.cc new file mode 100644 index 000000000000..51245e59d5ed --- /dev/null +++ b/chromium_src/chrome/renderer/media/chrome_key_systems.cc @@ -0,0 +1,25 @@ +/* Copyright (c) 2019 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 + +#include "build/build_config.h" +#include "media/base/key_system_properties.h" +#include "third_party/widevine/cdm/buildflags.h" + +#if defined(OS_ANDROID) && !BUILDFLAG(ENABLE_WIDEVINE) +namespace cdm { +// Fix upstream build failure with widevine disabled on android. +// chrome_key_systems.cc calls cdm::AddAndroidWidevine() regardless of +// ENABLE_WIDEVINE build flag. +// But, it's only declared in that build flag. See android_key_systems.h. +void AddAndroidWidevine( + std::vector>* + concrete_key_systems) { +} +} // namespace cdm +#endif + +#include "../../../../../chrome/renderer/media/chrome_key_systems.cc" // NOLINT diff --git a/test/BUILD.gn b/test/BUILD.gn index 3b8ca185bbea..76a4fa454f14 100644 --- a/test/BUILD.gn +++ b/test/BUILD.gn @@ -277,12 +277,6 @@ test("brave_unit_tests") { ] } - if (is_win || is_mac) { - sources += [ - "//brave/chromium_src/chrome/browser/component_updater/widevine_cdm_component_installer_unittest.cc", - ] - } - if (is_mac) { sources += [ "//brave/chromium_src/chrome/common/chrome_constants_unittest_mac.cc", @@ -366,11 +360,15 @@ test("brave_unit_tests") { ] } - if (bundle_widevine_cdm) { - sources += [ - "//brave/browser/widevine/brave_widevine_bundle_manager_unittest.cc", - ] - deps += [ "//third_party/widevine/cdm:headers" ] + if (enable_widevine) { + if (is_win || is_mac) { + sources += [ "//brave/chromium_src/chrome/browser/component_updater/widevine_cdm_component_installer_unittest.cc" ] + } + + if (is_linux) { + sources += [ "//brave/browser/widevine/brave_widevine_bundle_manager_unittest.cc" ] + } + deps += [ "//brave/browser/widevine" ] } public_deps = [ @@ -545,10 +543,12 @@ test("brave_browser_tests") { ] deps = [] - if (bundle_widevine_cdm || enable_widevine_cdm_component) { + if (enable_widevine) { sources += [ "//brave/browser/widevine/widevine_permission_request_browsertest.cc", + "//brave/browser/widevine/widevine_prefs_migration_browsertest.cc", ] + deps += [ "//brave/browser/widevine" ] } if (enable_greaselion) { From d82b60e715886b0c6dc7c7fa9ad88ad06c906a45 Mon Sep 17 00:00:00 2001 From: Simon Hong Date: Tue, 19 Nov 2019 11:17:55 +0900 Subject: [PATCH 3/4] Add comments about f/u issue --- browser/brave_profile_prefs.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/browser/brave_profile_prefs.cc b/browser/brave_profile_prefs.cc index f5d57246b33a..35636f806916 100644 --- a/browser/brave_profile_prefs.cc +++ b/browser/brave_profile_prefs.cc @@ -59,9 +59,10 @@ void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry) { brave_sync::prefs::Prefs::RegisterProfilePrefs(registry); - // TODO(shong): Move this pref to ENABLE_WIDEVINE. + // 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); // Default Brave shields From 11884318e09ab2f04b12a1ddc528fb0b159ec287 Mon Sep 17 00:00:00 2001 From: Simon Hong Date: Tue, 19 Nov 2019 14:55:40 +0900 Subject: [PATCH 4/4] Modify WidevinePrefsMigrationTest local state is persisted but not persisted after clearing prefs in the spanning test. At second test, local state has the value before the clearing. I expected it has default value because ClearePref() is called but it's not default value at second test. So, testing is done by explicitly calling MigrateWidevinePrefs() instead of spanning test. --- .../widevine_prefs_migration_browsertest.cc | 37 ++++++++++++++++++- 1 file changed, 35 insertions(+), 2 deletions(-) diff --git a/browser/widevine/widevine_prefs_migration_browsertest.cc b/browser/widevine/widevine_prefs_migration_browsertest.cc index ad137eb39627..9d23b3771d18 100644 --- a/browser/widevine/widevine_prefs_migration_browsertest.cc +++ b/browser/widevine/widevine_prefs_migration_browsertest.cc @@ -3,21 +3,54 @@ * 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 "brave/browser/widevine/widevine_utils.h" #include "brave/common/pref_names.h" #include "chrome/browser/browser_process.h" +#include "chrome/browser/profiles/profile.h" +#include "chrome/browser/ui/browser.h" #include "chrome/test/base/in_process_browser_test.h" #include "components/prefs/pref_service.h" #include "third_party/widevine/cdm/buildflags.h" +namespace { +bool kWidevineOptedInTestValue = true; +#if BUILDFLAG(BUNDLE_WIDEVINE_CDM) +char kWidevineInstalledVersionTestValue[] = "1.2.3.4"; +#endif +} // namespace + using WidevinePrefsMigrationTest = InProcessBrowserTest; IN_PROC_BROWSER_TEST_F(WidevinePrefsMigrationTest, PrefMigrationTest) { - // After migration, local state doesn't have default value. - EXPECT_FALSE(g_browser_process->local_state()-> + g_browser_process->local_state()->ClearPref(kWidevineOptedIn); + EXPECT_TRUE(g_browser_process->local_state()-> FindPreference(kWidevineOptedIn)->IsDefaultValue()); +#if BUILDFLAG(BUNDLE_WIDEVINE_CDM) + g_browser_process->local_state()->ClearPref(kWidevineInstalledVersion); + EXPECT_TRUE(g_browser_process->local_state()-> + FindPreference(kWidevineInstalledVersion)->IsDefaultValue()); +#endif + + // Set profile prefs explicitly for migration test. + browser()->profile()->GetPrefs()->SetBoolean(kWidevineOptedIn, + kWidevineOptedInTestValue); +#if BUILDFLAG(BUNDLE_WIDEVINE_CDM) + browser()->profile()->GetPrefs()->SetString( + kWidevineInstalledVersion, + kWidevineInstalledVersionTestValue); +#endif + // Migrate and check it's done properly with previous profile prefs value. + MigrateWidevinePrefs(browser()->profile()); + EXPECT_FALSE(g_browser_process->local_state()-> + FindPreference(kWidevineOptedIn)->IsDefaultValue()); + EXPECT_EQ(kWidevineOptedInTestValue, + g_browser_process->local_state()->GetBoolean(kWidevineOptedIn)); #if BUILDFLAG(BUNDLE_WIDEVINE_CDM) EXPECT_FALSE(g_browser_process->local_state()-> FindPreference(kWidevineInstalledVersion)->IsDefaultValue()); + EXPECT_EQ( + kWidevineInstalledVersionTestValue, + g_browser_process->local_state()->GetString(kWidevineInstalledVersion)); #endif }