930097 - Fix an exact rooting hazard in DispatchEvent

Status: RESOLVED FIXED

(Core :: DOM: Workers, defect)

Description

[Terrence Cole [:terrence]]

Attachment: hazard_event_dispatch-v0.diff

The exact rooting analysis is not able to tell that the existing Vector<Holder> is not safe. Let's just switch this over to AutoValueVector now.

There is a comment in the source about this approach triggering crashes on Android: is there a bug number for that? Hopefully the problem is gone now: if not we should track it down and fix the underlying issue so we can use the correct rooting primitives here.

https://tbpl.mozilla.org/?tree=Try&rev=87580a1fd5a1

Comment 1

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Comment on attachment 821119 [details] [diff] [review]
hazard_event_dispatch-v0.diff

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

Are you sure this works? I wrote almost the exact same patch in bug 814026...

And I filed bug 835826 on it.

Comment 2

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

You should review the patch that removes all of this code instead :-P

Comment 3

[Terrence Cole [:terrence]]

Thanks for the bug references! I'm sad we never figured out what was going on there. Opt only crashes make me think it was a compiler issue and I believe we've upgraded since then, even on B2G. In any case, try appears to be quite green now, so it should be good to go.

(In reply to Kyle Huey [:khuey] ([e-mail]) from comment #2)
> You should review the patch that removes all of this code instead :-P

What is the timeframe on that?

Comment 4

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

(In reply to Terrence Cole [:terrence] from comment #3)
> (In reply to Kyle Huey [:khuey] ([e-mail]) from comment #2)
> > You should review the patch that removes all of this code instead :-P
> 
> What is the timeframe on that?

Whenever bent and peterv review bug 928312.  Ideally early next cycle.

Comment 5

[Terrence Cole [:terrence]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2dc38d7aba6f

Okay, that's not too long to wait if this push fails horribly. Hopefully this will just work, however.

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/2dc38d7aba6f