921502 - Startup cache test uses JS from external linkage

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Benjamin Smedberg]

Currently there is a test TestStartupCache.cpp. This test is a standalone C++ test (external linkage) which also uses JS code. Since we're going to stop exporting JS symbols from libxul, this is going to stop working.

I remember being the initial reviewer of this test, which IIRC couldn't be an xpcshell test because we didn't want to make nsIStartupCache scriptable for fear of being abused by addons.

Since then in bug 711297 we've added tests to ensure that various telemetry data surrounding the startup cache are correct. This currently involves mucking about with JSAPI from within the C++ test, which has caused pain several times because this is one of the few or the only test which does this.

I *think* that we could probably achieve better results and maintenance by doing something like putting the telemetry tests into a JS component which the test calls into.

It would be nice to move this test to be a gtest, but since it has lots of weird side-effects, I really don't want to do that without first fixing gtest to be able to run some tests in their own process.

I'm going to play with this, but bholley/nfroyd please let me know if you have thoughts about the best way to do this.

Comment 1

[Benjamin Smedberg]

Attachment: Use a JS component to collect telemetry from the startup cache C++ test, instead of using raw JSAPI, r?nfroyd or bholley

Comment 2

[Nathan Froyd [:froydnj]]

Comment on attachment 811190 [details] [diff] [review]
Use a JS component to collect telemetry from the startup cache C++ test, instead of using raw JSAPI, r?nfroyd or bholley

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

Works for me.  That JS API stuff was a pain to write in the first place, glad it's gone.

Comment 3

[Bobby Holley (:bholley)]

Comment on attachment 811190 [details] [diff] [review]
Use a JS component to collect telemetry from the startup cache C++ test, instead of using raw JSAPI, r?nfroyd or bholley

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

I'm happy with nfroyd's review here.

Comment 4

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/8ee9a40710b7