Figure out how to deal with mainthread-only atoms in worker events

RESOLVED FIXED in mozilla26

Status

()

RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: smaug, Assigned: smaug)

Tracking

unspecified
mozilla26
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Should we have per-thread atom tables? And perhaps store the thread info in atom, and
if for some reason comparing atom from another thread, do slow string
comparison.
Created attachment 789185 [details] [diff] [review]
v3
Assignee: nobody → bugs
Comment on attachment 789185 [details] [diff] [review]
v3

Let non-main-thread to deal with strings, not atoms.
This is not super pretty, but we agreed to use strings in workers, for now.
Attachment #789185 - Flags: review?(khuey)
Note, I assume the setup may need some tweaking while landing more stuff to support
DOM events in workers.
Blocks: 853893
No longer blocks: 887239
Comment on attachment 789185 [details] [diff] [review]
v3

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

::: content/events/src/EventTarget.cpp
@@ +40,5 @@
> +  }
> +  return SetEventHandler(nullptr,
> +                         Substring(aType, 2), // Remove "on"
> +                         aHandler, rv);
> +}

Yuck.  Have you done perf testing of code that goes through this method?

::: content/events/src/nsDOMEvent.cpp
@@ +57,5 @@
>                              nsPresContext* aPresContext, nsEvent* aEvent)
>  {
>    SetIsDOMBinding();
>    SetOwner(aOwner);
> +  mIsMainThreadEvent = mOwner || NS_IsMainThread();

Is this going to be true for long?  Why wouldn't worker thread events have owners?
Attachment #789185 - Flags: review?(khuey) → review+
That mOwner is nsPIDOMWindow, so certainly we won't have that in the workers.
Also, the owner is there just for parent object. In workers we should be able to return null
as parent.
And that SetEventHandler is for chrome only JS event targets to implement onfoos.
https://hg.mozilla.org/mozilla-central/rev/17c36f363203
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
You need to log in before you can comment on or make changes to this bug.