Closed
Bug 1385115
Opened 7 years ago
Closed 7 years ago
dom/media/gmp/GMPChild.cpp:438:33: error: 'XUL_LIB_FILE' was not declared in this scope
Categories
(Core :: Audio/Video: GMP, defect, P1)
Tracking
()
RESOLVED
FIXED
mozilla56
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox54 | --- | unaffected |
firefox55 | --- | unaffected |
firefox56 | --- | fixed |
People
(Reporter: jbeich, Assigned: jbeich)
References
Details
(Keywords: regression)
Attachments
(1 file)
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.
The regression is caused by mozilla-central changeset eb898389180a. Can you adjust "Blocks" field?
Flags: needinfo?(cpearce)
Comment 2•7 years ago
|
||
(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.
Flags: needinfo?(cpearce)
Updated•7 years ago
|
Component: DOM → Audio/Video: GMP
Updated•7 years ago
|
Rank: 17
Priority: -- → P1
(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 hidden (mozreview-request) |
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.
Do we support platforms where XP_WIN != OS_WIN or XP_UNIX != OS_POSIX?
Flags: needinfo?(cpearce)
Comment 7•7 years ago
|
||
mozreview-review |
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.
Attachment #8891315 -
Flags: review?(cpearce) → review+
Comment 8•7 years ago
|
||
(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.
Flags: needinfo?(cpearce)
Comment hidden (mozreview-request) |
Assignee | ||
Comment 10•7 years ago
|
||
mozreview-review-reply |
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.
Keywords: checkin-needed
Comment 11•7 years ago
|
||
Pushed by cpearce@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/9408284f094a Unbreak build on Tier3 after bug 1382883. r=cpearce
Keywords: checkin-needed
Comment 12•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/9408284f094a
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Updated•7 years ago
|
Assignee: nobody → jbeich
status-firefox54:
--- → unaffected
status-firefox55:
--- → unaffected
status-firefox-esr52:
--- → unaffected
You need to log in
before you can comment on or make changes to this bug.
Description
•