1386200 - Drop OS_* defines under dom/media/

Status: NEW

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

Description

[Jan Beich]

Gecko provides XP_<PLATFORM> while Blink provides OS_<PLATFORM>. While Gecko can also uses the latter it's both discouraged and can be confusing if both styles are used in the same file.

Comment 1

[Jan Beich]

Attachment: Bug 1386200 - Don't use Chromium platform macros under dom/media/.

Review commit: https://reviewboard.mozilla.org/r/163360/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/163360/

Comment 2

[Jan Beich]

Comment on attachment 8892413 [details]
Bug 1386200 - Don't use Chromium platform macros under dom/media/.

https://reviewboard.mozilla.org/r/163360/#review168732

::: dom/media/gmp/GMPProcessChild.cpp
(Diff revision 1)
> -#elif defined(OS_WIN)
> -  std::vector<std::wstring> values = CommandLine::ForCurrentProcess()->GetLooseValues();
> -  MOZ_ASSERT(values.size() >= 1, "not enough loose args");
> -  pluginFilename = nsDependentString(values[0].c_str());
> -#else
> -#error Not implemented

After bug 969757 everything but Windows is Unix. No need for an exilicit #error what isn't supported even as Tier3 platform.

http://searchfox.org/mozilla-central/rev/09c065976fd4/build/moz.configure/init.configure#683
http://searchfox.org/mozilla-central/rev/09c065976fd4/ipc/chromium/chromium-config.mozbuild#38

Comment 3

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8892413 [details]
Bug 1386200 - Don't use Chromium platform macros under dom/media/.

https://reviewboard.mozilla.org/r/163360/#review169040

Thanks!

Comment 4

[Emma Humphries ☕️🎸🧞‍♀️✨ (she/they) [:emceeaich] (Pacific Time) use needinfo]

This is a P1 bug without an assignee. 

P1 are bugs which are being worked on for the current release cycle/iteration/sprint. 

If the bug is not assigned by Monday, 28 August, the bug's priority will be reset to '--'.

Comment 5

[Bulk Bug Changes for mreavy's org]

Mass change P1->P2 to align with new Mozilla triage process

Comment 6

[Jan Beich]

Comment on attachment 8892413 [details]
Bug 1386200 - Don't use Chromium platform macros under dom/media/.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/163360/diff/1-2/

Comment 7

[Jan Beich]

Reduced churn a bit per bug 1386207 comment 4.

Comment 8

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8892413 [details]
Bug 1386200 - Don't use Chromium platform macros under dom/media/.

https://reviewboard.mozilla.org/r/163360/#review208308

Comment 9

[James Cheng[:JamesCheng]]

Hi Jan,

Do you want to land this r+ed patch?

Just want to cleanup the P2 bugs and want to know the status.

Thanks