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

Status: RESOLVED FIXED

(Core :: DOM: Workers, defect)

Description

[Olli Pettay [:smaug][[e-mail]]]

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.

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

Attachment: v3

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

https://tbpl.mozilla.org/?tree=Try&rev=54250f135e04

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

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.

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Note, I assume the setup may need some tweaking while landing more stuff to support
DOM events in workers.

Comment 5

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

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?

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

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.

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

And that SetEventHandler is for chrome only JS event targets to implement onfoos.

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/17c36f363203

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/17c36f363203