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

Remove Widevine ARM64 DLL fix #23415

Merged
merged 6 commits into from
May 7, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
#include "brave/browser/net/brave_geolocation_buildflags.h"
#include "brave/browser/safebrowsing/buildflags.h"
#include "brave/components/constants/network_constants.h"
#include "brave/components/widevine/static_buildflags.h"
#include "extensions/common/url_pattern.h"
#include "net/base/net_errors.h"

Expand Down Expand Up @@ -88,10 +87,6 @@ int OnBeforeURLRequest_StaticRedirectWorkForGURL(
static base::NoDestructor<URLPattern> widevine_google_dl_pattern(
URLPattern::SCHEME_HTTP | URLPattern::SCHEME_HTTPS,
kWidevineGoogleDlPrefix);
#if BUILDFLAG(WIDEVINE_ARM64_DLL_FIX)
static base::NoDestructor<URLPattern> widevine_google_dl_pattern_win_arm64(
URLPattern::SCHEME_HTTPS, kWidevineGoogleDlPrefixWinArm64);
#endif // BUILDFLAG(WIDEVINE_ARM64_DLL_FIX)

if (geo_pattern->MatchesURL(request_url)) {
*new_url = GURL(BUILDFLAG(GOOGLEAPIS_URL));
Expand Down Expand Up @@ -170,9 +165,6 @@ int OnBeforeURLRequest_StaticRedirectWorkForGURL(
}

if (googleDl_pattern->MatchesURL(request_url) &&
#if BUILDFLAG(WIDEVINE_ARM64_DLL_FIX)
!widevine_google_dl_pattern_win_arm64->MatchesURL(request_url) &&
#endif
!widevine_google_dl_pattern->MatchesURL(request_url)) {
replacements.SetSchemeStr("https");
replacements.SetHostStr(kBraveRedirectorProxy);
Expand Down
1 change: 0 additions & 1 deletion browser/net/sources.gni
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ brave_browser_net_deps = [
"//brave/components/localhost_permission",
"//brave/components/query_filter",
"//brave/components/update_client:buildflags",
"//brave/components/widevine:static_buildflags",
"//brave/extensions:common",
"//chrome/browser:browser_process",
"//chrome/browser/profiles",
Expand Down
74 changes: 0 additions & 74 deletions browser/widevine/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,77 +28,3 @@ with a redirect to Google's server in this case. This works as long as a single
update check request only polls for Widevine, and not also for any other
components. We have a special class, `SequentialUpdateChecker`, that makes sure
that this is the case.

### Widevine in Arm64 Brave on Windows: WIDEVINE_ARM64_DLL_FIX

As of this writing, Google does not offer Arm64 binaries for Widevine on
Windows. When Arm64 Brave polls Google's component update server for Widevine,
then it receives a "noupdate" response, meaning that no version could be found.
In effect, out of the box, Widevine cannot be installed in Arm64 Brave on
Windows.

However, there is a workaround: When we poll for an x64 version of the
component, then we get the necessary base files, plus some x64-specific
binaries. The remaining necessary binaries for Arm64 are available in a Zip file
on Google's servers. By placing them into the
`WidevineCdm\<version>\_platform_specific\win_arm64` directory of the component,
we can obtain a copy of Widevine that works in Arm64 Brave on Windows.

Brave's `WIDEVINE_ARM64_DLL_FIX` implements the above workaround in code. When
an update check for Widevine returns "noupdate", then the workaround repeats the
update check, pretending to be from the `x64` architecture. It installs the x64
version of the component and then additionally downloads the Arm64 zip file from
Google's server. This gives us a working copy of Widevine in Arm64 Brave.

The facts that upstream can be compiled for Arm64 on Windows and that Google
publicly serves Arm64 binaries of Widevine make it seem likely that a native
Widevine component for this architecture will also be available in the future.
For this reason, our workaround is guarded by a build flag. Once the workaround
is no longer required, the build flag makes it very easy to clean up the
associated code.

**Further details about the present implementation of WIDEVINE_ARM64_DLL_FIX:**

The details below were compiled at the time of the initial implementation, to
simplify the associated code review. They may be wholly or partially out of
date by the time you read this.

The first entry point into the implementation is in `update_checker.cc`, in
`SequentialUpdateChecker::UpdateResultAvailable`. It handles the symptom of the
current limitation: The browser asks the component update server "Do you have
the Widevine component?", implicitly sending its architecture, which is Arm64.
The server responds `"noupdate"`, which means there is no such version. When
this happens, `UpdateResultAvailable` repeats the update check with a fake
`x64` architecture.

There is some additional code in `UpdateResultAvailable` that handles the case
when the server responds `noupdate` not because there is no Arm64 version of
Widevine at all, but because there is just no _new_ version. If we ever receive
an Arm64 version of Widevine from the update server, then we disable our
workaround to not fall back to `x64`. This information is persisted via
`SequentialUpdateChecker::SetPersistedFlag` and `...:GetPersistedFlag`.

The fake architecture is passed from `update_checker.cc` to
`protocol_serializer.cc` via the `additional_attributes` parameter of
`MakeProtocolRequest`.

When the browser receives the response from the update server, it installs the
Widevine component. Upstream has some custom logic for this in
`widevine_cdm_component_installer.cc`. We extend this logic to in particular
overwrite `WidevineCdmComponentInstallerPolicy::OnCustomInstall`. This gets
called by upstream when the "normal" installation of the component is complete.
Our additional code downloads and unpacks the external zip file with the
necessary Arm64 binaries. It also applies some patches to make upstream accept
the foreign implementation.

To be able to download the Arm64 binaries, the `OnCustomInstall` method
mentioned above needs a `SharedURLLoaderFactory`. Several sections of our code
make sure that it receives such an instance in our and upstream's calls of
`RegisterWidevineCdmComponent`.

Brave intercepts network requests to certain domains for privacy reasons. One
such domain is `dl.google.com`, which Brave redirects to `redirector.brave.com`.
Our attempt to download the Zip file with additional Arm64 binaries would
normally be prevented by this mechanism. We add an exception to
`brave_static_redirect_network_delegate_helper.cc` so that the Zip file can in
fact be downloaded.
4 changes: 0 additions & 4 deletions browser/widevine/widevine_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
#include "brave/browser/widevine/widevine_permission_request.h"
#include "brave/components/constants/pref_names.h"
#include "brave/components/widevine/constants.h"
#include "brave/components/widevine/static_buildflags.h"
#include "brave/grit/brave_generated_resources.h"
#include "build/build_config.h"
#include "chrome/browser/browser_process.h"
Expand Down Expand Up @@ -41,9 +40,6 @@ void EnableWidevineCdm() {
SetWidevineEnabled(true);
#if BUILDFLAG(ENABLE_WIDEVINE_CDM_COMPONENT)
RegisterWidevineCdmComponent(g_browser_process->component_updater(),
#if BUILDFLAG(WIDEVINE_ARM64_DLL_FIX)
g_browser_process->shared_url_loader_factory(),
#endif
base::BindOnce(&InstallWidevineOnceRegistered));
#endif
}
Expand Down
4 changes: 0 additions & 4 deletions chromium_src/base/threading/DEPS

This file was deleted.

29 changes: 4 additions & 25 deletions chromium_src/base/threading/thread_restrictions.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@
#ifndef BRAVE_CHROMIUM_SRC_BASE_THREADING_THREAD_RESTRICTIONS_H_
#define BRAVE_CHROMIUM_SRC_BASE_THREADING_THREAD_RESTRICTIONS_H_

#include "brave/components/widevine/static_buildflags.h"

class BraveBrowsingDataRemoverDelegate;
namespace ipfs {
class IpfsService;
Expand All @@ -16,32 +14,13 @@ namespace brave {
class ProcessLauncher;
}

#if BUILDFLAG(WIDEVINE_ARM64_DLL_FIX)
namespace component_updater {
class WidevineArm64DllInstaller;
}
#endif

#define BRAVE_SCOPED_ALLOW_BASE_SYNC_PRIMITIVES_H_BASE \
friend class ::BraveBrowsingDataRemoverDelegate; \
friend class ipfs::IpfsService; \
#define BRAVE_SCOPED_ALLOW_BASE_SYNC_PRIMITIVES_H \
friend class ::BraveBrowsingDataRemoverDelegate; \
friend class ipfs::IpfsService; \
friend class brave::ProcessLauncher;

#if BUILDFLAG(WIDEVINE_ARM64_DLL_FIX)
// WidevineArm64DllInstaller needs to use TimedWait:
#define BRAVE_SCOPED_ALLOW_BASE_SYNC_PRIMITIVES_WIDEVINE_ARM64_DLL_FIX \
friend class component_updater::WidevineArm64DllInstaller;
#else
#define BRAVE_SCOPED_ALLOW_BASE_SYNC_PRIMITIVES_WIDEVINE_ARM64_DLL_FIX
#endif

#define BRAVE_SCOPED_ALLOW_BASE_SYNC_PRIMITIVES_H \
BRAVE_SCOPED_ALLOW_BASE_SYNC_PRIMITIVES_H_BASE \
BRAVE_SCOPED_ALLOW_BASE_SYNC_PRIMITIVES_WIDEVINE_ARM64_DLL_FIX

#include "src/base/threading/thread_restrictions.h" // IWYU pragma: export

#undef BRAVE_SCOPED_ALLOW_BASE_SYNC_PRIMITIVES_H
#undef BRAVE_SCOPED_ALLOW_BASE_SYNC_PRIMITIVES_H_BASE
#undef BRAVE_SCOPED_ALLOW_BASE_SYNC_PRIMITIVES_WIDEVINE_ARM64_DLL_FIX

#endif // BRAVE_CHROMIUM_SRC_BASE_THREADING_THREAD_RESTRICTIONS_H_
1 change: 0 additions & 1 deletion chromium_src/check_chromium_src_config.json5
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
"components/privacy_sandbox/privacy_sandbox_settings_unittest.cc",
"components/search_engines/brave_template_url_prepopulate_data_unittest.cc",
"components/search_engines/brave_template_url_service_util_unittest.cc",
"components/update_client/brave_persisted_data_unittest.cc",
"components/variations/service/field_trial_unittest.cc",
"net/base/brave_proxy_string_util_unittest.cc",
"net/cookies/brave_canonical_cookie_unittest.cc",
Expand Down
3 changes: 0 additions & 3 deletions chromium_src/chrome/browser/component_updater/DEPS

This file was deleted.

11 changes: 0 additions & 11 deletions chromium_src/chrome/browser/component_updater/registration.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,22 +5,11 @@

#include "chrome/browser/component_updater/registration.h"
#include "base/functional/bind.h"
#include "brave/components/widevine/static_buildflags.h"
#include "chrome/browser/component_updater/widevine_cdm_component_installer.h"

#define RegisterComponentsForUpdate RegisterComponentsForUpdate_ChromiumImpl

#if BUILDFLAG(WIDEVINE_ARM64_DLL_FIX)
#define RegisterWidevineCdmComponent(cus) \
RegisterWidevineCdmComponent(cus, \
g_browser_process->shared_url_loader_factory())
#else
#define RegisterWidevineCdmComponent(cus) RegisterWidevineCdmComponent(cus)
#endif

#include "src/chrome/browser/component_updater/registration.cc"

#undef RegisterWidevineCdmComponent
#undef RegisterComponentsForUpdate

#include "brave/components/brave_wallet/browser/wallet_data_files_installer.h"
Expand Down
Loading
Loading