Startup cache test uses JS from external linkage

RESOLVED FIXED in mozilla27

Status

()

Core
XPCOM
RESOLVED FIXED
4 years ago
2 years ago

People

(Reporter: Benjamin Smedberg, Assigned: Benjamin Smedberg)

Tracking

(Blocks: 1 bug)

Trunk
mozilla27
x86_64
Windows 7
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
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.
(Assignee)

Updated

4 years ago
Assignee: nobody → benjamin
(Assignee)

Comment 1

4 years ago
Created 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
Attachment #811190 - Flags: review?(bobbyholley+bmo)
(Assignee)

Updated

4 years ago
Attachment #811190 - Flags: review?(nfroyd)
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.
Attachment #811190 - Flags: review?(nfroyd) → review+
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.
Attachment #811190 - Flags: review?(bobbyholley+bmo)

Comment 4

4 years ago
https://hg.mozilla.org/mozilla-central/rev/8ee9a40710b7
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
Blocks: 1177103
You need to log in before you can comment on or make changes to this bug.