Use widevine component on linux #7228

simonhong · 2020-11-23T07:05:32Z

So far, we've been using our own widevine downloader because chromium on linux didn't support it via component.
At that time, chrome also only used bundled widevine.
But, chromium started to support to get widevine with component. So, we don't need to use our module for fetching widevine.

If enable_widevine is enabled, enable_widevine_cdm_component is turned on by default for non-google-chrome branding.
So, we don't need to touch widevine.gni anymore.

Resolves brave/brave-browser#7081
Resolves brave/brave-browser#7831
Resolves brave/brave-browser#3955

Test scenario:

Test with clean profile

Launch browser with clean profile
Check widevine is not available from brave://components
Load netflix and agree for installing widevine
Check widevine is available from brave://components
Relaunch browser and check netflix works.

Test with old profile(widevine enabled)

Launch browser
Check widevine is available from brave://components and it's version is 4.10.1610.0
Load netflix and check netflix works
Relaunch browser and load netflix and check netflix works

Test case

npm run test brave_unit_tests -- --filter=*Widevine*
npm run test brave_browser_tests --filter=*Widevine*

Submitter Checklist:

There is a ticket for my issue.
Used Github auto-closing keywords in the commit message.
Wrote a good PR/commit description
Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
Checked the PR locally: npm run test -- brave_browser_tests, npm run test -- brave_unit_tests, npm run lint, npm run gn_check, npm run tslint
Ran git rebase master (if needed).
Requested a security/privacy review as needed.

Reviewer Checklist:

New files have MPL-2.0 license header.
Adequate test coverage exists to prevent regressions
Major classes, functions and non-trivial code blocks are well-commented
Changes in component dependencies are properly reflected in gn
Code follows the style guide
Test plan is specified in PR before merging

After-merge Checklist:

Test Plan:

npm run test brave_unit_tests -- --filter=WidevineBuildFlag.FlagTest

simonhong · 2020-11-24T00:48:37Z

Hmm, it looks like CI doesn't work well with dedicated b-b branch. will try to run CI again after deleting b-b branch.

simonhong · 2020-11-24T01:09:32Z

patches/third_party-widevine-cdm-widevine.gni.patch

- # Widevine CDM is bundled as part of Google Chrome builds.
- bundle_widevine_cdm = enable_library_widevine_cdm && is_chrome_branded
-+bundle_widevine_cdm = enable_library_widevine_cdm && is_desktop_linux
-+enable_widevine_cdm_component = enable_library_widevine_cdm && (is_win || is_mac)


We will only enable enable_widevine_cdm_component for all desktop platform.

simonhong · 2020-11-24T01:09:56Z

test/BUILD.gn

-     "//third_party/widevine/cdm:headers",
-    ]
+  if (enable_widevine && !is_android) {
+    sources += [ "//brave/chromium_src/chrome/browser/component_updater/widevine_cdm_component_installer_unittest.cc" ]


Will move this to separate target.

simonhong · 2020-11-24T03:06:41Z

test/BUILD.gn

@@ -652,14 +640,17 @@ if (!is_android) {
      "//media:test_support",
    ]

-    if (enable_widevine && !is_asan) {
+    if (enable_widevine_cdm_component && !is_asan) {
      # Remove the !is_asan check once
      # https://github.com/brave/brave-browser/issues/11901 is fixed
      sources += [
        "//brave/browser/widevine/widevine_permission_request_browsertest.cc",
        "//brave/browser/widevine/widevine_prefs_migration_browsertest.cc",


simonhong · 2020-11-25T06:02:28Z

Ready to review! - ~~Forgot to delete old widevine binaries. Will add cleanup logic~~ -> Done.

simonhong · 2020-11-26T04:54:50Z

PTAL!

iefremov · 2020-11-26T10:08:29Z

common/brave_content_client.cc

+// loaded after re-launching.
+void CreateDefaultWidevineCdmHintFile() {
+  base::FilePath hint_file_path;
+  CHECK(base::PathService::Get(chrome::FILE_COMPONENT_WIDEVINE_CDM_HINT,


are you sure we should kill the browser?

Yes, cdm lib should be loaded during the zygot initialization before sandboxing.

It's linux-only constraint.

mkarolin

chromium_src/patches changes LGTM

simonhong · 2020-12-02T04:51:57Z

Kindly ping :)

iefremov · 2020-12-03T10:27:08Z

browser/brave_drm_tab_helper.h

@@ -22,6 +22,10 @@ class BraveDrmTabHelper final
      public brave_drm::mojom::BraveDRM,
      public component_updater::ComponentUpdateService::Observer {
 public:
+  // Copied from widevine_cdm_component_installer.cc.
+  // There is no shared constant value.
+  static char kWidevineComponentId[];


iefremov · 2020-12-03T10:33:17Z

browser/widevine/widevine_permission_request.h

@@ -16,7 +17,8 @@ class WebContents;

 class WidevinePermissionRequest : public permissions::PermissionRequest {
 public:
-  explicit WidevinePermissionRequest(content::WebContents* web_contents);
+  explicit WidevinePermissionRequest(content::WebContents* web_contents,


not explicit now

iefremov · 2020-12-03T10:46:44Z

browser/widevine/widevine_utils.cc

-  }
+bool DoDeleteOldWidevineBinary() {
+  base::FilePath widevine_base_path;
+  CHECK(base::PathService::Get(chrome::DIR_USER_DATA,


I'm slightly concerned with these CHECKs, since CHECK is for critical situation when the browser just cannot work anymore (i.e. detecting a security breach). Inability to delete the widevine binary (or get a path) doesn't look like a critical problem that prevents the program from working

Ok. Changed to early return if it gives false.

Chromium started to support widevine cdm component since c79. Before that, only bundling was supported. So, we've been using our widevine manager for downloading/managing widevine during the runtime. With enabling widevine cdm component, all our codes are deleted.

simonhong · 2020-12-04T08:41:51Z

Merged because only unrelated lint & gn check were failed and all are fixed by other PRs.

simonhong self-assigned this Nov 23, 2020

simonhong force-pushed the widevine_component_linux branch 2 times, most recently from 520f34d to 1754972 Compare November 23, 2020 08:23

simonhong changed the title ~~Use widevine component on linux~~ WIP: Use widevine component on linux Nov 23, 2020

simonhong marked this pull request as ready for review November 23, 2020 10:02

simonhong requested a review from a team as a code owner November 23, 2020 10:02

simonhong force-pushed the widevine_component_linux branch 2 times, most recently from 6ef540a to 9df3896 Compare November 23, 2020 11:10

simonhong changed the title ~~WIP: Use widevine component on linux~~ Use widevine component on linux Nov 23, 2020

simonhong force-pushed the widevine_component_linux branch 2 times, most recently from e287548 to 6551056 Compare November 23, 2020 11:51

simonhong changed the title ~~Use widevine component on linux~~ WIP: Use widevine component on linux Nov 23, 2020

simonhong force-pushed the widevine_component_linux branch from 6551056 to e64347b Compare November 23, 2020 12:12

simonhong requested review from bsclifton and bbondy November 24, 2020 00:49

simonhong commented Nov 24, 2020

View reviewed changes

simonhong force-pushed the widevine_component_linux branch from e64347b to aa02219 Compare November 24, 2020 01:39

simonhong commented Nov 24, 2020

View reviewed changes

simonhong marked this pull request as draft November 25, 2020 03:49

simonhong force-pushed the widevine_component_linux branch from aa02219 to d16a5b5 Compare November 25, 2020 03:53

simonhong changed the title ~~WIP: Use widevine component on linux~~ Use widevine component on linux Nov 25, 2020

simonhong marked this pull request as ready for review November 25, 2020 04:56

simonhong force-pushed the widevine_component_linux branch from 83cc26a to 3a59238 Compare November 25, 2020 05:05

simonhong requested a review from iefremov November 25, 2020 05:31

iefremov reviewed Nov 26, 2020

View reviewed changes

simonhong force-pushed the widevine_component_linux branch from eafea07 to c3b9d11 Compare November 29, 2020 00:06

simonhong force-pushed the widevine_component_linux branch from c3b9d11 to 9f1824b Compare November 29, 2020 01:10

mkarolin approved these changes Nov 30, 2020

View reviewed changes

simonhong force-pushed the widevine_component_linux branch from 9f1824b to b755b5e Compare December 2, 2020 04:51

simonhong requested a review from kkuehlz December 3, 2020 02:20

simonhong force-pushed the widevine_component_linux branch from b755b5e to 4137f21 Compare December 3, 2020 02:39

iefremov reviewed Dec 3, 2020

View reviewed changes

simonhong added 5 commits December 3, 2020 20:37

Use separate targets for widevine test

90e0483

Use proper text for widevine permission req

390b484

Cleanup old widevine binary

352be56

Address review comments

9a94d62

simonhong force-pushed the widevine_component_linux branch from 4137f21 to 9a94d62 Compare December 3, 2020 12:14

iefremov approved these changes Dec 4, 2020

View reviewed changes

simonhong merged commit a4d6c35 into master Dec 4, 2020

simonhong deleted the widevine_component_linux branch December 4, 2020 08:42

simonhong added this to the 1.20.x - Nightly milestone Dec 4, 2020

This was referenced Jan 21, 2021

Enable widevine cdm component updater in linux brave/brave-browser#7081

Closed

Linux doesn't list Widevine component module in brave://components brave/brave-browser#3955

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use widevine component on linux #7228

Use widevine component on linux #7228

simonhong commented Nov 23, 2020 •

edited

Loading

simonhong commented Nov 24, 2020

simonhong Nov 24, 2020

simonhong Nov 24, 2020

simonhong Nov 24, 2020

simonhong commented Nov 25, 2020 •

edited

Loading

simonhong commented Nov 26, 2020

iefremov Nov 26, 2020

simonhong Nov 26, 2020

simonhong Nov 26, 2020

mkarolin left a comment

simonhong commented Dec 2, 2020

iefremov Dec 3, 2020

simonhong Dec 3, 2020

iefremov Dec 3, 2020

simonhong Dec 3, 2020

iefremov Dec 3, 2020

simonhong Dec 3, 2020

simonhong commented Dec 4, 2020

Use widevine component on linux #7228

Use widevine component on linux #7228

Conversation

simonhong commented Nov 23, 2020 • edited Loading

Test scenario:

Test with clean profile

Test with old profile(widevine enabled)

Test case

Submitter Checklist:

Reviewer Checklist:

After-merge Checklist:

Test Plan:

simonhong commented Nov 24, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

simonhong commented Nov 25, 2020 • edited Loading

simonhong commented Nov 26, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mkarolin left a comment

Choose a reason for hiding this comment

simonhong commented Dec 2, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

simonhong commented Dec 4, 2020

simonhong commented Nov 23, 2020 •

edited

Loading

simonhong commented Nov 25, 2020 •

edited

Loading