1464890 - Use integers instead of raw pointer values as PRemoteSpellCheck IDs

Status: RESOLVED FIXED

(Core :: Spelling checker, defect, P3)

Description

[Brian Hackett [Laid off!]]

Attachment: patch

RemoteSpellCheckEngineChild uses pointer values from its process as IPDL identifiers, which is causing problems with Web Replay as in a replaying process the pointer values will be reproduced exactly and are unsafe to dereference.  The attached patch changes things to use integer identifiers instead.

Comment 1

[(no longer active)]

Comment on attachment 8981230 [details] [diff] [review]
patch

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

::: extensions/spellcheck/hunspell/glue/RemoteSpellCheckEngineChild.cpp
@@ +58,3 @@
>    }
> +  mResponsePromises[aId] = nullptr;
> +  while (mResponsePromises.Length() && !mResponsePromises.LastElement()) {

Hmm, is the loop condition right here?  It seems to me that if aId points to the last element in mResponsePromises, this loop would do the wrong thing, because in the first iteration, mResponsePromises.LastElement() would return nullptr and we would never enter the body of the loop.

r=me with this fixed or explained in case I'm missing something.

Comment 2

[Brian Hackett [Laid off!]]

(In reply to :Ehsan Akhgari from comment #1)
> Comment on attachment 8981230 [details] [diff] [review]
> patch
> 
> Review of attachment 8981230 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: extensions/spellcheck/hunspell/glue/RemoteSpellCheckEngineChild.cpp
> @@ +58,3 @@
> >    }
> > +  mResponsePromises[aId] = nullptr;
> > +  while (mResponsePromises.Length() && !mResponsePromises.LastElement()) {
> 
> Hmm, is the loop condition right here?  It seems to me that if aId points to
> the last element in mResponsePromises, this loop would do the wrong thing,
> because in the first iteration, mResponsePromises.LastElement() would return
> nullptr and we would never enter the body of the loop.
> 
> r=me with this fixed or explained in case I'm missing something.

The loop condition tests !mResponsePromises.LastElement(), so we should only enter the loop and remove old entries if the last entry has been null'ed out.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ff3c10d0cc05
Use integers instead of raw pointer values as PRemoteSpellCheck IDs, r=ehsan.

Comment 4

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/ff3c10d0cc05