1385115 - dom/media/gmp/GMPChild.cpp:438:33: error: 'XUL_LIB_FILE' was not declared in this scope

Status: RESOLVED FIXED

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

Description

[Jan Beich]

In file included from objdir/dom/media/gmp/Unified_cpp_dom_media_gmp0.cpp:2:
dom/media/gmp/ChromiumCDMAdapter.cpp:180:5: error: use of undeclared identifier 'close'
    close(mFile);
    ^
In file included from objdir/dom/media/gmp/Unified_cpp_dom_media_gmp0.cpp:65:
dom/media/gmp/GMPChild.cpp:438:33: error: use of undeclared identifier 'XUL_LIB_FILE'
      NS_SUCCEEDED(path->Append(XUL_LIB_FILE)) && FileExists(path) &&
                                ^
2 errors generated.

Comment 1

[Jan Beich]

The regression is caused by mozilla-central changeset eb898389180a. Can you adjust "Blocks" field?

Comment 2

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

(In reply to Jan Beich from comment #1)
> The regression is caused by mozilla-central changeset eb898389180a. Can you
> adjust "Blocks" field?

Certainly.

The code in eb898389180a won't have any practical effect on FreeBSD, so I'd be happy so to take a patch to ifdef it out on non Mac/Windows/Linux builds.

Comment 3

[Jan Beich]

(In reply to Chris Pearce (:cpearce) from comment #2)
> ifdef it out on non Mac/Windows/Linux builds

GMP itself (not just CDM) is a liability on Tier3 due to lack of sandboxing. Without knowing the context and being able to test I'd prefer to keep the statu quo. Adding --disable-gmp is preferable[1] but is also more work.

[1] Tor Browser can probably take advantage of it:
    https://trac.torproject.org/projects/tor/ticket/15910
    https://trac.torproject.org/projects/tor/ticket/16285

Comment 4

[Jan Beich]

Attachment: Bug 1385115 - Unbreak build on Tier3 after bug 1382883.

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

Comment 5

[Jan Beich]

Comment on attachment 8891315 [details]
Bug 1385115 - Unbreak build on Tier3 after bug 1382883.

https://reviewboard.mozilla.org/r/162522/#review167780

::: dom/media/gmp/GMPChild.cpp:351
(Diff revision 1)
>  #define FIREFOX_FILE NS_LITERAL_STRING("firefox")
>  #define XUL_LIB_FILE NS_LITERAL_STRING("XUL")
> -#elif defined(XP_LINUX)
> -#define FIREFOX_FILE NS_LITERAL_STRING("firefox")
> -#define XUL_LIB_FILE NS_LITERAL_STRING("libxul.so")
>  #elif defined(OS_WIN)

This is weird. `OS_*` come from Chromium. Gecko shouldn't use them outside of IPC and WebRTC.

Comment 6

[Jan Beich]

Do we support platforms where XP_WIN != OS_WIN or XP_UNIX != OS_POSIX?

Comment 7

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

Comment on attachment 8891315 [details]
Bug 1385115 - Unbreak build on Tier3 after bug 1382883.

https://reviewboard.mozilla.org/r/162522/#review168542

r+, provided you please update the patch to fix the OS_WIN/XP_WIN issue you pointed out. Thanks for the patch! Sorry for breaking tier3!

::: dom/media/gmp/GMPChild.cpp:351
(Diff revision 1)
>  #define FIREFOX_FILE NS_LITERAL_STRING("firefox")
>  #define XUL_LIB_FILE NS_LITERAL_STRING("XUL")
> -#elif defined(XP_LINUX)
> -#define FIREFOX_FILE NS_LITERAL_STRING("firefox")
> -#define XUL_LIB_FILE NS_LITERAL_STRING("libxul.so")
>  #elif defined(OS_WIN)

Yeah, that should be XP_WIN, and not OS_WIN. I must have copypasta'd that from somewhere.

Please change that to XP_WIN.

Comment 8

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

(In reply to Jan Beich from comment #6)
> Do we support platforms where XP_WIN != OS_WIN or XP_UNIX != OS_POSIX?

I don't believe so.

I recall now I had copied OS_POSIX from Chromium's content_decryption_module.h in a previous version of my patch in bug 1382883, and that didn't work, and now you've explained why. That explains how that got there.

Comment 9

[Jan Beich]

Comment on attachment 8891315 [details]
Bug 1385115 - Unbreak build on Tier3 after bug 1382883.

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

Comment 10

[Jan Beich]

Comment on attachment 8891315 [details]
Bug 1385115 - Unbreak build on Tier3 after bug 1382883.

https://reviewboard.mozilla.org/r/162522/#review168542

OK. Fixed but only `OS_WIN` coming from bug 1382883 to allow backporting to FF56 if this bug misses merge date. Other instances of `OS_*` to be fixed in a separate bug.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9408284f094a
Unbreak build on Tier3 after bug 1382883. r=cpearce

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/9408284f094a