Closed Bug 1212906 Opened 4 years ago Closed 4 years ago

avoid handling windows messages while waiting for the responseto a sync a11y message

Categories

(Core :: IPC, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla44
Tracking Status
firefox44 --- fixed

People

(Reporter: tbsaunde, Unassigned)

References

Details

Attachments

(1 file)

No description provided.
Windows messages can trigger sync ipc messages to the child process.  That
means if we handle windows messages while waiting for the response to a sync
a11y ipc message we can end up reentering the code to send ipc messages which
is bad.  Try and avoid this situation by not handling windows messages while
waiting for a sync a11y message.
Attachment #8671432 - Flags: review?(wmccloskey)
Comment on attachment 8671432 [details] [diff] [review]
don't handle windows messages while waiting for a sync a11y ipc message

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

Looks good. Thanks.

::: dom/ipc/ContentParent.cpp
@@ +5477,5 @@
> +
> +bool
> +ContentParent::HandleWindowsMessages(const Message& aMsg) const
> +{
> +  if (!aMsg.is_sync()) {

Please MOZ_ASSERT(aMsg.is_sync())

::: ipc/glue/MessageLink.h
@@ +98,5 @@
>          return RIPChildWins;
>      }
>  
> +    /**
> +     * Return true if windows messages can be handled while waiting for a reply

Please replace 'can' with 'should'.
Attachment #8671432 - Flags: review?(wmccloskey) → review+
> ::: dom/ipc/ContentParent.cpp
> @@ +5477,5 @@
> > +
> > +bool
> > +ContentParent::HandleWindowsMessages(const Message& aMsg) const
> > +{
> > +  if (!aMsg.is_sync()) {
> 
> Please MOZ_ASSERT(aMsg.is_sync())

is that actually true? I'd think MessageChannel::Send() could call this for async messages too (though maybe we should avoid that there?)
Flags: needinfo?(wmccloskey)
MessageChannel::Send(Message* aMsg, Message* aReply) is only called for sync messages. There's a separate overload without a reply parameter that's used for async messages.
Flags: needinfo?(wmccloskey)
https://hg.mozilla.org/mozilla-central/rev/f753f2c6e79c
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
Blocks: 1210549
You need to log in before you can comment on or make changes to this bug.