Closed Bug 1060046 Opened 10 years ago Closed 10 years ago

e10s EventTarget shim is broken


(Firefox :: Extension Compatibility, defect, P1)




Firefox 35
Tracking Status
e10s m3+ ---


(Reporter: billm, Assigned: billm)



(Keywords: addon-compat)


(1 file)

Attached patch fix-shimsSplinter Review
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.
Attachment #8480845 - Flags: review?(mconley)
Blocks: old-e10s-m2
tracking-e10s: --- → +
Keywords: addon-compat
OS: Linux → All
Priority: -- → P1
Hardware: x86_64 → All
Summary: EventTarget shim is broken → e10s EventTarget shim is broken
Comment on attachment 8480845 [details] [diff] [review]

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.
Attachment #8480845 - Flags: review?(mconley) → review+
Moving old M2 P1 bugs to M3.
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 35
You need to log in before you can comment on or make changes to this bug.