Add a more descriptive name for webworkers

RESOLVED WORKSFORME

Status

()

defect
RESOLVED WORKSFORME
6 years ago
3 years ago

People

(Reporter: BenWa, Assigned: BenWa)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

1.40 KB, patch
BenWa
: review+
Details | Diff | Splinter Review
Assignee

Description

6 years ago
No description provided.
Assignee

Comment 1

6 years ago
Posted patch patch (obsolete) — Splinter Review
Assignee: nobody → bgirard
Status: NEW → ASSIGNED
Attachment #751544 - Flags: review?(bent.mozilla)
Comment on attachment 751544 [details] [diff] [review]
patch

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

r=me as long as the reuse case is alright.

::: dom/workers/RuntimeService.cpp
@@ +522,5 @@
>      }
>  
>      JSRuntime* rt = JS_GetRuntime(cx);
>  
> +    nsCString escapedDomain(workerPrivate->Domain());

Nit: nsAutoCString

@@ +533,5 @@
> +    workerName = NS_LITERAL_CSTRING("WebWorker (") +
> +                 escapedDomain + NS_LITERAL_CSTRING(") (") +
> +                 escapedURL + NS_LITERAL_CSTRING(")");
> +
> +    profiler_register_thread(workerName.BeginReading());

What happens if you call this more than once? Will it just replace the old name? Threads are reused for different workers in some cases.
Attachment #751544 - Flags: review?(bent.mozilla) → review+
Assignee

Comment 3

6 years ago
(In reply to ben turner [:bent] (gone until July 22) from comment #2)
> @@ +533,5 @@
> > +    workerName = NS_LITERAL_CSTRING("WebWorker (") +
> > +                 escapedDomain + NS_LITERAL_CSTRING(") (") +
> > +                 escapedURL + NS_LITERAL_CSTRING(")");
> > +
> > +    profiler_register_thread(workerName.BeginReading());
> 
> What happens if you call this more than once? Will it just replace the old
> name? Threads are reused for different workers in some cases.

As long as they run through profiler_unregister_thread then first it should be fine.
Assignee

Comment 4

6 years ago
Posted patch patchSplinter Review
https://tbpl.mozilla.org/?tree=Try&rev=8f9912ae2555
Attachment #751544 - Attachment is obsolete: true
Attachment #777988 - Flags: review+
Assignee

Comment 5

3 years ago
This got fixed in another bug. A similar patch has landed.
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.