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

Enable Widevine VMP on MacOS and Windows #2023

Merged
merged 2 commits into from
Apr 3, 2019

Conversation

simonhong
Copy link
Member

@simonhong simonhong commented Mar 21, 2019

VMP is enabled with official build and all below envs are set.
SIGN_WIDEVINE_KEY: private key
SIGN_WIDEVINE_CERT: widevine cert
SIGN_WIDEVINE_PASSPHRASE: pass for key

sig generator should be located in
//third_party/widevine/scripts/signature_generator.py before build starts on buildbot.

Issue brave/brave-browser#944

Submitter Checklist:

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Verified that these changes build without errors on
    • Windows
    • macOS
    • Linux
  • Verified that these changes pass automated tests (npm test brave_unit_tests && npm test brave_browser_tests) on
    • Windows
    • macOS
    • Linux
  • Verified that all lint errors/warnings are resolved (npm run lint)
  • Ran git rebase master (if needed).
  • Ran git rebase -i to squash commits (if needed).
  • Tagged reviewers and labelled the pull request as needed.
  • Request a security/privacy review as needed.
  • Add appropriate QA labels (QA/Yes or QA/No) to include the closed issue in milestone

Test Plan:

  1. Check sig file is in installed folder.
    On MacOS, sig file(Brave Browser Framework.sig) is in Brave Browser.app/Contents/Versions/7x.x.x.x/Widevine Resources.bundle/Contents/Resources/Brave Browser Framework.sig
    On Windows, sig files(brave.exe.sig, chrome.dll.sig and chrome_child.dll.sig) are in C:\Program Files (x86)\BraveSoftware\Brave-Browser-XXX\Application\7x.x.x.x\

  2. Check netflix works

Reviewer Checklist:

  • New files have MPL-2.0 license header.
  • Request a security/privacy review as needed.
  • Adequate test coverage exists to prevent regressions
  • Verify test plan is specified in PR before merging to source

@simonhong simonhong self-assigned this Mar 21, 2019
@simonhong simonhong force-pushed the enable_cdm_host_verification branch 2 times, most recently from e595a9b to 2e1ce10 Compare March 21, 2019 09:25
@simonhong simonhong force-pushed the enable_cdm_host_verification branch 5 times, most recently from 9bd374d to ef65c7a Compare March 26, 2019 09:26
@simonhong simonhong force-pushed the enable_cdm_host_verification branch from ef65c7a to 0214901 Compare March 27, 2019 02:03
@simonhong simonhong requested review from bsclifton and darkdh March 27, 2019 04:02
@simonhong simonhong force-pushed the enable_cdm_host_verification branch 2 times, most recently from 86421d1 to 4dc1ca1 Compare March 27, 2019 10:01
@simonhong simonhong added this to the 0.64.x - Nightly milestone Mar 27, 2019
@simonhong simonhong marked this pull request as ready for review March 27, 2019 10:14
Copy link
Member

@darkdh darkdh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, need a rebase

@simonhong simonhong force-pushed the enable_cdm_host_verification branch 4 times, most recently from bbf2819 to 52a897b Compare March 29, 2019 02:28
@simonhong
Copy link
Member Author

@darkdh Rebased on master

@simonhong simonhong requested a review from darkdh March 29, 2019 02:43
darkdh
darkdh previously approved these changes Mar 29, 2019
@simonhong
Copy link
Member Author

I'll not merge this now because this PR makes conflict with C74 and C74 will be merged soon.

@simonhong simonhong removed this from the 0.64.x - Dev milestone Apr 3, 2019
@simonhong simonhong added this to the 0.65.x - Nightly milestone Apr 3, 2019
VMP is enabled with official build and all below envs are set.
SIGN_WIDEVINE_KEY: private key
SIGN_WIDEVINE_CERT: widevine cert
SIGN_WIDEVINE_PASSPHRASE: pass for key

sig generator should be located in
//third_party/widevine/scripts/signature_generator.py
browser signing is removed from `create_dist` target.
Instead, build` step will do signing and generate widevine sig file.
Then, those signed files and sig files will be used during the
`create-dist`.
This is because gn python can't use external cryptgraphy module needed
by sig generator scripts. So, brave-browser `build` target will use
system python for it.
@simonhong
Copy link
Member Author

simonhong commented Apr 3, 2019

@darkdh Would you approve again? I rebased this PR on C74.
Verified this on my local Win/MacOS.

Copy link
Member

@bsclifton bsclifton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious why this is failing patching, when CI runs. It uses brave-browser branch enable_cdm_host_verification, which is properly rebased and has the correct Chromium 74 pinned: brave/brave-browser@65f03ec

@bsclifton
Copy link
Member

bsclifton commented Apr 3, 2019

Whoops - RE: the above, it needs brave/brave-browser#3903 (which isn't in enable_cdm_host_verification branch, which is what PR builder uses). Let me merge that commit in and start CI

Copy link
Member

@bsclifton bsclifton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved, pending CI passing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants