--disable-webrtc builds now error out with "dom/ipc/PContent.ipdl:53: error: can't locate include file `PWebrtcGlobal.ipdl'"

RESOLVED FIXED in Firefox 40

Status

()

Core
WebRTC
RESOLVED FIXED
3 years ago
8 months ago

People

(Reporter: dholbert, Assigned: pkerr)

Tracking

Trunk
mozilla40
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox40 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

3 years ago
Bug 1100502 added "include protocol PWebrtcGlobal;" to dom/ipc/PContent.ipdl, and now I hit this when building with ac_add_options --disable-webrtc:
{
 0:21.86   in file included from `$SRC/accessible/ipc/PDocAccessible.ipdl', line 7:
 0:21.86 $SRC/dom/ipc/PContent.ipdl:53: error: can't locate include file `PWebrtcGlobal.ipdl'
 0:21.89 Traceback (most recent call last):
 0:21.89   File "$SRC/config/pythonpath.py", line 56, in <module>
 0:21.89     main(sys.argv[1:])
 0:21.89   File "$SRC/config/pythonpath.py", line 48, in main
 0:21.89     execfile(script, frozenglobals)
 0:21.89   File "$SRC/ipc/ipdl/ipdl.py", line 132, in <module>
 0:21.89     if not ipdl.typecheck(ast):
 0:21.89   File "$SRC/ipc/ipdl/ipdl/__init__.py", line 35, in typecheck
 0:21.89     return TypeCheck().check(ast, errout)
 0:21.89   File "$SRC/ipc/ipdl/ipdl/type.py", line 616, in check
 0:21.89     if not runpass(GatherDecls(builtinUsing, self.errors)):
 0:21.89   File "$SRC/ipc/ipdl/ipdl/type.py", line 608, in runpass
 0:21.89     tu.accept(tcheckpass)
 0:21.89   File "$SRC/ipc/ipdl/ipdl/ast.py", line 139, in accept
 0:21.89     return visit(self)
 0:21.89   File "$SRC/ipc/ipdl/ipdl/type.py", line 731, in visitTranslationUnit
 0:21.89     if inc.tu.filetype == 'header':
 0:21.89 AttributeError: 'NoneType' object has no attribute 'filetype'
 0:22.02 Makefile:24: recipe for target 'export' failed
 0:22.02 make[5]: *** [export] Error 1
}
(Reporter)

Updated

3 years ago
Flags: needinfo?(pkerr)
(Reporter)

Updated

3 years ago
Summary: --disable-webrtc builds now hit: dom/ipc/PContent.ipdl:53: error: can't locate include file `PWebrtcGlobal.ipdl' → --disable-webrtc builds now error out with "dom/ipc/PContent.ipdl:53: error: can't locate include file `PWebrtcGlobal.ipdl'"
(Assignee)

Comment 1

3 years ago
Taking a look at this bug with a view to keep the new IPC stuff out of the disable-webrtc builds.
Flags: needinfo?(pkerr)
(Assignee)

Updated

3 years ago
Assignee: nobody → pkerr
(Assignee)

Comment 2

3 years ago
Created attachment 8598928 [details] [diff] [review]
disable-webrtc builds fail after bug 1100502 applied
(Assignee)

Comment 3

3 years ago
I cannot drop an #ifdef in to an IPDL file. So, I moved around the necessary files that support the PWebrtcGlobal.ipdl to a branch of the source tree that was not disabled during a --disable-webrtc build. MOZ_WEBRTC is then used in the ContentParent.cpp and ContentChild.cpp to disable the creation of a WebRTC IPC pipeline.
(Assignee)

Comment 4

3 years ago
Are you able to provide a review for this patch?
Flags: needinfo?(dholbert)
(Reporter)

Comment 5

3 years ago
You should probably ask someone with media or WebRTC knowledge to review.  (Thanks for the quick action coming up with a fix!)
Flags: needinfo?(dholbert)
(Assignee)

Updated

3 years ago
Attachment #8598928 - Flags: review?(rjesup)
Comment on attachment 8598928 [details] [diff] [review]
disable-webrtc builds fail after bug 1100502 applied

Review of attachment 8598928 [details] [diff] [review]:
-----------------------------------------------------------------

moz.build changes need build-system peer
Attachment #8598928 - Flags: review?(rjesup)
Attachment #8598928 - Flags: review?(mh+mozilla)
Attachment #8598928 - Flags: review+
(Assignee)

Comment 7

3 years ago
Created attachment 8599390 [details] [diff] [review]
disable-webrtc builds fail after bug 1100502 applied

simplified version of first patch
(Assignee)

Updated

3 years ago
Attachment #8599390 - Flags: review?(mh+mozilla)
(Assignee)

Updated

3 years ago
Attachment #8598928 - Attachment is obsolete: true
Attachment #8598928 - Flags: review?(mh+mozilla)
Comment on attachment 8599390 [details] [diff] [review]
disable-webrtc builds fail after bug 1100502 applied

Review of attachment 8599390 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/ipc/ContentChild.cpp
@@ +120,5 @@
>  
>  #include "nsIScriptSecurityManager.h"
>  
> +#ifdef MOZ_WEBRTC
> +#include "mozilla/media/webrtc/WebrtcGlobalChild.h"

I don't get it. If you conditionally include this header and make the corresponding code conditional, why do you need to move the headers and ipdl?
Attachment #8599390 - Flags: review?(mh+mozilla)
(Assignee)

Comment 9

3 years ago
When building with -disable-webrtc, the media/webrtc/moz.build is not processed. Any EXPORT or IPDL statement found in that file will not be executed. So, the PWebrtcGlobal.ipdl is not flagged and the files expected when the "include protocol PWebrtcGlobal" that cannot be ifdef'ed out of PContent.ipdl will not be created. This causes the problem noted in the bug. I fixed that by moving the IPDL declaration into the dom/media/webrtc branch which holds components that are shared between media and webrtc. (I moved the PWebrtcGlobal.ipdl file simply to have it reside in the same branch where the moz.build managed.)

This fixed the first problem but raised the second: the file WebrtcGlobal.h is imported directly into PWebrtcGlobal.ipdl to provide the data serialization definitions. I moved its EXPORT statement to dom/media/moz.build along with moving the file to dom/media/webrtc. This allowed the IPDL tools to generated the necessary files to fix the error with further propagation stopped by the #ifdef MOZ_WEBRTC lines added to the PContentParent.cpp and PContentChild.cpp files.

Is there a way to have part of the media/webrtc/moz.build file processed when -disable-webrtc is set?
(Assignee)

Comment 11

3 years ago
see comment 9.
Flags: needinfo?(mh+mozilla)
Comment on attachment 8599390 [details] [diff] [review]
disable-webrtc builds fail after bug 1100502 applied

Review of attachment 8599390 [details] [diff] [review]:
-----------------------------------------------------------------

Considering comment 9, I guess this is reasonable, although some webrtc peer should probably take a look. That being said, I don't understand why ipdl aren't preprocessed, which would make this so much easier...
Attachment #8599390 - Flags: review?(rjesup)
Flags: needinfo?(mh+mozilla)
(Assignee)

Comment 13

3 years ago
Created attachment 8601844 [details] [diff] [review]
disable-webrtc builds fail after bug 1100502 applied
(Assignee)

Updated

3 years ago
Status: NEW → ASSIGNED
(Assignee)

Updated

3 years ago
Attachment #8599390 - Attachment is obsolete: true
Attachment #8599390 - Flags: review?(rjesup)
(Assignee)

Comment 14

3 years ago
Comment on attachment 8601844 [details] [diff] [review]
disable-webrtc builds fail after bug 1100502 applied

Carry forward rjesup r+
Attachment #8601844 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/0d384c358563
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox40: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
Depends on: 1274518

Updated

8 months ago
Blocks: 1408716

Updated

8 months ago
No longer blocks: 1408716
You need to log in before you can comment on or make changes to this bug.