Closed Bug 914826 Opened 6 years ago Closed 6 years ago

clean up generated protocol headers a little bit

Categories

(Core :: IPC, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla27

People

(Reporter: froydnj, Unassigned)

Details

Attachments

(6 files, 2 obsolete files)

800 bytes, patch
ehsan
: review+
Details | Diff | Splinter Review
2.33 KB, patch
ehsan
: review+
Details | Diff | Splinter Review
1.52 KB, patch
ehsan
: review+
Details | Diff | Splinter Review
737 bytes, patch
ehsan
: review+
Details | Diff | Splinter Review
2.54 KB, patch
ehsan
: review+
Details | Diff | Splinter Review
10.26 KB, patch
froydnj
: review+
Details | Diff | Splinter Review
No description provided.
COMPILE_ASSERT was chromium's static_assert.  Use the new hotness.
Attachment #802566 - Flags: review?(ehsan)
We're going to add includes for only cpp files in later patches.
Attachment #802568 - Flags: review?(bent.mozilla)
No need to include nsIFile in headers just so we can declare things for parent
processes.  The condition is copied from the condition guarding the declaration
of GetMinidump further down; I wanted to factor out the condition, but the
latter condition also guards other things, so I left as-is.
Attachment #802570 - Flags: review?(bent.mozilla)
These files aren't actually needed in the generated headers; let's just include
them in the source files.
Attachment #802572 - Flags: review?(bent.mozilla)
Various bits of Gecko bootleg do_CreateInstance and do_GetService from the ipdl headers.
Make them stop so we can sanely refactor the generated headers.

I see that this patch doesn't fully compile on x86-64 Linux, though it does on Android.
I'd like to request a blanket rs+ on similar changes to these that are required to fix
up compile errors I discover when pushing this to try or similar.
We don't use anything from these headers in the generated headers.
Attachment #802574 - Flags: review?(bent.mozilla)
Comment on attachment 802573 [details] [diff] [review]
part 5 - fix source files that were bootlegging XPCOM do_* functions via generated ipdl headers

Please see patch comment or comment 5.
Attachment #802573 - Flags: review?(ehsan)
Comment on attachment 802566 [details] [diff] [review]
part 1 - use static_assert instead of COMPILE_ASSERT in IPCMessageStart.h

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

This is hot stuff!  ;-)
Attachment #802566 - Flags: review?(ehsan) → review+
Comment on attachment 802573 [details] [diff] [review]
part 5 - fix source files that were bootlegging XPCOM do_* functions via generated ipdl headers

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

r+rs=me
Attachment #802573 - Flags: review?(ehsan) → review+
Attachment #802568 - Flags: review?(bent.mozilla) → review+
Attachment #802570 - Flags: review?(bent.mozilla) → review+
Comment on attachment 802572 [details] [diff] [review]
part 4 - provide for cpp-only include files, starting with nsIFile.h and GeckoProfiler.h

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

::: ipc/ipdl/ipdl/lower.py
@@ +2575,4 @@
>          cf.addthings((
>              [ Whitespace.NL ]
> +            + cppheaders
> +            + [ Whitespace.NL ]

Please #include these at the very end of the #include list, to make sure that they won't declare names used by the other headers later in the list.
Attachment #802572 - Flags: review?(bent.mozilla) → review-
Comment on attachment 802574 [details] [diff] [review]
part 6 - don't include basictypes.h or nscore.h in ipdl headers

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

I've seen basictypes.h provide PrintfIncludeMacros.h stuff which is sometimes used in b2g specific code.  r=me if this builds.  :-)
Attachment #802574 - Flags: review?(bent.mozilla) → review+
OK, let's put them at the end, then!
Attachment #802572 - Attachment is obsolete: true
Attachment #803682 - Flags: review?(ehsan)
(In reply to :Ehsan Akhgari (needinfo? me!) from comment #11)
> I've seen basictypes.h provide PrintfIncludeMacros.h stuff which is
> sometimes used in b2g specific code.  r=me if this builds.  :-)

Should start using whatever's in mfbt...but it builds:

https://tbpl.mozilla.org/?tree=Try&rev=c6debde04d98
Attachment #803682 - Flags: review?(ehsan) → review+
Blocks: 922334
This broke YouTube playback on Firefox OS on trunk, which breaks a daily smoketest. See bug 922334 a followup.
No longer blocks: 922334
You need to log in before you can comment on or make changes to this bug.