490072 - Rewrite XPCOM unit tests to use new synchronization API

Status: RESOLVED FIXED

(Core :: XPCOM, enhancement)

Description

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Where possible.  It's a good test of the new code, and we should be guaranteeing that old XPCOM stuff works with the new sync primitives.  Depends on bug 456272 because I don't trust the hack job I made of the old deadlock detector as part of bug 58904.

Let's see if porky.py is up to this job...

Comment 1

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: First cut, awaiting tryserver approval

Comment 2

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

And BTW, thanks porky.py!

Comment 3

[Johnny Stenback (:jst)]

Comment on attachment 377362 [details] [diff] [review]
First cut, awaiting tryserver approval

Looks good! I noticed one thing when glancing through this...

-        nsAutoLock guard(gLock);
+        MutexAutoLock  guard(*gLock);

When rewriting this pattern the tools seem to consistently put two spaces between the type and the variable name. Is that intentional?

Comment 4

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

(In reply to comment #3)
> (From update of attachment 377362 [details] [diff] [review])
> Looks good! I noticed one thing when glancing through this...
> 
> -        nsAutoLock guard(gLock);
> +        MutexAutoLock  guard(*gLock);
> 
> When rewriting this pattern the tools seem to consistently put two spaces
> between the type and the variable name. Is that intentional?

Oops, no.  Let me check into this ...

Comment 5

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: v2

Fixed up some porky bugs, regenerated.

Comment 6

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: Rewrite spec, for archival

For future reference, the steps I took to generate the patch were:

  (1) Run the porky rewrite on preprocessed firefox source
  (2) Apply the generated patch
  (3) Compile the unit tests, fix errors
      - all tests needed to have |#include "mozilla/..."; using namespace mozilla;| added by hand, since neither porky nor pork supports this yet
      - porky doesn't rewrite constructor expressions well yet; |nsAutoLock _(lockPtr)| was rewritten to |MutexAutoLock _(lockPtr)|, but needed to be |MutexAutoLock _(*lockPtr)|.  I fixed this by hand.
  (4) Fiddle with whitespace glitches, such as the table-alignment of class members being screwed up

Comment 7

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Pushed
b43beeb87f15 Chris Jones - bug 490072: upgrade XPCOM unit tests to new thread sync API. drop nsAutoLock test. r=bsmedberg