Closed Bug 1442344 Opened 4 years ago Closed 4 years ago

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


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

Not set



Tracking Status
firefox60 --- fixed


(Reporter: bzbarsky, Assigned: bzbarsky)




(1 file)

Most globals just have their WrapObject() assert/crash.  Windows should too.
MozReview-Commit-ID: Hq2xA1UetQu
Attachment #8955238 - Flags: review?(kyle)
Attachment #8955238 - Flags: review?(kyle) → review+
Pushed by
Make it clearer that nsGlobalWindowInner::WrapObject is never called.  r=qdot
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
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:

Failure log:

Flags: needinfo?(bzbarsky)
Resolution: FIXED → ---
Target Milestone: mozilla60 → ---
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).
Flags: needinfo?(apavel)
Boris, hi. No, sorry, i had both bugs opened. The other patch is the one with the issue, will update the bugs shortly.
Flags: needinfo?(apavel)
Closed: 4 years ago4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
Flags: needinfo?(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.  :(
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.
Flags: needinfo?(apavel)
Boris: the failure log is this:

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!)
Flags: needinfo?(apavel)
> 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?
Flags: needinfo?(ted)
Backed this out for now:
Flags: needinfo?(aryx.bugmail)
Resolution: FIXED → ---
We _can_ in fact call WrapObject() on a window. Stack:

 0!nsGlobalWindowInner::WrapObject [nsGlobalWindowInner.h:5baa664904e612e58c1234d13c9cb0f292876553 : 282 + 0x0]
 1!XPCConvert::NativeInterface2JSObject [XPCConvert.cpp:5baa664904e612e58c1234d13c9cb0f292876553 : 757 + 0x5]
 2!NativeInterface2JSObject [nsXPConnect.cpp:5baa664904e612e58c1234d13c9cb0f292876553 : 603 + 0xd]
 3!nsXPConnect::WrapNativeToJSVal [nsXPConnect.cpp:5baa664904e612e58c1234d13c9cb0f292876553 : 651 + 0xd]
 4!nsContentUtils::WrapNative [nsContentUtils.cpp:5baa664904e612e58c1234d13c9cb0f292876553 : 6774 + 0xd]
 5!xpcJSWeakReference::Get [nsContentUtils.h:5baa664904e612e58c1234d13c9cb0f292876553 : 2148 + 0xd]
 6!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 URL in the build log.
Closed: 4 years ago4 years ago
Flags: needinfo?(ted)
Flags: needinfo?(bzbarsky)
Resolution: --- → INVALID
Depends on: 1442892
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.