The lifetime of async IPDL returned promise may be longer than its actor.
That is, the handler (receiver) may have not resolve/reject the promise when the actor
is destroyed. In this case, we have to reject all the pending promises before
ActorDestroy() is called on the "sender" side.
Besides, the handler (receiver) can reject with reason "ActorDestroyed" to silently
cancel the promise without trying to reply to the remote actor which may
have died. The sender-side promise is responsible for rejecting the pending promises,
which will be done in MessageChannel::RejectPendingPromisesForActor().
MozReview-Commit-ID: 4XjmquZzDBO
--HG--
extra : rebase_source : 48539e35e4587e09be1d66497b1ea32d1a95ee9a
PLayerTransaction's constructor was previously synchronous so we could
return a TextureFactoryIdentifier. This is quite reliably available
already in the case of opening a tab, due to RenderFrameParent knowing
which compositor it is attached to, so we can make the constructor
asynchronous.
In the top-level widget case, we add a new synchronous message to find
the TextureFactoryIdentifier.
PLayerTransaction's constructor was previously synchronous so we could
return a TextureFactoryIdentifier. This is quite reliably available
already in the case of opening a tab, due to RenderFrameParent knowing
which compositor it is attached to, so we can make the constructor
asynchronous.
In the top-level widget case, we add a new synchronous message to find
the TextureFactoryIdentifier.
Remove sync protocol AllocateTabId. Instead we generate tabId in
each process with nsContentUtils::GenerateTabId, and register
RemoteFrameInfo in parent process. If the tab id was generated from
a content process, it's sent parent through either PBrowserConstructor
or PContent::CreateChildProcess.
MozReview-Commit-ID: D3W2fK9eCNH
--HG--
extra : rebase_source : 1913f8f586537be1c82a70a19cc8c6351671d0df
This patch implements async returns for IPDL using MozPromises. There
are following changes:
* Initialize AbstractThreads for MessageLoops
* Record promises and their reject functions
* When async message returns, call their resolve functions
* When send error or channel close, call their reject functions
* Implement "unresolved-ipc-promises" count for about:memory
* Test cases
See bug attachment for generated code from test cases
MozReview-Commit-ID: 7xmg8gwDGaW
--HG--
rename : ipc/ipdl/test/ipdl/error/AsyncReturn.ipdl => ipc/ipdl/test/ipdl/ok/AsyncReturn.ipdl
extra : rebase_source : 9a5821d6c0e5f7152b8152a17a409b94e8258dc3
This patch implements async returns for IPDL using MozPromises. There
are following changes:
* Initialize AbstractThreads for MessageLoops
* Record promises and their reject functions
* When async message returns, call their resolve functions
* When send error or channel close, call their reject functions
* Implement "unresolved-ipc-promises" count for about:memory
* Test cases
See bug attachment for generated code from test cases
MozReview-Commit-ID: 7xmg8gwDGaW
--HG--
rename : ipc/ipdl/test/ipdl/error/AsyncReturn.ipdl => ipc/ipdl/test/ipdl/ok/AsyncReturn.ipdl
extra : rebase_source : 55c4f68a3f8b7d0df5ca9f9c45b9a205b337282d
NPN_GetAuthenticationInfo is an NPAPI API we implemented for Java and since we only support Flash we no longer need it.
MozReview-Commit-ID: HxNn91QeaMB
--HG--
extra : rebase_source : 016320ce93bde525dbf1b74f50f8b60d141d31cd
This is the most important part of the patch series. It removes the
PScreenManager protocol and use ScreenManager directly in the content
processes.
Initial and subsequent updates are sent via PContent::RefreshScreens.
struct ScreenDetails are kept to serialize Screen over IPC.
nsIScreenManager::ScreenForNativeWidget is removed because
nsIWidget::GetWidgetScreen can replace it. nsIScreen::GetId is removed
because it's not useful for the more general Screen class.
MozReview-Commit-ID: 5dJO3isgBuQ
--HG--
extra : rebase_source : 06aa4e4fd56e2b2af1e7483aee7c0cc7f35bdb97
This merges two existing off-main-thread sync IPCs into a single operation. We
will change them into a single async operation in a follow up.
MozReview-Commit-ID: EfMozbRysGR
--HG--
extra : rebase_source : c7f5c395a719b9f3f13d398f8ca976b09f25ce49
- There appears to be no issues with simply changing
SetHaveEventListener from sync to async.
MozReview-Commit-ID: 3LKgDx9AZnm
--HG--
extra : rebase_source : 6c706f592f71a8c967a58f6906861fcff2525ebf
- PVRManager::GetDisplays was a sync IPC that was part of an optimization
for a use case that has been eliminated by changes to the WebVR spec.
- This was an optimization for Navigator.activeVRDisplays that would allow
enumeration of displays active in any content process without powering
on any additional VR hardware. This will no longer be necessary as the
activeVRDisplays has been restricted to returning only the displays
active in the current javascript context.
MozReview-Commit-ID: F6sOtM9nups
--HG--
extra : rebase_source : bd8967fab9677206d998eea922c8d1640551de1c
An IPDL unit test that is intended to fail should check that the
reason the test fails matches the expected reason for failure. We have
had a number of cases where some change, like renaming a keyword,
causes tests to start failing for the wrong reason, which means they
are no longer testing anything useful.
To support this, each file in error/ must contain one or more error
annotations. An error annotation is a line starting with "//error:",
followed by whatever the rest of the expected error is. For every one
of these annotations that a file has, the stderr output of compiling
the test must contain the specified string, including the "error:". It
is also an error for an error/ file to not contain an error
annotation.
To generate the initial set of annotations, I just copied and pasted
the error that each test produced. I did some light auditing to check
that the errors are reasonable, which did turn up one minor error
which I fixed as part of bug 1347527.
This patch does not check that every error produced by compiling the
file is in the list of expected errors. I think that's less of a
problem if it does occur.
MozReview-Commit-ID: BrePLGPPRil
--HG--
extra : rebase_source : 0ddb2f866c4b4ab74b7e975ce5877568c8cc3b62
- Eliminated the VRDisplay.GetImmediateSensorState sync call
and associated code as it is no longer needed.
MozReview-Commit-ID: 7BsCKC9EbsY
--HG--
extra : rebase_source : ae2de369d156e397d919d83b6c63b10374953bae
Adding this unused message prevents a compiler warning
about the private field mState being unused.
Also, get rid of some trailing whitespace.
MozReview-Commit-ID: Lb43JQhIbJU
--HG--
extra : rebase_source : c76eb5383a1535c79f2a66d3d6f8454e5b61d945
Using a separate error function will distinguish mismatched sentinels
from other errors, such as array length problems.
MozReview-Commit-ID: Gl8swNhqLns
--HG--
extra : rebase_source : 494759b105086c6c26f3ac53fb644bbf51c12800
Instead of initializing DataStorage objects on demand in the content
process, we initialize them at content process startup by getting the
parent to send down the information about the existing DataStorages at
child process startup. After that point, the dynamic change
notifications added in bug 1215723 will take care of keeping the
information in sync.
The parser always sets the first value of the tuple .nestedRange to
NOT_NESTED, so there's no need to actually store it. Instead, we
create a range when we're creating the ProtocolType. This makes it
clearer what is happening. The range is needed for the type because
the nesting pair is compared with those from messages, where the first
element can be something else.
Prior to bug 1306708, the lower range could be specified in the IPDL
file, but all uses were NOT_NESTED, so I suppose that is why it was
eliminated.
Note that the constructor for Protocol sets .nested to NOT_NESTED, but
prior to my patch, the field was never used. The constructor also
never initialized .nestedRange, but the parser always sets it so that
isn't much of an issue.
MozReview-Commit-ID: FMnoZRrkfoA
--HG--
extra : rebase_source : 3adb64b27d4a7a4e9c6c7695b145136813dbed5d
See MutRecHeader1.ipdlh for a more detailed explanation.
MozReview-Commit-ID: JHYd7qKSjrr
--HG--
extra : rebase_source : 2d405b3cb4384d6c815fe1634216682fd828f930
type is some existing Python object, so this assertion can never fail.
Looking at GatherDecls::visitMessageDecl, I think this is supposed to
be checking cdecl.
MozReview-Commit-ID: 8Oppw5SYdWO
--HG--
extra : rebase_source : d2e5d6c4f22a490e14f668a1f6ed28738dc56ca8
Instead, just stick a "visited" property on them. The symtab property
on nodes isn't used for any other purpose.
The next patch will eliminate the unused args to enterScope and
exitScope.
MozReview-Commit-ID: 3WW2NPVB0gM
--HG--
extra : rebase_source : 2c2add652e1156a397a155c74015671877af3690
This code gets deleted in the next patch, but the point here is that
we can still compile Firefox with this assertion in place.
MozReview-Commit-ID: 33dw0xm7L4R
--HG--
extra : rebase_source : 140fca3b0b5abf5b3d6636ea63bae2adf1311b09
It is always initially set to None, and the other subclass of
TcheckVisitor (CheckTypes) never uses it.
MozReview-Commit-ID: CKxXoHXopqF
--HG--
extra : rebase_source : 2422e31fb99f894948da01a0bcd7d5a2bf92ceb7
Actor is used in the process checking code, which will get deleted in
a separate bug. Instead, just convert the actor name + side to a
string manually.
MozReview-Commit-ID: 9zfD4MuscVw
--HG--
extra : rebase_source : 95037d1db717618ebb34ec44832e9d2498815ebd
This just wraps all the XRE method calls to go through the Bootstrap API
instead of relying on the XPCOM glue methods.
--HG--
extra : rebase_source : eccbe18b9b21ca1ab6c403515ffd60f0a9174d9c
This is gnarly IPDL code, but the generated code is probably easier to
review. Before when sending a sync message, we had:
bool sendok__ = (GetIPCChannel())->Send(msg__, (&(reply__)));
if ((!(sendok__))) {
return false;
}
Now, we have:
bool sendok__;
{
GeckoProfilerTracingRAII syncIPCTracer(
"IPC",
"PJavaScript::Msg_PreventExtensions");
sendok__ = (GetIPCChannel())->Send(msg__, (&(reply__)));
}
if ((!(sendok__))) {
return false;
}
The bug referred to in the comment was fixed 6 years ago.
The includes in cgen.py are unused.
--HG--
extra : rebase_source : f4521112ee3f2280836ac7030b25ddcba2321eda
This patch removes all user-specified state machine support from the
IPDL parser, then deletes all of the code related to it from the AST,
type checking, and code generation. The default state machine code
relating to tracking whether the protocol is dead or not will still be
generated. In fact, this patch should not change the code that is
generated for any protocol that does not use the custom state machine
syntax.
MozReview-Commit-ID: 1fABHR3zJx
--HG--
extra : rebase_source : a2c6655e767741eb7d697e55548c2409a4bdff12
The state machine stuff provides a decent compact overview of where
races are supposed to happen, so I just commented them out.
MozReview-Commit-ID: 1K5mw2kyXWb
--HG--
extra : rebase_source : db3fea5e80c47e04c103b1231077bd9b2c62c4d4
With the removal of state machines, the state() method is no longer
supported, so these assertions must be removed.
MozReview-Commit-ID: 4HV8cQqowlp
--HG--
extra : rebase_source : 512c740e7a7de8904b237d745cddea82dc82a603
The uses of state() in TestHangs and TestStackHooks only have two
states, depending on how many times the method has been called.
Two uses of state() have to be fixed in TestLatency. Each waits for 5
messages, then resets the state and sends replies.
MozReview-Commit-ID: 7Glj7wbl1ni
--HG--
extra : rebase_source : 55a88a9b31b2effc8af5629262a5f4d34987ba40
The parent process crashes if it gets a bad message from the child,
but that makes it hard to test. This patch overrides the fatal error
handling method and uses the old behavior, that kills the child.
I copied the code to kill the child from TestHangs.
MozReview-Commit-ID: 3YgqaCgHGI0
--HG--
extra : rebase_source : cbecee2742014e969c641b89833cff5f46b99a33
Bill said it is okay to declare interrupt parent-to-child messages now.
MozReview-Commit-ID: 5Ma6pfkUZmt
--HG--
extra : rebase_source : 68fd3f51a9154136003871425762816593d66139
managerNoCtor.ipdl: Add a message so that parsing doesn't just fail
immediately with a syntax error.
shmem_access_union.ipdl: Remove this test, which involves the
semi-removed ACL feature for shmem.
oldIncludesyntax.ipdl: To be safe, make the protocol more valid by
adding a message.
multimanNonexistentMgrs.ipdl: Make the name of the protocol match the
name of the file.
MozReview-Commit-ID: 9zx5fmAWIIc
--HG--
extra : rebase_source : a5b9b9f19bc0329a06ca4ff8be3ef4b879054dd9
Change message annotations from "rpc" to "intr" to match the current
name, and similarly rename the protocols and the files.
MozReview-Commit-ID: Dd9ikvAHMnV
--HG--
extra : rebase_source : 0b2d57ca2c4405319f4ecd6ba2f633128355b381
If qname has no quals, then fullname would be None, which breaks the
string concatenation in parentEndpointDecl and childEndpointDecl, even
if no endpoints are declared. This patch uses the short name if there
are no quals, while preserving the behavior that we want to pass None
into declare for fullname.
MozReview-Commit-ID: 9nuO8GWhBRH
--HG--
extra : rebase_source : df7e8b80d06b5cf1d4905624c0a3c4eac6703612
This patch fixes multipleUsingCxxTypes.ipdl. This is a regression from
bug 918651.
MozReview-Commit-ID: 3ByBvp6FZUe
--HG--
extra : rebase_source : 8038d8662360a8f12e571716eb7c59d2b3754508
As of bug 1240871 these are no longer optional.
MozReview-Commit-ID: 2r2uxJP9dDr
--HG--
extra : rebase_source : 6ca55ae336a7c7d37764b657333e331f3b6158c9
The IPDL parser handles include statements by using a stack of
parsers. If an inner parser encounters a parsing error, it will print
out an error message, which is maybe okay, but then it makes two
mistakes:
1. It does not pop the current parser off of the parser stack. This
means that the file that included the syntactically invalid file will
be parsed as though it were the invalid file. In bkelly's case, an
.ipdl file included an invalid .ipdlh file, so he got a bizarre error
message about how you can't define a protocol in a header, because the
parser was treating the protocol in the .ipdl file as though it were
in the .ipdlh file. I fixed this by using a "finally" clause to pop
the parser stack, ensuring that it is correct even in case of error.
2. A parse error in the include should cause the entire parse to fail,
but instead it will keep going. inc.tu will get set to None, which
eventually causes an error later in type checking, when it attempts to
examine inc.tu. I fixed this by only catching the parse error where we
invoke the outermost parser. This has the drawback that later errors
in other files will not be reported. An alternate fix would set a
global flag to indicate that a parse error had occured, and somehow
report that to the caller.
I think this bug was introduced in 2009 by commit
cb8189926a69872c73508fba50830f0d07af341f.
MozReview-Commit-ID: DhbDUO7MXGB
--HG--
extra : rebase_source : cee371cd54ebf575f78aa8b2441afbde8b3c2b8f
The other fields, spec, array, and nullable, are set in some places.
Also remove a dead chunk of code with a FIXME that refers to a bug
that was WONTFIXed in 2010.
--HG--
extra : rebase_source : 184d001a1233e9c035af070bc920c8cbeabc27cc
MessageId has the production "'~' ID", but if you use it, it produces
an error. This error was added in 2009, in bug 525342. I doubt anybody
expects it to work any more, so it should just be a regular parse
error. This is the only usage of the literal ~ so it can now be
removed from there.
MozReview-Commit-ID: AivlLE8Nubv
--HG--
extra : rebase_source : 66f76d1528f0bcf624af97b9437834874e537eb8
We will use the new type for the generated IPDL message handler
prototype to make sure correct error handling method is called.
MozReview-Commit-ID: AzVbApxFGZ0
This patch moves FatalError to IProtocol. I had to make a few changes.
- I added a ProtocolName() method to find the name of the protocol.
- I gave the two-argument version of FatalError its own name. Otherwise
C++ doesn't like there to be two virtual methods with the same name
in cases where one is overridden and the other isn't (as happens
in IToplevelProtocol).
Moves OnProcessingError, OnChannelError, OnChannelConnected so that they're
only generated for toplevel protocols. For some reason APZCTreeManagerChild
implemented OnProcessingError. I'm not sure what the intention was there so
I removed it.
This moves some of the generated methods in subprotocols that simply defer
to the parent protocol to IProtocol. These methods are still overridden in
the toplevel protocol.
This patch stores mManager in IProtocol rather than in each individual
PFoo. It also adds a generic accessor for that field. Note that each
individual protocol still defines a Manager() function that returns
PFooParent or whatever. I tried to get rid of that but it was a lot
of work.
With this change, MessageChannel stores mListener as an IToplevelProtocol
rather than a MessageListener (which isn't really a useful concept on
its own). The MessageListener methods are split out to IProtocol and
IToplevelProtocol. MessageListener gets deleted. Some of the inline
functions in MessageChannel had to be moved to MessageChannel.cpp since
IToplevelProtocol isn't defined in MessageChannel.h.
Currently toplevel protocols inherit from both IProtocolManager<IProtocol>
and IToplevelProtocol. This change makes IToplevelProtocol inherit from
IProtocol, so now toplevel protocols only inherit from
IToplevelProtocol.
Currently all our protocols inherit from IProtocolManager<IProtocol>. I have
no idea why. This patch switches everything over to IProtocol, without any
templates. I had to move ReadActor to the .cpp file to avoid redefinition
errors.
This patch moves FatalError to IProtocol. I had to make a few changes.
- I added a ProtocolName() method to find the name of the protocol.
- I gave the two-argument version of FatalError its own name. Otherwise
C++ doesn't like there to be two virtual methods with the same name
in cases where one is overridden and the other isn't (as happens
in IToplevelProtocol).
Moves OnProcessingError, OnChannelError, OnChannelConnected so that they're
only generated for toplevel protocols. For some reason APZCTreeManagerChild
implemented OnProcessingError. I'm not sure what the intention was there so
I removed it.
This moves some of the generated methods in subprotocols that simply defer
to the parent protocol to IProtocol. These methods are still overridden in
the toplevel protocol.
This patch stores mManager in IProtocol rather than in each individual
PFoo. It also adds a generic accessor for that field. Note that each
individual protocol still defines a Manager() function that returns
PFooParent or whatever. I tried to get rid of that but it was a lot
of work.
With this change, MessageChannel stores mListener as an IToplevelProtocol
rather than a MessageListener (which isn't really a useful concept on
its own). The MessageListener methods are split out to IProtocol and
IToplevelProtocol. MessageListener gets deleted. Some of the inline
functions in MessageChannel had to be moved to MessageChannel.cpp since
IToplevelProtocol isn't defined in MessageChannel.h.
Currently toplevel protocols inherit from both IProtocolManager<IProtocol>
and IToplevelProtocol. This change makes IToplevelProtocol inherit from
IProtocol, so now toplevel protocols only inherit from
IToplevelProtocol.
Currently all our protocols inherit from IProtocolManager<IProtocol>. I have
no idea why. This patch switches everything over to IProtocol, without any
templates. I had to move ReadActor to the .cpp file to avoid redefinition
errors.
For every protocol's RemoveManagee method, and every sub-protocol that
protocol manages, we generate:
MOZ_DIAGNOSTIC_ASSERT(mManagedPSubProtcolChild.Contains(actor), "...");
which dumps strings into the binary like:
(mManagedPAsmJSCacheEntryChild).Contains(actor) (actor not managed by this!)
MOZ_RELEASE_ASSERT((mManagedPAsmJSCacheEntryChild).Contains(actor)) (actor not managed by this!)
The linker is capable of merging multiple strings together, but
including the sub-protocol in every assert expression effectively
defeats this linker optimization, resulting in ~40KB of unnecessary
strings.
We can improve this situation by taking a reference to the managee
container, and using that reference in the assertion expression. All
the assertion expressions are identical, and the linker can perform the
expected string merging, for a savings of ~40KB.
Most calls to MaybeDestroy() have their return value checked, but those that
take a T__None argument legitimately don't. Coverity gets upset by this, so
let's cast those calls to void to make Coverity happier.
--HG--
extra : rebase_source : 798c852371a682372c61cae953cc13c160b9ff9a
The IPDL compiler generates code like this in DestroySubtree methods:
// Recursively shutting down PAPZ kids
nsTArray<PAPZChild*> kids((mManagedPAPZChild).Count());
// Accumulate kids into a stable structure to iterate over
ManagedPAPZChild(kids);
for (uint32_t i = 0; (i) < ((kids).Length()); (++(i))) {
// Guarding against a child removing a sibling from the list during the iteration.
if ((mManagedPAPZChild).Contains(kids[i])) {
(kids[i])->DestroySubtree(subtreewhy);
}
}
There are three problems with this code:
1) We initialize |kids| with a capacity, which is wasted work;
ManagedPAPZChild() is going to resize the array for us anyway.
2) We're constantly reading from |kids.Length()| in the loop, when we
only really need to read from it once.
3) We're repeatedly accessing kids[i], and doing needless bounds checks.
Let's fix those three problems.
For better or worse, the IPDL compiler, for protocols that send or
receive arrays, generates Write methods for transferring those arrays,
rather than going through the standard ParamTraits specializations.
These generated methods perform unnecessary bounds checks when accessing
elements; such checks can be safely bypassed because we know the exact
length of all the arrays involved. (Some compilers are not smart enough
to eliminate the bounds checks on their own.)
For better or worse, the IPDL compiler, for protocols that send or
receive arrays, generates Read methods for transferring those arrays,
rather than going through the standard ParamTraits specializations.
These generated methods perform unnecessary bounds checks when accessing
elements; such checks can be safely bypassed because we know the exact
length of all the arrays involved. (Some compilers are not smart enough
to eliminate the bounds checks on their own.)
This addition makes many of the upcoming patches significantly easier to
write, and enables us to avoid unpleasantness trying to fiddle with
ipdl.py's notions of C++ types. (For instance, there's no good way,
when you have a type in-hand, to say the moral equivalent of
std::add_pointer<T>::type.)
The only thing we needed opened actor tracking for was the ability to
clone all the actors. But now that we no longer have support for
cloning actors, we no longer need to track the actors that we've cloned,
which makes a number of things significantly simpler.
CloneOpenedToplevels, which is never called, is the only interesting
caller of CloneToplevel. And CloneToplevel, in turn, is the only
interesting caller of CloneManagees. Which means we can ditch all this
code for a decent amount of space savings, both in code and writable
static data (no more useless virtual function entries in vtables).
The standard placement new function requires a null check, implicitly
generated by the compiler, on its returned value. For places we know
don't deal with null pointers, such as all the generated methods of IPDL
code, we can use an overloaded operator new that doesn't require the
null check.
We construct Trigger structures for every IPDL send and recv to run
the (currently defunct) IPDL state machine for each protocol. These
structures are 64 bits to hold an enum that could be represented in 1
bit and an IPDL message enum that can easily be represented in 31 bits.
Therefore, we can make the Trigger structure smaller by storing the
members as bitfields, rather than full-width integers. (The message
enums are formed from a 16-bit protocol enum and a 16-bit
protocol-specific message enum; since the protocol enum goes in the
upper bits, we'd need 32768 protocols to overflow the bitfield we're
using in Trigger...a number that we're not going to hit anytime soon.)
This change saves ~15K of space on x86-64 Linux.
The calls to IPDL Transition() functions consistently look like:
Transition(VAR, ..., &VAR);
We can save space by only passing &VAR and deriving the state we're
coming from by loading VAR in Transition itself. It's not great using
inout parameters here, but we call Transition enough times that this
change saves a reasonable amount of space: about 10K on x86-64 Linux.
The unsightliness of inout parameters here is lessened by the only
callers of Transition being generated code.
There's no reason to generate identical code for every kind of managed
protocol; we can have a single instance that operates on void* and cast
our way to victory. This change saves ~60K of text on x86-64 Linux.
The patch is generated from following command:
rgrep -l unused.h|xargs sed -i -e s,mozilla/unused.h,mozilla/Unused.h,
MozReview-Commit-ID: AtLcWApZfES
--HG--
rename : mfbt/unused.h => mfbt/Unused.h
This removes the unnecessary setting of c-basic-offset from all
python-mode files.
This was automatically generated using
perl -pi -e 's/; *c-basic-offset: *[0-9]+//'
... on the affected files.
The bulk of these files are moz.build files but there a few others as
well.
MozReview-Commit-ID: 2pPf3DEiZqx
--HG--
extra : rebase_source : 0a7dcac80b924174a2c429b093791148ea6ac204
To enable string sharing, we're going to have helpful functions that
take a small, distinguishable, sharable string and construct a more
complete error message out of that. To do that easily with checkedRead,
we need to be able to pass custom parameters into the error function.
Actor reading from IPC message is codegen'd with a lot of repeated code.
We can improve that by moving the core actor reading code out of
subclasses into IProtocolmanager. While we still need to codegen a bit
of code to cast the read actor to the proper type, the code overall is
smaller. The lone downside is that if we do encounter an error reading
the actor id out of the message, the precision of our crash messages is
reduced somewhat: we no longer have the protocol name doing the reading,
nor do we get crash report annotations, since we can't tell whether
we're in the parent or child process.
checkedRead is set up to single-quote whatever message is passed in.
This scheme works great for all existing messages, but it makes some
callsites a little surprising ("where's the matching quote?") and
doesn't work well with message changes to be made in future patches.
Let's move the quoting out to client code.
Similar to part 1, this change enables the strings passed to
ProtocolErrorBreakpoint to be collapsed into a single string, saving
~60K of read-only data (!). This change does affect debuggability
slightly, but given that ProtocolErrorBreakpoint only tries to throw the
passed-in string to stderr, I don't think it's a huge deal.
We have better ways of getting the protocol name at the point of the
error (e.g. backtraces). Removing it means the error message can be
condensed to a single string by the compiler/linking, saving ~8k of
read-only data.
The original changeset that is being backed out had comment:
Bug 1023941 - Part 5: Loader hook to redirect the missing import.
The changes made in bug 1023941 were to work around the fact that with VS2013, msvcr120.dll imports kernel32!GetLogicalProcessorInformation, which is not available on Windows XP SP2.
In VS2015, the GetLogicalProcessorInformation requirement has moved into concrt140.dll (concurrency runtime), which we don't use.
So, now that our build infra is building with VS2015, we can remove the hooking and static runtime linking required to get the VS2013 fix to work.
In addition we need to do that to be able us to link the Chromium sandbox code into firefox.exe and get it to build and run with both VS2015 and VS2013.
MozReview-Commit-ID: 1tlXaYJ8dHH
--HG--
extra : rebase_source : 49b216e34fc5c77af96df1f67ee44c46d5368bfe
Specifically, it's important not to try to use Move() in the
Alloc+SendConstructor convenience method, because that Move()s the same
value twice (as discovered in bug 1186706), and neither callee
requires rvalue references in that case anyway.
This also removes the previous feature of calling makeCxxArgs with
params=0 to omit the message's input parameters, because it's not being
used and doesn't appear to have ever been used; it could be restored
(e.g., as paramsems=None) if needed.
It's an annotation that is used a lot, and should be used even more, so a
shorter name is better.
MozReview-Commit-ID: 1VS4Dney4WX
--HG--
extra : rebase_source : b26919c1b0fcb32e5339adeef5be5becae6032cf
We do this for the same reasons outlined in part 1: calls to
NS_RUNTIMEABORT are rather large and we generate a lot of them (~1000
left after part 1). This patch reduces .text size by ~20K on x86-64
Linux.
We don't do anything with it in terms of error reporting, we pass in 0
in the child process, and if you're in a debugger, presumably you can
figure out the other process's PID yourself.
NS_RUNTIMEABORT generates rather a lot of code, since we have to load up
five arguments, two of which are strings, and then call NS_DebugBreak.
Instead of doing this, let's just call each protocol's FatalError, which
only requires loading one string argument. Each protocol's FatalError
calls mozilla::ipc::FatalError, which communicates exactly the same
information as the inlined NS_RUNTIMEABORT would (e.g. crash reporter
annotations), but at a significant code savings: this patch reduces
.text by ~80K on Linux x86-64.
In bug 1259428, we changes a bunch of things that were previously
classes to refer to functions instead. Jed suggested in bug 1259428
comment 10 that we might want to rename things to reflect the new world
order as a followup. Consider it done.
All we use our Message subclasses for nowadays is the constructor, so we
might as well strip the class away and just have functions that perform
the construction for us. This change eliminates unnecessary vtables as
well as making the included headers somewhat smaller, which is always
nice.
Various bits of IPDL code would do something like:
Message* m = ...;
if (m.type() == particular message type) {
static_cast<ParticularMessageType*>(m)->name();
}
The static_cast is a remnant of having to do the downcast to access the
Log() method on the concrete subclass. Since name() is defined on
Message, there's no need for these casts anymore, so let's remove them.
The first step to eliminating all the generated Message subclasses the
IPDL compiler spits out is to move the functionality of their Log
methods someplace else. In addition to eliminating the need for the Log
methods, this change has the welcome effect of moving a bunch of code
that would be generated hundreds of times into a single place, which
should reduce code size a bit (debug builds only). We don't actually
remove the generation of the Log methods; that change will be done for a
future patch.
Henry Chang
JW Wang
David Anderson
Phil Ringnalda
Samael Wang
Kan-Ru Chen
Iris Hsiao
Sebastian Hengst
Aaron Klotz
raouldc
Bill McCloskey
Kartikaya Gupta
Chris Pearce
Andrew McCreight
Carsten "Tomcat" Book
Eric Rahm
Wes Kocher
Michael Layzell
Ehsan Akhgari
Kearwood Gilbert
Andrea Marchesini
Eitan Isaacson
David Parks
Blake Kaplan
Bevis Tseng
Mike Hommey
Nathan Froyd
Tomislav Jurin
Matt Woodrow
Nicholas Nethercote
Nathan Froyd
David Anderson
Tom Tromey
Gabor Krizsanits
Cervantes Yu
Jonathan Watt
Bob Owen
Chris Peterson
Jed Davis
Kyle Huey
Nathan Froyd
Lee Salzman
Nicolas Silva