Closed Bug 1442344 Opened 2 years ago Closed 2 years ago
Make it clearer that Wrap
Object does not get called on globals (like windows)
Most globals just have their WrapObject() assert/crash. Windows should too.
Attachment #8955238 - Flags: review?(kyle)
Attachment #8955238 - Flags: review?(kyle) → review+
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/b89286a88588 Make it clearer that nsGlobalWindowInner::WrapObject is never called. r=qdot
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
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).
Boris, hi. No, sorry, i had both bugs opened. The other patch is the one with the issue, will update the bugs shortly.
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. :(
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.
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?
Flags: needinfo?(apavel) → needinfo?(aryx.bugmail)
> 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!)
> Android is dump. "dumb", that is. ;)
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?
Flags: needinfo?(apavel) → needinfo?(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?
Backed this out for now: https://hg.mozilla.org/mozilla-central/rev/9caf14e0b300
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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.
Status: REOPENED → RESOLVED
Closed: 2 years ago → 2 years ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.