After bug 793735 will have landed it would be good to switch XRE_StartupTimelineRecord() to use the TimeStamp class in order to have end-to-end compatible timestamps in the startup timeline. Changing the function signature from PRTime to TimeStamp will also require fixing up the current users of this function, namely browser/app/nsBrowserApp.cpp and mozglue/android/APKOpen.cpp.

Now that TimeStamp is in mozglue we can finally remove these 100+ lines of redundant code. The try run is here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=cff5b63a2187

Whoops, I attached the wrong patch, this is the right one.

It turns out that the Android build was broken, I've updated the patch to fix this issue.

Comment on attachment 8626042 [details] [diff] [review] [PATCH v2] Switch XRE_StartupTimelineRecord() from PRTime to TimeStamp Review of attachment 8626042 [details] [diff] [review]: ----------------------------------------------------------------- Hooray for deleted code. ::: mozglue/android/moz.build @@ +30,5 @@ > > GENERATED_INCLUDES += ['/build'] > LOCAL_INCLUDES += [ > '../linker', > + '../misc', I assume this is to pick up TimeStamp.h? Can't we just get it by including mozilla/TimeStamp.h in APKOpen.cpp instead of bare TimeStamp.h?

(In reply to Nathan Froyd [:froydnj] [:nfroyd] from comment #4) > I assume this is to pick up TimeStamp.h? Can't we just get it by including > mozilla/TimeStamp.h in APKOpen.cpp instead of bare TimeStamp.h? I'm not sure, I can try, I just went along with what the code did with the rest of the headers :-)

I've just tried using mozilla/TimeStamp.h so that I don't have to add misc to the local includes, this seems to work across all of our architectures: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ab357f02d24a Shall I land the patch this way then?

Yes!

Final version of the patch addressing the review comment in comment 4, carrying over the r+ flag.

Time to land, the green try run for patch v3 is this one: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ab357f02d24a

A surprise side effect of this change is that mozglue.dll (and in turn msvcr120.dll) is now loaded slightly earlier in startup. Previously the load was caused by DllBlocklist_Initialize() and now it is caused by TimeStamp::Now(). I *think* this is okay, but there is a very delicate balance in the startup sequence on Windows, so keep an eye out for regressions.