Merge paths in GMP IPC code the handle node ID as strings and structs
Categories
(Core :: Audio/Video: GMP, enhancement, P3)
Tracking
()
Tracking | Status | |
---|---|---|
firefox86 | --- | fixed |
People
(Reporter: bryce, Assigned: bryce)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
Bug 1651239 merged some of our code that handles node IDs in the GMP to reduce duplication and the risk of changing one path but not the other.
We can merge some of the other parts around the IPC boundary to help reduce duplication and make the code more concise and (hopefully) understandable.
Background: Node IDs are used when fetching GMPs such that GMPs with the same node ID can be shared (note, there's some other conditions the need also be met[0]). We currently have 2 ways of storing node IDs
- As a string. If we know the node ID early on in GMP creation we can immediately store it in a string and not mutate it further.
- As parts that will later be converted to a final string[1]. For EME we want to ensure GMPs are not shared between origins, so we pass around the GMP name + the top level origin and the origin of the media element involved. We later turn this into a string that will be used as the final node ID[1].
The code for handling these 2 cases is largely similar around IPC, but for the second case when the node ID reaches the GMPServiceParent we need to ensure it's processed into a string.
I think we can unify and trim some code here. I'm not planning on making any functional changes, but aiming to reduce duplication as well as consolidating some of our data types so it's easier to find where they're being used in tools like searchfox (e.g. right now we have 2 NodeId structs NodeId
and NodeIdData
which are essentially the same thing).
[0] https://searchfox.org/mozilla-central/rev/9c72508fcf2bba709a5b5b9eae9da35e0c707baa/dom/media/gmp/GMPServiceParent.cpp#711,722-724,727
[1] https://searchfox.org/mozilla-central/rev/9c72508fcf2bba709a5b5b9eae9da35e0c707baa/dom/media/gmp/GMPServiceParent.cpp#1065
Assignee | ||
Comment 1•4 years ago
|
||
Passing a union here allows us to reuse code and trim some code which was
duplicated to handle the different NodeId formats. This also consolidates the
former NodeId
and NodeIdData
structures into a new structure (working name
NodeIdParts
) which represents parts that will later be converted to a string
based NodeId.
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Updated•4 years ago
|
Comment 2•4 years ago
|
||
There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:bryce, could you have a look please?
For more information, please visit auto_nag documentation.
Assignee | ||
Comment 3•4 years ago
|
||
I've gotten bogged down in a few other things, but rest assured, release management bot, I have not forgotten about this.
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Updated•4 years ago
|
Comment 5•4 years ago
|
||
bugherder |
Description
•