714778 - `toString` is broken when used in multiple content script

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: General, defect, P1)

Description

[Alexandre Poirot [:ochameau]]

I noticed that annotator example is currently broken.
There is multiple issues, the main one is an exception happening on:
  document.location.toString()

It was quite hard to track it down, but the precise issue lives here:
https://github.com/mozilla/addon-sdk/blob/master/packages/api-utils/data/content-proxy.js#L696
  if (name == "toString")
    return wrap(obj.wrappedJSObject ? obj.wrappedJSObject.toString
                                    : obj.toString,
                obj, name);

The mistake is that we share `obj.wrappedJSObject.toString` between all content scripts. We do not share XrayWrappers, but we share unwrapped DOM nodes. So that `obj` is unique per content script (each compartment comes with its own set of XrayWrappers), but `obj.wrappedJSObject` refer to a shared object reference between all sandboxes/compartements.
We end up sharing the same proxy function for such `toString` attribute.
And this sharing ends up breaking all content scripts except the first one.

We share a proxy because of this code:
https://github.com/mozilla/addon-sdk/blob/master/packages/api-utils/data/content-proxy.js#L336
  if ("___proxy" in fun)
    return fun.___proxy;
`fun` is equal to `obj.wrappedJSObject.toString`


There is multiple ways to address this bug:
 - hotfix `toString` in order to avoid passing a shared function to `wrap()`
 - start using WeakMaps in order to havoid setting `___proxy` attributes on objects being proxified

Comment 1

[Alexandre Poirot [:ochameau]]

Attachment: Pull request 316 - hotfix

As it looks like a blocker for 1.4, I think it makes more sense to apply a simplier hotfix.

Comment 2

[[github robot]]

Commits pushed to https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/51fd428bcf4940fce41b421126c3269af5023044
Bug 714778: Hotfix `toString` being broken in content scripts.

https://github.com/mozilla/addon-sdk/commit/95fd988264e993b52bb00868012b92312d4bad88
Merge pull request #316 from ochameau/fix-toString

fix Bug 714778: Hotfix `toString` being broken in content scripts. r=@mykmelez

Comment 3

[[github robot]]

Commit pushed to https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/6ccbc0ceff19238661e6148b277a21de3a8af90e
Bug 714778: Hotfix `toString` being broken in content scripts.
(cherry picked from commit 51fd428bcf4940fce41b421126c3269af5023044)