Closed Bug 694775 Opened 10 years ago Closed 3 years ago
Crash [@ ns
Computed DOMStyle::Get Property CSSValue]
121 bytes, text/html
8.40 KB, text/plain
MozReview Request: Bug 694775 - avoid crashing by simply not climbing past the top of the tree (debug only)
40 bytes, text/x-review-board-request
The crash only happens on Windows (I don't know why), and only in debug builds (because http://hg.mozilla.org/mozilla-central/file/3b58a9df4c8c/layout/style/nsComputedDOMStyle.cpp#l514 is debug-only code).
That's bizarre. The document proto should so not matter to this code...
No crash for me on Windows 7.
[21:45:52.837] TypeError: anonymous method called on incompatible Proxy @ file:///C:/Users/jruderman/Desktop/b.html:5 Looks like it's no longer possible to set document.__proto__. Is that intentional?
Sounds like something to do with HTMLDocument-on-WebIDL.
I think that might be a result of bug 770344. We might be able to support this (treating it as an expando on setting), but it'll need JS engine changes afaict.
Er, wait. Why does the JS engine not allow setting __proto__ on proxies, exactly?
We should probably spin off a separate bug on making __proto__ work here again, then revisit this one once that's fixed...
It's up to the proxy to make [[Prototype]] setting work. Doing so requires overriding the proxy nativeCall trap. Right now we've done it for the inside-the-engine proxy handlers (for new Proxy(), Proxy.create(), etc.) and for the wrapper classes that are basically engine-internal, but we haven't touched anything else. If you want your proxies to allow this stuff, you need to add that trap and implement it to allow the mutation attempt to proceed.
Do we actually want to allow it?
Jesse: yes, since ES is adding proto-mutation in a standard way. Jeff: what does "allow the mutation attempt to proceed" mean, exactly? How should an object that happens to be a proxy merely as an implementation detail go about mutating its proto?
Actually, I spun off the __proto__ thing into bug 889198.
Depends on: 889198
Bug 694775 - avoid crashing by simply not climbing past the top of the tree (debug only) I've recently tripped across this one on live webpages while trying to debug other things.
Comment on attachment 8673967 [details] MozReview Request: Bug 694775 - avoid crashing by simply not climbing past the top of the tree (debug only) https://reviewboard.mozilla.org/r/22165/#review19763 OK, I guess. We'll still assert if that root style context isn't for a pseudo-element. And I think there are some obscure cases where we have style contexts that don't chain back up to the root element, where I suppose that's not a crazy thing to happen.
Attachment #8673967 - Flags: review+
FWIW, the attached testcase doesn't crash even without this patch applied. Would be nice if we could get one that still does.
Hrm, it didn't crash locally for me (using a trunk win64 debug build), but it does crash on Try still. https://treeherder.mozilla.org/logviewer.html#?job_id=13301032&repo=try
Flags: in-testsuite? → in-testsuite+
To be clear, the crashing Try link above includes the patch from this bug.
Nice. That's a slightly different stack so the patch is apparently just incomplete. I'll look into it. I'll also try the testcase in the environment where I first saw this crash.
Pushed by email@example.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/d4d6f3565400 Add a crashtest. r=mats
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.