Closed Bug 1598968 Opened 5 months ago Closed 4 months ago

MediaControlUtils.h contains headers and symbols, leading to a failed linking process when used elsewhere

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla72
Tracking Status
firefox72 --- fixed

People

(Reporter: MeFisto94, Assigned: emilio)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

So, as suggested on https://phabricator.services.mozilla.com/D47999#1575254 and later comments, I wanted to use the gMediaControlLog.

This however didn't work as MediaControlUtils.h didn't only contain Declarations but also Definitions. Thinking this is an oversight, I was preparing to move gMediaControlLog into MediaController, but the build failed again due to the methods in said file.

Is there any particular reason this Header-File Contains code? Or should this file be renamed to .cpp and should an appropriate header be added?

Flags: needinfo?(alwu)

Could you try to rebase your patches onto the latest codebase? I believe this issue have been fixed, because we have removed the use of MediaUtils from the outer window.

Flags: needinfo?(alwu) → needinfo?(marc.streckfuss)

That still doesn't change the fact that MediaControlUtils.h cannot be included in two different cpp files, right?

Flags: needinfo?(alwu)

No, it can [1], but I don't understand why including it on the outer window would cause that issue, maybe involving with compling order?

[1] https://searchfox.org/mozilla-central/search?q=MediaControlUtils.h&path=

Flags: needinfo?(alwu)

No, the issue is that those files get pulled into the same translation unit, so it doesn't get included multiple times. If you include it anywhere else in the project, it will file, as it defines the logger and other functions that are not marked as inline.

Does that make sense? :)

Flags: needinfo?(alwu)

Well I'll just fix this, hopefully the patch makes the problem obvious.

Assignee: marc.streckfuss → emilio
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(marc.streckfuss)
Flags: needinfo?(alwu)
Priority: -- → P3
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3886adb1a5e2
Make MediaControlUtils a proper header. r=MeFisto94,alwu
Status: NEW → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla72
You need to log in before you can comment on or make changes to this bug.