1060046 - e10s EventTarget shim is broken

Status: RESOLVED FIXED

(Firefox :: Extension Compatibility, defect, P1)

Description

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Attachment: fix-shims

I found a major bug in the shim code while working on bug 1059007. When you create a new Interposition, you're allowed to pass in an existing one that it inherits from. We use the proto chain to link them together. However, when looking for shims for a particular property, we use interp.hasOwnProperty, which doesn't actually look at the prototype.

The reason I switched to hasOwnPrototype is because I didn't want stuff from Object.prototype showing up as a shim. However, we can just set the prototype of the base shims to null to avoid that.

I also added a name property to every shim, which helps debugging. And I fixed a variable that needed |let| in front of it.

It's pretty clear that we need tests for this code though. I'll get working on that and post in this bug when I'm done.

Comment 1

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8480845 [details] [diff] [review]
fix-shims

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

This makes sense, but yes please for tests.

::: toolkit/components/addoncompat/RemoteAddonsParent.jsm
@@ +457,5 @@
>    },
>  
>    dispatch: function(browser, type, isTrusted, event) {
>      let targets = this.getTargets(browser);
> +    for (let target of targets) {

Ouch - good catch.

Comment 2

[Chris Peterson [:cpeterson]]

Moving old M2 P1 bugs to M3.

Comment 3

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/68b381faa378

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/68b381faa378