Closed Bug 1299627 Opened 5 years ago Closed 5 years ago

Compilation fails with --disable-eme: "dom/media/gmp/GMPDecryptorProxy.h:9:10: fatal error: 'mozilla/DecryptorProxyCallback.h' file not found"

Categories

(Core :: Audio/Video: GMP, defect, P3)

defect

Tracking

()

RESOLVED FIXED
Tracking Status
firefox51 --- affected

People

(Reporter: mats, Assigned: JamesCheng)

References

Details

STEPS TO REPRODUCE
1. add the following to your .mozconfig file:
ac_add_options --disable-eme

2. ./mach build

ACTUAL RESULTS
In file included from dom/media/gmp/Unified_cpp_dom_media_gmp0.cpp:47:
In file included from dom/media/gmp/GMPContentParent.cpp:8:
In file included from dom/media/gmp/GMPDecryptorParent.h:12:
dom/media/gmp/GMPDecryptorProxy.h:9:10: fatal error: 'mozilla/DecryptorProxyCallback.h' file not found
#include "mozilla/DecryptorProxyCallback.h"
         ^
libgfx_ycbcr.a.desc
1 error generated.


mozilla-central (rev 312017:b3ec8a3373e8) debug build on Linux
I will try to fix this.
Assignee: nobody → jacheng
Thank you James.

I just had a quick look just before you took the bug, this seems to come from bug 1290830.
Unfortunately the whole dom/media/eme directory is skipped for --disable-eme, so the DecryptorProxyCallback interface cannot be used outside of it; you might have to undo https://hg.mozilla.org/mozilla-central/rev/3942a7c1224c .
Blocks: 1290830
Currently I have no idea how to fix these errors due to some reasons,

1. DecryptorProxyCallback.h has included three headers which are available only when MOZ_EME is enabled

#include "mozilla/dom/MediaKeyStatusMapBinding.h" // For MediaKeyStatus
#include "mozilla/dom/MediaKeyMessageEventBinding.h" // For MediaKeyMessageType
#include "mozilla/CDMProxy.h"

2. DecryptorProxyCallback.h is used by GMPDecryptorProxy.h  [1]

These two header is excluded by MOZ_EME
GMPCDMCallbackProxy.h
GMPCDMProxy.h

But these two will be compiled no matter MOZ_EME is disabled.

GMPDecryptorParent.h
TestGMPCrossOrigin.cpp

I wonder why we did not stop building GMPDecryptorXXX headers and implementation when MOZ_EME is not defined.

Is there any use case the GMPDecryptorXXX could be used without Javascript EME API?

If the answer is no, I think we should disable building GMPDecryptorXXX API when MOZ_EME is not defined.


Hi SingingTree, do you have any idea about my questions?

Thank you.

[1] https://dxr.mozilla.org/mozilla-central/search?q=GMPDecryptorProxy.h&redirect=false
Flags: needinfo?(bvandyk)
:cpearce will also know about this, as the module owner. He's on PTO until next week, but once he's back he should be able offer further insight or let you know if anything I've said is incorrect. I've need infoed him, comment on this at the end.

(In reply to James Cheng[:JamesCheng] from comment #3)
> 
> I wonder why we did not stop building GMPDecryptorXXX headers and
> implementation when MOZ_EME is not defined.

I believe we'd like to remove MOZ_EME defines throughout the code. Motivation for this is lowering the complexity of having them around: developers having to worry about them, different builds, testing, etc. We can also use GMPs for non EME related code, and managing conditional compilation around that adds complexity. That said, I understand the desire to keep DRM optional, though I think that is achieved by process separation of the CDM.

> Is there any use case the GMPDecryptorXXX could be used without Javascript
> EME API?

I don't think so.

> If the answer is no, I think we should disable building GMPDecryptorXXX API
> when MOZ_EME is not defined.

My personal preference would be to go in the other direction and have the code compiled in all cases. However, this is something that warrants discussion. This discussion may have already happened and I don't know about it, or it may need to take place. I think it's best if we can to see what he thinks before going either way.
Flags: needinfo?(bvandyk) → needinfo?(cpearce)
(In reply to Bryce Van Dyk (:SingingTree) from comment #4)
> :cpearce will also know about this, as the module owner. He's on PTO until
> next week, but once he's back he should be able offer further insight or let
> you know if anything I've said is incorrect. I've need infoed him, comment
> on this at the end.
> 
> (In reply to James Cheng[:JamesCheng] from comment #3)
> > 
> > I wonder why we did not stop building GMPDecryptorXXX headers and
> > implementation when MOZ_EME is not defined.
> 
> I believe we'd like to remove MOZ_EME defines throughout the code.
> Motivation for this is lowering the complexity of having them around:
> developers having to worry about them, different builds, testing, etc. We
> can also use GMPs for non EME related code, and managing conditional
> compilation around that adds complexity. That said, I understand the desire
> to keep DRM optional, though I think that is achieved by process separation
> of the CDM.
> 
> > Is there any use case the GMPDecryptorXXX could be used without Javascript
> > EME API?
> 
> I don't think so.
> 
> > If the answer is no, I think we should disable building GMPDecryptorXXX API
> > when MOZ_EME is not defined.
> 
> My personal preference would be to go in the other direction and have the
> code compiled in all cases. However, this is something that warrants
> discussion. This discussion may have already happened and I don't know about
> it, or it may need to take place. I think it's best if we can to see what he
> thinks before going either way.

Thank you,

I know the GMPs could be used for non EME related code.

But I just want to make GMPDecryptorXXX which is only used by EME(I am not pretty sure about this)

not to be compiled when MOZ_EME is defined.

I think this is the only way to make this bug be fixed :(

I still want to know why we need a build for disable MOZ_EME....We can only prevent user to use EME from the preference.

https://dxr.mozilla.org/mozilla-central/source/dom/webidl/Navigator.webidl#429,431
Rank: 20
Priority: -- → P2
In my opinion we don't need a build that disables EME, I think the other means by which is can be disabled or hidden are sufficient.

If wrapping more code in the MOZ_EME conditional compilation is the only way to resolve this now, and this is blocking you, I say do it, from a pragmatic stand point. If it's non-blocking I'd wait to see what :cpearce has to say.
I am happy to see we can get rid of MOZ_EME at all, I don't know that this kind of conditional build errors should be a blocking issue or not.  

I tried to fix this issue and I almost move all the essential source code out of MOZ_EME enabled blocks...

This is ridiculous....

I'd wait for cpearce :) hope we can extinguish MOZ_EME from this time on.
We should remove MOZ_EME. It's better to have EME disabled by preferences, so that in builds where EME is disabled, users are prompted to enable it when the browser encounters EME content on the web. Otherwise, users will see mysterious failures.
Flags: needinfo?(cpearce)
Priority: P2 → P3
Depends on: 1300654
I've created Bug 1300654 as a follow-up bug.
Hi mats,

I think Bug 1300654 is fixing the compile error of this bug.

Could you please check it again?

Thank you.
Flags: needinfo?(mats)
Yes, --disable-eme works now, thanks!
Flags: needinfo?(mats)
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.