Closed Bug 616992 Opened 14 years ago Closed 13 years ago

DOM classes have no properties when accessed from a sandbox

Categories

(Core :: XPConnect, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Tracking Status
blocking2.0 --- betaN+

People

(Reporter: asaf, Assigned: mrbkap)

References

Details

(Keywords: regression, Whiteboard: [hardblocker])

Attachments

(2 files)

If a the prototype of a sandbox is set to a dom window, the window's "dom classes" properties (eg. window.NodeFilter) are accessible within the sandbox context.  However, the properties of these classes are inaccessible.

To reproduce, try this code in the js console:
alert('keys: ' + Object.keys(NodeFilter));
var sandbox = Components.utils.Sandbox(window, { sandboxPrototype: window });
var codeStr = "alert('keys in sandbox: ' + Object.keys(NodeFilter));";
Components.utils.evalInSandbox(codeStr, sandbox);
blocking2.0: --- → ?
Attached patch Proposed fixSplinter Review
When an object passes across a compartment boundary (such as between the chrome compartment and the sandbox) we attempt to rewrap it. Constructors didn't have a precreate hook, so we'd recreate the constructor in the sandbox's compartment, but the newly created object didn't have the PostCreatePrototype-created properties. This patch gives constructors a PreCreate hook, so we end up creating a transparent wrapper for the sandbox instead of a whole new object.
Assignee: nobody → mrbkap
Status: NEW → ASSIGNED
Attachment #495702 - Flags: review?(Olli.Pettay)
Depends on: 617836
This is a compartments regression, need to fix this for Firefox 4.
blocking2.0: ? → betaN+
Keywords: regression
Blocks: 613459
Attachment #495702 - Flags: review?(Olli.Pettay) → review?(peterv)
(In reply to comment #1)
> but the newly created object didn't have the
> PostCreatePrototype-created properties.

Trying to understand that, why not?
Summary: DOM classes have no properties when accessed from a sandbox → [needs review] DOM classes have no properties when accessed from a sandbox
mrbkap, can you answer peterv's question above please?
No longer blocks: 613459
Blocks: 613459
(In reply to comment #3)
> Trying to understand that, why not?

Because all we're wrapping is the constructor, we never actually create an object (or the real prototype for one). Furthermore, nsDOMConstructorSH::PostCreatePrototype purposely doesn't forward to nsDOMClassInfo::PostCreatePrototype.
Whiteboard: hardblocker
Whiteboard: hardblocker → [hardblocker]
Attachment #495702 - Flags: review+
With this additional change we get through all unit tests (didn't run talos) on Linux and Mac, but for some reason this (and the original patch as well) seems to cause an orange in xpcshell tests on windows only (?). The failure is in test_resource_async.js, here's the logs:

With my additional changes:

http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1294455964.1294458187.853.gz&fulltext=1
http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1294458189.1294459682.9181.gz&fulltext=1

W/o my additional changes:

http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1294434465.1294437012.26657.gz&fulltext=1
http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1294437831.1294439301.5596.gz&fulltext=1
I tried to reproduce the windows problem locally w/o luck, running all of xpcshell tests succeeds with the two patches in this bug applied, as does running the test that failed on try by itself. Therefore I pushed the two patches to try again and got no failures this time! My best guess is that my previous push was based on a bad changeset, or something.

Either way, I think this is ready to land now, once someone reviews my additional changes.
Attachment #502204 - Flags: review?(mrbkap)
Comment on attachment 502204 [details] [diff] [review]
Additional change to make the above get through mochitest.

Looks good to me.
Attachment #502204 - Flags: review+
Attachment #502204 - Flags: review?(mrbkap) → review+
Landed on m-c:

http://hg.mozilla.org/mozilla-central/rev/14752eafce65
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Summary: [needs review] DOM classes have no properties when accessed from a sandbox → DOM classes have no properties when accessed from a sandbox
Attachment #495702 - Flags: review?(peterv) → review+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: