Closed Bug 501593 Opened 13 years ago Closed 13 years ago

update TestXPC to use Request Model


(Core :: XPConnect, defect)

Not set





(Reporter: timeless, Assigned: timeless)



(1 file, 1 obsolete file)

14.46 KB, patch
: review+
: superreview+
Details | Diff | Splinter Review
for a long time, gecko conspired with spidermonkey to give a free pass from the request model to js code running on the main thread, as long as it didn't try to pass the context to another thread.

i can't remember when spidermonkey revoked that pass, but it was a while ago, and no one updated the tests.
Attached patch use requests (obsolete) — Splinter Review
i just need someone to confirm that i've used the request model correctly.

note that this runs w/o fails, it does get plenty of assertions ("Script compiled without principals").
Attachment #386225 - Flags: superreview?(brendan)
Attachment #386225 - Flags: review?(brendan)
Comment on attachment 386225 [details] [diff] [review]
use requests

I owe jorendorff a review and I am out on paternity leave, so redirecting to Jason.

Attachment #386225 - Flags: superreview?(jorendorff)
Attachment #386225 - Flags: superreview?(brendan)
Attachment #386225 - Flags: review?(jorendorff)
Attachment #386225 - Flags: review?(brendan)
Comment on attachment 386225 [details] [diff] [review]
use requests

Thanks for updating these.

In the new EvaluateScript function, please do printf("%s", msg);.  I think gcc warns about that otherwise.

In TestArgFormatter, it looks like you could just declare the AutoRequest at the top and stay in a request for the duration of the function. Then you could avoid the do-while(0) loop. That approach might also work in TestSecurityManager. I didn't look closely.

Would you be willing to s/%%/%/ and again use printf("%s", msg) instead of a plain printf(msg)?
Attachment #386225 - Flags: review?(jorendorff) → review+
Attachment #386225 - Flags: superreview?(jorendorff) → superreview?(mrbkap)
Comment on attachment 386225 [details] [diff] [review]
use requests

I meant to say, r=me with those changes, and once again thanks for the patch.

I'm not a superreviewer; maybe mrbkap...?
Attachment #386225 - Flags: superreview?(mrbkap) → superreview+
Attached patch updatedSplinter Review
i've decided not to broaden the autorequests, i'm treating them as units and for tests this is perfectly fine.
Attachment #386225 - Attachment is obsolete: true
Attachment #389590 - Flags: superreview+
Attachment #389590 - Flags: review+
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.