Cleanup Widevine build flag usages and add some migration tests

Add WidevinePrefsMigrationTest.PrefMigrationTest for prefs migration test.
brave · Nov 18, 2019 · dec8edf · dec8edf
1 parent 0b748d9
commit dec8edf
Show file tree

Hide file tree

Showing 15 changed files with 114 additions and 75 deletions.
diff --git 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
@@ -5,22 +5,19 @@
 
 #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) {}
 
 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<Profile*>(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
@@ -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,15 +34,13 @@ 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;
 
   // 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
@@ -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
@@ -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
@@ -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"
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -7,18 +7,20 @@
 #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
 
 #define MigrateObsoleteProfilePrefs MigrateObsoleteProfilePrefs_ChromiumImpl
 #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
 }