1559468 - Clean up uses of JS_DefineProfilingFunctions

Status: RESOLVED FIXED

(Core :: XPConnect, task, P3)

Description

[Andrew McCreight [:mccr8]]

In his review comment for bug 1559263, bz said: "We should really kill the non-working JProfFunctions thing and find a less sprinkly way of doing JS_DefineProfilingFunctions for all our globals."

Comment 1

[Andrew McCreight [:mccr8]]

This bug is for the latter part.

Comment 2

[Boris Zbarsky [:bzbarsky]]

So in particular, we call JS_DefineProfilingFunctions in the following:

1. nsJSContext::InitClasses
2. WorkerPrivate::RegisterBindings
3. XPCShellEnvironment::Init
4. mozJSComponentLoader::CreateLoaderGlobal
5. XRE_XPCShellMain

#5, #4, #3 all create the global via xpc::InitClassesWithNewWrappedGlobal. They are also the only callers of that method, so we could just move the JS_DefineProfilingFunctions call into there, I would think.

#2 creates the global via a WrapGlobalObject call on one of the WorkerGlobalScope implementations. Those eventually land in dom::CreateGlobal via binding Wrap methods.

#1 creates the global via CreateNativeGlobalForInner (except when it's getting a window out of bfcache, in which case it will try to define this stuff a second time??), which calls Window_Binding::Wrap, which lands in dom::CreateGlobal.

The only other callers of dom::CreateGlobal are AudioWorkletGlobalScope_Binding::Wrap, PaintWorkletGlobalScope_Binding::Wrap, and WorkerDebuggerGlobalScope_Binding::Wrap. Should those globals have profiling functions? If yes, we can just move the call into dom::CreateGlobal. If not, we could maybe add specializations of CreateGlobalOptions that indicate whether profiling functions are desired...

(My gut feeling is that we do want them in worklets; no idea about the worker debugger global.)

Comment 3

[Karl Tomlinson (:karlt)]

Profiling functions look useful for worklets also, yes.

I can't answer for WorkerDebuggerGlobalScope, but, even if not useful, would there be any harm in having the methods?

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: Bug 1559468 part 1. Move JS_DefineProfilingFunctions into InitClassesWithNewWrappedGlobal. r=mccr8

All callers of InitClassesWithNewWrappedGlobal already call it.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: Bug 1559468 part 2. Move JS_DefineProfilingFunctions into dom::CreateGlobal. r=mccr8

Vairious callers either already call it or should but are forgetting to. The exception is WorkerDebuggerGlobalScope_Binding::Wrap, but it should be OK to set up the profiling functions there too. r=mcr8

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/14b069bf9799
part 1.  Move JS_DefineProfilingFunctions into InitClassesWithNewWrappedGlobal.  r=mccr8
https://hg.mozilla.org/integration/autoland/rev/740f7d999d04
part 2.  Move JS_DefineProfilingFunctions into dom::CreateGlobal.  r=mccr8

Comment 7

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/14b069bf9799
https://hg.mozilla.org/mozilla-central/rev/740f7d999d04