1442344 - Make it clearer that WrapObject does not get called on globals (like windows)

Status: RESOLVED INVALID

(Core :: DOM: Core & HTML, enhancement)

Description

[Boris Zbarsky [:bzbarsky]]

Most globals just have their WrapObject() assert/crash.  Windows should too.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Make it clearer that nsGlobalWindowInner::WrapObject is never called

MozReview-Commit-ID: Hq2xA1UetQu

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b89286a88588
Make it clearer that nsGlobalWindowInner::WrapObject is never called.  r=qdot

Comment 3

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/b89286a88588

Comment 4

[Andreea Pavel [:apavel]]

Backed out for frequently failing crashtests on android, e.g. libeditor/crashtests/772282.html and layout/generic/crashtests/542136-1.html

Push that caused the failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=b89286a885886784f8fa728e2652bed07362935c

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=165537196&repo=mozilla-inbound&lineNumber=1894

Backout: https://hg.mozilla.org/mozilla-central/rev/accb1b3cf5932526b4cf983819f9fb4cc1111546

Comment 5

[Boris Zbarsky [:bzbarsky]]

Are you sure it was this patch?  This patch explicitly puts a MOZ_CRASH on a codepath.  If we were hitting that codepath at all (which we shouldn't be), I would expect to see that MOZ_CRASH message in the log (which I don't).

Comment 6

[Andreea Pavel [:apavel]]

Boris, hi. No, sorry, i had both bugs opened. The other patch is the one with the issue, will update the bugs shortly.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Ah, the "extra" log does have a MozCrashReason.  Need to figure out why we're getting there, but the stack in the logs is useless.  :(

Comment 8

[Boris Zbarsky [:bzbarsky]]

Hold on.  We are clearly hitting a "We should never get here!" MOZ_CRASH, which is very likely to be this bug, and not at all likely to be bug 1442313.  Furthermore, the tests involved don't even use workers, which is all bug 1442313 touched.

So you _should_ be backing out this bug and should _not_ be backing out bug 1442313.

Comment 9

[Andreea Pavel [:apavel]]

Boris: the failure log is this: https://treeherder.mozilla.org/logviewer.html#?job_id=165537196&repo=mozilla-inbound&lineNumber=1894

I searched for MOZ_CRASH in that log and did not find anything. Could you tell me what i'm missing? 

Aryx: can you help clarify this?

Comment 10

[Boris Zbarsky [:bzbarsky]]

> Could you tell me what i'm missing? 

Android is dump.  The MOZ_CRASH is in the .extra file, not in the log:

  MozCrashReason=MOZ_CRASH(We should never get here!)

Comment 11

[Boris Zbarsky [:bzbarsky]]

> Android is dump.

"dumb", that is.  ;)

Comment 12

[Andreea Pavel [:apavel]]

The failures keep occurring, so you are right :) i'm sorry for the confusion. I will backout the other bug, can you reland this one or should we?

Comment 13

[Boris Zbarsky [:bzbarsky]]

Ted, do you have any idea how I can get a useful stack here?  minidump_stackwalk gives me the same thing as the logs, with no function info.  Can I trigger a symbol upload for a particular push so I can point minidump_stackwalk to those symbols?

Comment 14

[Boris Zbarsky [:bzbarsky]]

Backed this out for now:

  https://hg.mozilla.org/mozilla-central/rev/9caf14e0b300

Comment 15

[Boris Zbarsky [:bzbarsky]]

We _can_ in fact call WrapObject() on a window. Stack:

 0  libxul.so!nsGlobalWindowInner::WrapObject [nsGlobalWindowInner.h:5baa664904e612e58c1234d13c9cb0f292876553 : 282 + 0x0]
 1  libxul.so!XPCConvert::NativeInterface2JSObject [XPCConvert.cpp:5baa664904e612e58c1234d13c9cb0f292876553 : 757 + 0x5]
 2  libxul.so!NativeInterface2JSObject [nsXPConnect.cpp:5baa664904e612e58c1234d13c9cb0f292876553 : 603 + 0xd]
 3  libxul.so!nsXPConnect::WrapNativeToJSVal [nsXPConnect.cpp:5baa664904e612e58c1234d13c9cb0f292876553 : 651 + 0xd]
 4  libxul.so!nsContentUtils::WrapNative [nsContentUtils.cpp:5baa664904e612e58c1234d13c9cb0f292876553 : 6774 + 0xd]
 5  libxul.so!xpcJSWeakReference::Get [nsContentUtils.h:5baa664904e612e58c1234d13c9cb0f292876553 : 2148 + 0xd]
 6  libxul.so!NS_InvokeByIndex [xptcinvoke_arm.cpp:5baa664904e612e58c1234d13c9cb0f292876553 : 167 + 0x3]

This involves us having a Window that is torn down enough that we have nulled out the wrapper, then someone trying to touch it from script.

For future reference, I got there by compiling google-breakpad (to get minidump_stackwalk) then grabbing symbols from the target.crashreporter-symbols.zip URL in the build log.