Closed Bug 1265272 Opened 5 years ago Closed 5 years ago

[EME] Update to generate voucher for MacOSX binaries


(Release Engineering :: General, defect)

Not set


(firefox49 fixed)

Tracking Status
firefox49 --- fixed


(Reporter: cpearce, Assigned: mshal)


(Blocks 1 open bug)



(4 files)

In order to support Adobe Primetime EME on MacOSX, we need [1] to handle generating vouchers for our MacOSX plugin-container binary.

We also need to ensure we run the voucher generation tool on MacOSX, and that the voucher is actually packaged on MacOSX.

See Also bug 1091668 which added the for Windows.

Uploading this patch on behalf of Adobe...

Update to handle MacOSX binaries.

Note: this requires the macholib package to be installed.
Attachment #8742166 - Flags: review?(ted)
Michael: You did the initial work in bug 1091668 to make the script generate a voucher on Windows, can you do the same thing here for MacOSX? The MacOSX version uses the macholib package, so we'll need that to ensure we install that too.
Flags: needinfo?(mshal)
Sure, I can take a look. Hopefully it's easier than the Windows one was :).

Do we know for sure that macholib works cross-platform? Or do we need a per-platform version of

Where should the voucher.bin file end up in OSX? $(STAGEPATH)$(MOZ_PKG_DIR)? Or do we need it in under $(_RESPATH)?
Assignee: nobody → mshal
Flags: needinfo?(mshal) → needinfo?(cpearce)
(In reply to Michael Shal [:mshal] from comment #3)
> Sure, I can take a look. Hopefully it's easier than the Windows one was :).
> Do we know for sure that macholib works cross-platform? Or do we need a
> per-platform version of

I was able to install macholib and run this script on my Windows machine. So I don't think we'll need per-platform versions of this script, but we will need to install macholib on our Windows machines for the script to run, even though it shouldn't hit the macholib path.
> Where should the voucher.bin file end up in OSX? $(STAGEPATH)$(MOZ_PKG_DIR)?
> Or do we need it in under $(_RESPATH)?

Gecko expects to find the plugin-container.voucher file in NS_GRE_DIR, which the code says "On OSX, this typically points to Contents/Resources in the app bundle." Ah... so maybe that is $(_RESPATH)?
Flags: needinfo?(cpearce)
Are you able to check if this try build puts the voucher in the right place?

The staged plugin-container actually goes into a somewhat funky location on OSX, which in make-land corresponds to:


Where STAGEPATH can be "universal" or blank, MOZ_PKG_DIR is "firefox", _BINPATH is something like, and MOZ_CHILD_PROCESS_NAME is plugin_container.

I put the voucher.bin in the same location as the plugin-container binary, which is what we do on Windows. If it turns out we do need the voucher in _RESPATH rather than with the binary, just let me know.
Flags: needinfo?(cpearce)
If voucher.bin is a binary file, it should live under Contents/MacOS. Any other files should live under Contents/Resources. Based on comment 5, it appears that this is put in the right place.
Flags: needinfo?(cpearce)
spohl: the voucher file isn't an executable, it's a binary data file. Should binary data files be in Resources or in Contents/MacOS?
Flags: needinfo?(spohl.mozilla.bugs)
If it's not an executable you're correct that it should live under Contents/Resources. The Apple docs don't call out binary data files specifically, but they do say that Contents/MacOS typically only contains the main executable and sometimes other, standalone executables[1].

I believe both locations would still be accepted by codesign, but since this is a new file we may as well put it in the right place from the start, i.e. Contents/Resources.

Flags: needinfo?(spohl.mozilla.bugs)
mshal: The consensus is that the voucher should go in Content/Resources. Please make it so.
Flags: needinfo?(mshal)
Sounds good. Can you test out this build with voucher.bin in Contents/Resources?

If that looks good I can publish it for review.
Flags: needinfo?(mshal) → needinfo?(cpearce)
That looks good to me.
Flags: needinfo?(cpearce)
Review commit:
See other reviews:
Attachment #8745999 - Flags: review?(ted)
Attachment #8745999 - Flags: review?(gerv)
Attachment #8746000 - Flags: review?(ted)
Attachment #8746000 - Flags: review?(gerv)
Attachment #8746001 - Flags: review?(ted)
r?gerv for the licensing of macholib and altgraph. Both should be MIT.

r?ted for the implementation bits.
Attachment #8745999 - Flags: review?(gerv) → review+
Attachment #8746000 - Flags: review?(gerv) → review+
Comment on attachment 8745999 [details]
MozReview Request: Bug 1265272 - Import macholib 1.7; r?ted,gerv
Attachment #8745999 - Flags: review?(ted) → review+
Comment on attachment 8746000 [details]
MozReview Request: Bug 1265272 - Import altgraph 0.12; r?ted,gerv
Attachment #8746000 - Flags: review?(ted) → review+
Comment on attachment 8746001 [details]
MozReview Request: Bug 1265272 - Generate EME voucher for MacOSX; r?ted

Terrible, but not really worse than what's already there.
Attachment #8746001 - Flags: review?(ted) → review+
Comment on attachment 8742166 [details] [diff] [review]
Patch: Update to handle MacOSX binaries

Review of attachment 8742166 [details] [diff] [review]:

Some nit-picky stuff, but as usual I'm not terribly invested in making this code match existing Mozilla standards.

::: python/eme/
@@ +168,5 @@
> +FAT_CIGAM = 0xbebafeca
> +FAT_MAGIC =	0xcafebabe
> +
> +LC_SEGMENT = 0x1
> +LC_SEGMENT_64	= 0x19	# 64-bit segment of this file to be

All of these constants are defined in the `macholib.mach_o` module:
>>> import macholib.mach_o
>>> hex(macholib.mach_o.MH_MAGIC)

Just use those instead of redefining them.

@@ +432,5 @@
> +
> +		arch_digest.setComponentByName('cpuType', CPUType(header.header.cputype))
> +		arch_digest.setComponentByName('cpuSubType', CPUSubType(header.header.cpusubtype))
> +
> +		if header.header.cputype == 0x1000007:

If there isn't a constant defined in macholib somewhere it'd be good to define one for this.

@@ +438,5 @@
> +
> +
> +
> +		segment_commands = list(filter(lambda x: x[0].cmd == lc_segment, header.commands))
> +		text_segment_commands = list(filter(lambda x: x[1].segname.decode("utf-8").startswith("__TEXT"), segment_commands))

I would probably write these as list comprehensions, but I'm not that hung up on it:
segment_commands = [x for x in header.commands if x[0].cmd == lc_segment]

@@ +454,5 @@
> +			set_of_digest = SetOfCodeSectionDigest()
> +			for section in text_command[2]:
> +				digester = hashlib.sha256()
> +				digester.update(section.section_data)
> +				digest = digester.digest()

You can make this one line: `hashlib.sha256(section.section_data).digest()`

@@ +581,5 @@
> +
> +	dict = processCOFFBinary(stream)
> +
> +	if dict['result'] == False:
> +		dict = processMachoBinary(app_args.input)

It might be nicer if you had an argument to pass in the expected binary type, and default to the right thing depending on sys.platform.
Attachment #8742166 - Flags: review?(ted) → review+
Are these things we can or should update in our tree? Or do we need to send the feedback in #c19 to Adobe?
Flags: needinfo?(cpearce)
From the Adobe perspective -- feel free to make the changes. We can also make the changes, but it will take longer.
The main benefit of review here is ensuring the script does what we expect it to do. I think we don't need to be too hung up on making it match our coding style. I think we could just land it as is.
Flags: needinfo?(cpearce)
Have you had a chance to see if the voucher is working properly here? It looks like it is present in nightly builds, but I wanted to make sure it was usable.
Flags: needinfo?(cpearce)
It looks like the voucher is on disk as expected, and being found by Firefox. Thanks!
Flags: needinfo?(cpearce)
Component: General Automation → General
You need to log in before you can comment on or make changes to this bug.