Closed Bug 1257480 Opened 4 years ago Closed 4 years ago

null check for GetOrCreateGlobalScope() in WorkerDebuggerGlobalScope

Categories

(Core :: DOM: Workers, defect)

47 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: baku, Assigned: baku)

References

Details

Attachments

(1 file)

Attached patch console0.patchSplinter Review
No description provided.
Attachment #8731626 - Flags: review?(khuey)
Blocks: 1246091
I wrote this patch because I'm not 100% sure that we have a WorkerGlobalScope at any time we are playing with a WorkerDebuggerGlobalScope. If this is, we don't need all these security checks and we can use directory WorkerPrivate::GlobalScope().
Comment on attachment 8731626 [details] [diff] [review]
console0.patch

GetOrCreateGlobalScope gets the worker's global scope, or creates it if it doesn't yet exist (this can happen, because the worker is registered before it starts running, so the debugger can initialise before the worker itself).

In other words, GetOrCreateGlobalScope can never return nullptr, so this patch is unnecessary.
Attachment #8731626 - Flags: feedback?(ejpbruel) → feedback-
Good. It means that we can use WorkerPrivate::GlobalScope() directly. New patch coming.
Hold on, GetOrCreateGlobalScope() can fail. This patch covers this scenario.
Flags: needinfo?(ejpbruel)
Huh. You're right!

I don't know if this was originally not the case, or that we just forgot to check for it properly. In any case, we need to check against it, which is what your patch.

Good catch!
Flags: needinfo?(ejpbruel)
Attachment #8731626 - Flags: feedback- → feedback+
https://hg.mozilla.org/mozilla-central/rev/472e569106be
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in before you can comment on or make changes to this bug.