Don't try to download Widevine by default on Tier3 platforms as well

RESOLVED FIXED in Firefox 50

Status

defect
RESOLVED FIXED
3 years ago
a year ago

People

(Reporter: jbeich, Assigned: cpearce)

Tracking

Trunk
mozilla52
Unspecified
FreeBSD
Dependency tree / graph

Firefox Tracking Flags

(firefox49 wontfix, firefox50 fixed, firefox51 fixed, firefox52 fixed)

Details

Attachments

(2 attachments)

Reporter

Description

3 years ago
+++ This bug was initially created as a clone of Bug #1295853 +++

Bug 1295853 failed to remove Widevine from about:addons#plugins.
Comment hidden (mozreview-request)
Attached patch at least makes `./mach configure` work on FreeBSD, and will basically partially revert some of the changes I made to address the review comments for bug 1295853 comment 21.
Reporter

Comment 3

3 years ago
Comment on attachment 8788019 [details]
Bug 1299694 - Ensure we don't enable Widevine unintentionally.

mozbuild.frontend.reader.BuildReaderError:
==============================
ERROR PROCESSING MOZBUILD FILE
==============================

The error occurred while processing the following file:

    browser/app/moz.build

The error was triggered on line 45 of this file:

    for cdm in CONFIG['MOZ_EME_MODULES']:

An error was encountered as part of executing the file itself. The error appears to be the fault of the script.

The error as reported by Python is:

    ["TypeError: 'NoneType' object is not iterable\n"]

*** Fix above errors and then restart with\
               "/usr/local/bin/gmake -f client.mk build"
gmake: *** [client.mk:377: configure] Error 1
Attachment #8788019 - Flags: feedback-
(In reply to Jan Beich from comment #3)
> Comment on attachment 8788019 [details]
> Bug 1299694 - Ensure we don't enable Widevine unintentionally.
> 
> The error was triggered on line 45 of this file:
> 
>     for cdm in CONFIG['MOZ_EME_MODULES']:


Right. That needs to be:

def eme_modules(value):
    if value:
        for cdm in value:
            log.info('EME key system %s enabled' % cdm.upper())
    return value

Comment 5

3 years ago
mozreview-review
Comment on attachment 8788019 [details]
Bug 1299694 - Ensure we don't enable Widevine unintentionally.

https://reviewboard.mozilla.org/r/76554/#review74920

::: toolkit/moz.configure
(Diff revision 1)
>          target.os not in ('Android', 'iOS') and
>          target.cpu in ('x86', 'x86_64')):
>          return value
>      elif value and value.origin != 'default':
>          die('%s is not supported on %s' % (value.format('--enable-eme'), target.alias))
> -    return value

cf. subsequent comments, this doesn't work without modifying the eme_modules function. But it would be better not to touch it.

I'd suggest this:
    # Return the same type of OptionValue (Positive or Negative), with an empty tuple.
    return value.__class__(()).
Attachment #8788019 - Flags: review?(mh+mozilla)
Comment hidden (mozreview-request)

Comment 8

3 years ago
mozreview-review
Comment on attachment 8788019 [details]
Bug 1299694 - Ensure we don't enable Widevine unintentionally.

https://reviewboard.mozilla.org/r/76554/#review75294
Attachment #8788019 - Flags: review?(mh+mozilla) → review+
Jan Beich: Does the above patch work for you?
Flags: needinfo?(jbeich)
Reporter

Comment 11

3 years ago
Comment on attachment 8788019 [details]
Bug 1299694 - Ensure we don't enable Widevine unintentionally.

Widevine in about:addons#plugins and MOZ_EME_MODULES in obj/config/autoconf.mk disappeared as intended. Thanks.
Flags: needinfo?(jbeich)
Attachment #8788019 - Flags: feedback+
Reporter

Comment 12

3 years ago
(In reply to Chris Pearce (:cpearce) from comment #10)
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=10696291b91d

Maybe use Automation -> Trigger a Try Build via MozReview (Reviews tab) next time. For a second I thought you've posted a different version.
With this patch the build fails on Android, as now we correctly don't enable Widevine on Android...

libdom_media_mediasource_gtest.a.desc
42:32.27 In file included from /Users/CPearce/src/mozilla/central/dom/media/gtest/TestGMPCrossOrigin.cpp:15:0,
42:32.27                  from /Users/CPearce/src/mozilla/central/objdir-android/dom/media/gtest/Unified_cpp_dom_media_gtest0.cpp:20:
42:32.27 /Users/CPearce/src/mozilla/central/dom/media/gmp/GMPDecryptorProxy.h:9:44: fatal error: mozilla/DecryptorProxyCallback.h: No such file or directory
42:32.27  #include "mozilla/DecryptorProxyCallback.h"
42:32.27                                             ^
42:32.38 compilation terminated.

These errors are reported in Bug 1299627.

This is a consequence of Bug 1290830, which is in Firefox 51, and should be fixed when Bug 1300654 lands.

So I can't land this patch until Bug 1300654 lands. :(
Depends on: 1300654

Comment 16

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/7f7c7197e8ea
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Is it something that we want to uplift? If so, please submit the uplift request. Thanks
Comment on attachment 8788019 [details]
Bug 1299694 - Ensure we don't enable Widevine unintentionally.

Approval Request Comment
[Feature/regressing bug #]: EME on tier 3 Firefox builds.
[User impact if declined]: Firefox on Tier 3 systems will show UI by default saying that EME plugins can be downloaded, but the download will never succeed, and indeed EME will never work there.
[Describe test coverage new/current, TreeHerder]: We've got EME test coverage, so we'll be sure we don't regress our Tier 1 builds.
[Risks and why]: Low; build system change.
[String/UUID change made/needed]: None.
Attachment #8788019 - Flags: approval-mozilla-beta?
Attachment #8788019 - Flags: approval-mozilla-aurora?
Comment on attachment 8788019 [details]
Bug 1299694 - Ensure we don't enable Widevine unintentionally.

We tried fixing this in bug 1295853, it seems we need more, Aurora51+, Beta50+
Attachment #8788019 - Flags: approval-mozilla-beta?
Attachment #8788019 - Flags: approval-mozilla-beta+
Attachment #8788019 - Flags: approval-mozilla-aurora?
Attachment #8788019 - Flags: approval-mozilla-aurora+
Assignee: nobody → cpearce

Updated

a year ago
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.