Last Comment Bug 550309 - window.ImageData prototype should exist
: window.ImageData prototype should exist
Status: RESOLVED FIXED
: dev-doc-complete
Product: Core
Classification: Components
Component: Canvas: 2D (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla14
Assigned To: :Ms2ger
:
Mentors:
data:text/html,<script>alert(window.C...
: 637077 (view as bug list)
Depends on: 729680 730554 734334 736752 743615 746773 792561 805210
Blocks: 622842 538729 637077
  Show dependency treegraph
 
Reported: 2010-03-04 13:46 PST by Masatoshi Kimura [:emk]
Modified: 2014-11-25 10:33 PST (History)
15 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
add ImageData interface (14.28 KB, patch)
2012-01-28 21:49 PST, Benoit Jacob [:bjacob] (mostly away)
Ms2ger: feedback-
Details | Diff | Splinter Review
Patch v2 (45.40 KB, patch)
2012-01-29 06:19 PST, :Ms2ger
no flags Details | Diff | Splinter Review
Patch v3 (50.21 KB, patch)
2012-02-09 10:06 PST, :Ms2ger
mrbkap: feedback-
Details | Diff | Splinter Review
Patch v4 (49.76 KB, patch)
2012-02-22 13:24 PST, :Ms2ger
no flags Details | Diff | Splinter Review
attempt to use HoldScriptObject, and print debug info (5.21 KB, patch)
2012-02-22 15:52 PST, Benoit Jacob [:bjacob] (mostly away)
no flags Details | Diff | Splinter Review
Patch v5 (50.08 KB, patch)
2012-02-23 11:45 PST, :Ms2ger
no flags Details | Diff | Splinter Review
Part a: Tighten up support for fake ImageData objects (11.37 KB, patch)
2012-02-25 02:14 PST, :Ms2ger
bzbarsky: review-
Details | Diff | Splinter Review
Part b: Allow custom quickstubs that return JS::Values (2.93 KB, patch)
2012-02-25 02:17 PST, :Ms2ger
jorendorff: review+
Details | Diff | Splinter Review
Part c: Implement ImageData (17.18 KB, patch)
2012-02-25 03:58 PST, :Ms2ger
bzbarsky: review+
Details | Diff | Splinter Review
Part d: Try unwrapping to nsIDOMImageData in GetImageData() (1.34 KB, patch)
2012-02-25 04:01 PST, :Ms2ger
bzbarsky: review+
Details | Diff | Splinter Review
Part e: Remove custom quickstub for getImageData and return an actual ImageData (25.13 KB, patch)
2012-02-25 04:39 PST, :Ms2ger
no flags Details | Diff | Splinter Review
Part f: Return real ImageData from CreateImageData (3.84 KB, patch)
2012-02-25 04:51 PST, :Ms2ger
no flags Details | Diff | Splinter Review
Part e: Remove custom quickstub for getImageData and return an actual ImageData (26.09 KB, patch)
2012-02-25 05:35 PST, :Ms2ger
bzbarsky: review+
Details | Diff | Splinter Review
Part f: Return real ImageData from CreateImageData (5.61 KB, patch)
2012-02-25 05:36 PST, :Ms2ger
bzbarsky: review+
Details | Diff | Splinter Review
Part a: Tighten up support for fake ImageData objects (v2) (12.80 KB, patch)
2012-03-10 12:49 PST, :Ms2ger
bzbarsky: review+
Details | Diff | Splinter Review

Description Masatoshi Kimura [:emk] 2010-03-04 13:46:58 PST
Subject says all.
Comment 1 Boris Zbarsky [:bz] 2010-03-04 15:31:58 PST
So what is this bug about, exactly?  How are they not exposed?
Comment 2 Vladimir Vukicevic [:vlad] [:vladv] 2010-03-04 15:51:08 PST
I believe this is allowing 'new ImageData()' and 'new CanvasPixelArray()'.  I don't know if there's any value in having these, though.
Comment 3 Masatoshi Kimura [:emk] 2010-03-07 09:22:06 PST
At present window.CanvasPixelArray returns 'undefined', but it shouldn't.
There's already a test to check window.CanvasPixelArray.
http://hg.mozilla.org/mozilla-central/annotate/1dcea1cb129d/content/canvas/test/test_canvas.html#l7401
I don't care about 'new ImageData()' or 'new CanvasPixelArray()'.
Comment 4 Boris Zbarsky [:bz] 2010-03-07 11:20:14 PST
Ah, I see.  Would have been nice to say that in comment 0!

So this is just about the DOM prototypes needing to exist.  Is that specced somewhere?
Comment 5 Masatoshi Kimura [:emk] 2010-03-07 14:19:41 PST
(In reply to comment #4)
> So this is just about the DOM prototypes needing to exist.  Is that specced
> somewhere?
It seems to be specced in WebIDL.
http://www.w3.org/TR/WebIDL/#es-interfaces
> 4.3. Interfaces
> For every interface that is not declared with the [NoInterfaceObject] extended attribute, a corresponding property MUST exist on the ECMAScript global object whose name is the identifier of the interface.
Comment 6 Vladimir Vukicevic [:vlad] [:vladv] 2010-03-08 11:28:02 PST
Is there any point to actually having this, though?  Only thing that I can think of is so that 'instanceof CanvasPixelArray' can work, but that'll be a little tricky to actually make work.
Comment 7 Masatoshi Kimura [:emk] 2010-03-08 12:21:25 PST
Actual CanvasPixelArray will dramatically reduce the memory usage (4x lower).
Users may want to know whether CanvasPixelArray is supported to optimize the memory usage as much as possible.
Comment 8 Vladimir Vukicevic [:vlad] [:vladv] 2010-03-08 12:55:26 PST
What change in developer behaviour would you expect based on whether CanvasPixelArray is visible or not?   They'll get some object that'll behave like a CanvasPixelArray via createImageData/getImageData.

Note also that the typed array work is really a superset of this, and I'd rather focus developers on that.
Comment 9 :Ms2ger 2011-09-25 11:49:38 PDT
CanvasPixelArray was removed in favour of Uint8ClampedArray.
Comment 10 bugzilla33 2011-11-09 06:21:07 PST
Opera 12, IE 10, Safari 5.1, Chrome 15 have ImageData.prototype

Please add it.
Comment 11 :Ms2ger 2011-11-09 13:10:09 PST
Patches welcome.
Comment 12 :Ehsan Akhgari 2012-01-04 13:56:03 PST
I won't get to this soon realistically...
Comment 13 Benoit Jacob [:bjacob] (mostly away) 2012-01-12 11:35:35 PST
Doing it now: this is blocking our ability to pass the WebGL conformance tests. See bug 637077
Comment 14 Benoit Jacob [:bjacob] (mostly away) 2012-01-28 21:49:40 PST
Created attachment 592464 [details] [diff] [review]
add ImageData interface
Comment 15 Benoit Jacob [:bjacob] (mostly away) 2012-01-28 21:54:34 PST
I'm only requesting feedback, not review, on this patch at this point, because calling createImageData gives me consistent crashes, with stack trace below. Any idea what I'm doing wrong? I have a suspiscion it's because I removed AutoValueRooter stuff from the custom quickstub, but I never understood that stuff and I can't get it right just by trial and error.

0x00007f011b86151d in nanosleep () at ../sysdeps/unix/syscall-template.S:82
82	../sysdeps/unix/syscall-template.S: No such file or directory.
	in ../sysdeps/unix/syscall-template.S
(gdb) bt
#0  0x00007f011b86151d in nanosleep () at ../sysdeps/unix/syscall-template.S:82
#1  0x00007f011b8613bc in __sleep (seconds=0)
    at ../sysdeps/unix/sysv/linux/sleep.c:138
#2  0x00007f01166730fb in ah_crap_handler (signum=11)
    at /home/bjacob/mozilla-central/toolkit/xre/nsSigHandlers.cpp:121
#3  0x00007f0116678d16 in nsProfileLock::FatalSignalHandler (signo=11, 
    info=0x7fff169ec4b0, context=0x7fff169ec380)
    at /home/bjacob/mozilla-central/obj-firefox-debug/toolkit/profile/nsProfileLock.cpp:226
#4  <signal handler called>
#5  0x00007f0116ce0880 in js::gc::Cell::compartment (this=0x0)
    at ../../../dist/include/jsgc.h:962
#6  0x00007f01182fe50b in js::IsObjectInContextCompartment (obj=0x0, 
    cx=0x7f00f85ca6e0)
    at /home/bjacob/mozilla-central/js/src/jsfriendapi.cpp:234
#7  0x00007f0117641149 in XPCConvert::NativeInterface2JSObject (lccx=..., 
    d=0x7fff169ecb60, dest=0x0, aHelper=..., iid=0x7f011883e5a0, 
    Interface=0x7f01199a5dd0, allowNativeWrapper=true, isGlobal=false, 
    pErr=0x7fff169eca1c)
    at /home/bjacob/mozilla-central/js/xpconnect/src/XPCConvert.cpp:938
#8  0x00007f011768f7fe in xpc_qsXPCOMObjectToJsval (lccx=..., aHelper=..., 
    iid=0x7f011883e5a0, iface=0x7f01199a5dd0, rval=0x7fff169ecb60)
    at /home/bjacob/mozilla-central/js/xpconnect/src/XPCQuickStubs.cpp:1088
---Type <return> to continue, or q <return> to quit---
#9  0x00007f0117695438 in CreateImageData (cx=0x7f00f85ca6e0, w=10, h=10, self=
    0x0, x=0, y=0, vp=0x7f01036ff460)
    at ../../../dist/include/CustomQS_Canvas2D.h:213
#10 0x00007f01176958f5 in nsIDOMCanvasRenderingContext2D_CreateImageData (
    cx=0x7f00f85ca6e0, argc=2, vp=0x7f01036ff460)
    at ../../../dist/include/CustomQS_Canvas2D.h:300
#11 0x00007f011836fd59 in js::CallJSNative (cx=0x7f00f85ca6e0, 
    native=0x7f01176956ad <nsIDOMCanvasRenderingContext2D_CreateImageData(JSContext*, uintN, jsval*)>, args=...)
    at /home/bjacob/mozilla-central/js/src/jscntxtinlines.h:311
#12 0x00007f011835643c in js::InvokeKernel (cx=0x7f00f85ca6e0, args=..., 
    construct=js::NO_CONSTRUCT)
    at /home/bjacob/mozilla-central/js/src/jsinterp.cpp:520
#13 0x00007f0118362cc1 in js::Interpret (cx=0x7f00f85ca6e0, 
    entryFrame=0x7f01036ff3f0, interpMode=js::JSINTERP_NORMAL)
    at /home/bjacob/mozilla-central/js/src/jsinterp.cpp:2799
#14 0x00007f01183561f0 in js::RunScript (cx=0x7f00f85ca6e0, 
    script=0x7f00fb0ee180, fp=0x7f01036ff3f0)
    at /home/bjacob/mozilla-central/js/src/jsinterp.cpp:475
#15 0x00007f0118356d31 in js::ExecuteKernel (cx=0x7f00f85ca6e0, 
    script=0x7f00fb0ee180, scopeChain=..., thisv=..., type=js::EXECUTE_GLOBAL, 
    evalInFrame=0x0, result=0x7fff169edfc0)
    at /home/bjacob/mozilla-central/js/src/jsinterp.cpp:678
---Type <return> to continue, or q <return> to quit---
#16 0x00007f0118356f6e in js::Execute (cx=0x7f00f85ca6e0, 
    script=0x7f00fb0ee180, scopeChainArg=..., rval=0x7fff169edfc0)
    at /home/bjacob/mozilla-central/js/src/jsinterp.cpp:719
#17 0x00007f0118299872 in EvaluateUCScriptForPrincipalsCommon (
    cx=0x7f00f85ca6e0, obj=0x7f00fb0ea060, principals=0x7f01083a1188, 
    originPrincipals=0x0, chars=0x7f00fe2a3000, length=374, 
    filename=0x7f00fd264628 "Scratchpad", lineno=1, rval=0x7fff169edfc0, 
    compileVersion=JSVERSION_1_8)
    at /home/bjacob/mozilla-central/js/src/jsapi.cpp:5336
#18 0x00007f0118299945 in JS_EvaluateUCScriptForPrincipals (cx=0x7f00f85ca6e0, 
    obj=0x7f00fb0ea060, principals=0x7f01083a1188, chars=0x7f00fe2a3000, 
    length=374, filename=0x7f00fd264628 "Scratchpad", lineno=1, 
    rval=0x7fff169edfc0) at /home/bjacob/mozilla-central/js/src/jsapi.cpp:5347
#19 0x00007f0117637bd3 in xpc_EvalInSandbox (cx=0x7f00fe874200, 
    sandbox=0x7f00fb0ea060, source=..., filename=0x7f00fd264628 "Scratchpad", 
    lineNo=1, jsVersion=JSVERSION_1_8, returnStringOnly=false, 
    rval=0x7fff169ee4f8)
    at /home/bjacob/mozilla-central/js/xpconnect/src/XPCComponents.cpp:3484
#20 0x00007f011763756d in nsXPCComponents_Utils::EvalInSandbox (
    this=0x7f00f8fe24c0, source=..., sandboxVal=..., version=..., 
    filenameVal=..., lineNumber=1, cx=0x7f00fe874200, optionalArgc=3 '\003', 
    retval=0x7fff169ee4f8)
    at /home/bjacob/mozilla-central/js/xpconnect/src/XPCComponents.cpp:3381
---Type <return> to continue, or q <return> to quit---
#21 0x00007f0117e44def in NS_InvokeByIndex_P (that=0x7f00f8fe24c0, 
    methodIndex=6, paramCount=8, params=0x7fff169ee450)
    at /home/bjacob/mozilla-central/xpcom/reflect/xptcall/src/md/unix/xptcinvoke_x86_64_unix.cpp:195
#22 0x00007f011767bb09 in CallMethodHelper::Invoke (this=0x7fff169ee410)
    at /home/bjacob/mozilla-central/js/xpconnect/src/XPCWrappedNative.cpp:2895
#23 0x00007f011767994f in CallMethodHelper::Call (this=0x7fff169ee410)
    at /home/bjacob/mozilla-central/js/xpconnect/src/XPCWrappedNative.cpp:2226
#24 0x00007f01176797e6 in XPCWrappedNative::CallMethod (ccx=..., 
    mode=XPCWrappedNative::CALL_METHOD)
    at /home/bjacob/mozilla-central/js/xpconnect/src/XPCWrappedNative.cpp:2192
#25 0x00007f0117686cd4 in XPC_WN_CallMethod (cx=0x7f00fe874200, argc=5, 
    vp=0x7f01036ff368)
    at /home/bjacob/mozilla-central/js/xpconnect/src/XPCWrappedNativeJSOps.cpp:1539
#26 0x00007f011836fd59 in js::CallJSNative (cx=0x7f00fe874200, 
    native=0x7f0117686a76 <XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*)>, args=...) at /home/bjacob/mozilla-central/js/src/jscntxtinlines.h:311
#27 0x00007f011835643c in js::InvokeKernel (cx=0x7f00fe874200, args=..., 
    construct=js::NO_CONSTRUCT)
    at /home/bjacob/mozilla-central/js/src/jsinterp.cpp:520
#28 0x00007f0118362cc1 in js::Interpret (cx=0x7f00fe874200, 
    entryFrame=0x7f01036ff0a8, interpMode=js::JSINTERP_NORMAL)
---Type <return> to continue, or q <return> to quit---
    at /home/bjacob/mozilla-central/js/src/jsinterp.cpp:2799
#29 0x00007f01183561f0 in js::RunScript (cx=0x7f00fe874200, 
    script=0x7f00fbfdf098, fp=0x7f01036ff0a8)
    at /home/bjacob/mozilla-central/js/src/jsinterp.cpp:475
#30 0x00007f011835652d in js::InvokeKernel (cx=0x7f00fe874200, args=..., 
    construct=js::NO_CONSTRUCT)
    at /home/bjacob/mozilla-central/js/src/jsinterp.cpp:538
#31 0x00007f01182bd84f in js::Invoke (cx=0x7f00fe874200, args=..., 
    construct=js::NO_CONSTRUCT)
    at /home/bjacob/mozilla-central/js/src/jsinterp.h:157
#32 0x00007f011835670e in js::Invoke (cx=0x7f00fe874200, thisv=..., fval=..., 
    argc=1, argv=0x7fff169ef950, rval=0x7fff169ef8a0)
    at /home/bjacob/mozilla-central/js/src/jsinterp.cpp:570
#33 0x00007f011829a1e0 in JS_CallFunctionValue (cx=0x7f00fe874200, 
    obj=0x7f0100206640, fval=..., argc=1, argv=0x7fff169ef950, 
    rval=0x7fff169ef8a0) at /home/bjacob/mozilla-central/js/src/jsapi.cpp:5459
#34 0x00007f01170d8adb in nsJSContext::CallEventHandler (this=0x7f00fafedc10, 
    aTarget=0x7f00f8fda1f0, aScope=0x7f00fd6df240, aHandler=0x7f00fd67f700, 
    aargv=0x7f00fd53fdf0, arv=0x7fff169efbd0)
    at /home/bjacob/mozilla-central/dom/base/nsJSEnvironment.cpp:1960
#35 0x00007f01171a0b3b in nsJSEventListener::HandleEvent (this=0x7f00f7d44d00, 
    aEvent=0x7f00f8fcb030)
    at /home/bjacob/mozilla-central/dom/src/events/nsJSEventListener.cpp:209
---Type <return> to continue, or q <return> to quit---
#36 0x00007f0116e81b38 in nsEventListenerManager::HandleEventSubType (
    this=0x7f00f8fda280, aListenerStruct=0x7f00f8fda2b8, 
    aListener=0x7f00f7d44d00, aDOMEvent=0x7f00f8fcb030, 
    aCurrentTarget=0x7f00f8fda1f0, aPhaseFlags=6, aPusher=0x7fff169effb0)
    at /home/bjacob/mozilla-central/content/events/src/nsEventListenerManager.cpp:735
#37 0x00007f0116e81e04 in nsEventListenerManager::HandleEventInternal (
    this=0x7f00f8fda280, aPresContext=0x7f00f8fcc800, aEvent=0x7f00f8f29280, 
    aDOMEvent=0x7fff169eff90, aCurrentTarget=0x7f00f8fda1f0, aFlags=6, 
    aEventStatus=0x7fff169eff98, aPusher=0x7fff169effb0)
    at /home/bjacob/mozilla-central/content/events/src/nsEventListenerManager.cpp:793
#38 0x00007f0116eaf768 in nsEventListenerManager::HandleEvent (
    this=0x7f00f8fda280, aPresContext=0x7f00f8fcc800, aEvent=0x7f00f8f29280, 
    aDOMEvent=0x7fff169eff90, aCurrentTarget=0x7f00f8fda1f0, aFlags=6, 
    aEventStatus=0x7fff169eff98, aPusher=0x7fff169effb0)
    at /home/bjacob/mozilla-central/content/events/src/nsEventListenerManager.h:168
#39 0x00007f0116eafc98 in nsEventTargetChainItem::HandleEvent (
    this=0x7f00ffe08578, aVisitor=..., aFlags=6, 
    aMayHaveNewListenerManagers=false, aPusher=0x7fff169effb0)
    at /home/bjacob/mozilla-central/content/events/src/nsEventDispatcher.cpp:215
---Type <return> to continue, or q <return> to quit---
#40 0x00007f0116eb019c in nsEventTargetChainItem::HandleEventTargetChain (
    this=0x7f00ffe08508, aVisitor=..., aFlags=6, aCallback=0x0, 
    aMayHaveNewListenerManagers=false, aPusher=0x7fff169effb0)
    at /home/bjacob/mozilla-central/content/events/src/nsEventDispatcher.cpp:347
#41 0x00007f0116eb13a0 in nsEventDispatcher::Dispatch (aTarget=0x7f00f8fda1f0, 
    aPresContext=0x7f00f8fcc800, aEvent=0x7f00f8f29280, 
    aDOMEvent=0x7f00f8fcb030, aEventStatus=0x7fff169f0178, aCallback=0x0, 
    aTargets=0x0)
    at /home/bjacob/mozilla-central/content/events/src/nsEventDispatcher.cpp:681
#42 0x00007f0116eb17ad in nsEventDispatcher::DispatchDOMEvent (
    aTarget=0x7f00f8fda1f0, aEvent=0x0, aDOMEvent=0x7f00f8fcb030, 
    aPresContext=0x7f00f8fcc800, aEventStatus=0x7fff169f0178)
    at /home/bjacob/mozilla-central/content/events/src/nsEventDispatcher.cpp:744
#43 0x00007f0116d774bd in nsINode::DispatchEvent (this=0x7f00f8fda1f0, 
    aEvent=0x7f00f8fcb030, aRetVal=0x7fff169f02bf)
    at /home/bjacob/mozilla-central/content/base/src/nsGenericElement.cpp:1171
#44 0x00007f0116cf1bf1 in nsContentUtils::DispatchXULCommand (
    aTarget=0x7f00f8fda1f0, aTrusted=true, aSourceEvent=0x7f00f8fcaf80, 
    aShell=0x0, aCtrl=true, aAlt=false, aShift=false, aMeta=false)
    at /home/bjacob/mozilla-central/content/base/src/nsContentUtils.cpp:5215
---Type <return> to continue, or q <return> to quit---
#45 0x00007f0117479f1e in nsXULElement::PreHandleEvent (this=0x7f00f7d3c500, 
    aVisitor=...)
    at /home/bjacob/mozilla-central/content/xul/content/src/nsXULElement.cpp:1679
#46 0x00007f0116eafe6e in nsEventTargetChainItem::PreHandleEvent (
    this=0x7f00ffe08540, aVisitor=...)
    at /home/bjacob/mozilla-central/content/events/src/nsEventDispatcher.cpp:276
#47 0x00007f0116eb117d in nsEventDispatcher::Dispatch (aTarget=0x7f00f7d3c500, 
    aPresContext=0x7f00f8fcc800, aEvent=0x7f00f8f29220, 
    aDOMEvent=0x7f00f8fcaf80, aEventStatus=0x7fff169f0768, aCallback=0x0, 
    aTargets=0x0)
    at /home/bjacob/mozilla-central/content/events/src/nsEventDispatcher.cpp:626
#48 0x00007f0116eb17ad in nsEventDispatcher::DispatchDOMEvent (
    aTarget=0x7f00f7d3c500, aEvent=0x0, aDOMEvent=0x7f00f8fcaf80, 
    aPresContext=0x7f00f8fcc800, aEventStatus=0x7fff169f0768)
    at /home/bjacob/mozilla-central/content/events/src/nsEventDispatcher.cpp:744
#49 0x00007f0116d774bd in nsINode::DispatchEvent (this=0x7f00f7d3c500, 
    aEvent=0x7f00f8fcaf80, aRetVal=0x7fff169f08af)
    at /home/bjacob/mozilla-central/content/base/src/nsGenericElement.cpp:1171
#50 0x00007f0116cf1bf1 in nsContentUtils::DispatchXULCommand (
---Type <return> to continue, or q <return> to quit---
    aTarget=0x7f00f7d3c500, aTrusted=true, aSourceEvent=0x0, aShell=0x0, 
    aCtrl=true, aAlt=false, aShift=false, aMeta=false)
    at /home/bjacob/mozilla-central/content/base/src/nsContentUtils.cpp:5215
#51 0x00007f0117086d78 in nsXBLPrototypeHandler::DispatchXULKeyCommand (
    this=0x7f00f9571500, aEvent=0x7f00fe591c98)
    at /home/bjacob/mozilla-central/content/xbl/src/nsXBLPrototypeHandler.cpp:542
#52 0x00007f0117085794 in nsXBLPrototypeHandler::ExecuteHandler (
    this=0x7f00f9571500, aTarget=0x7f00f8fda1f0, aEvent=0x7f00fe591c98)
    at /home/bjacob/mozilla-central/content/xbl/src/nsXBLPrototypeHandler.cpp:264
#53 0x00007f0117084c20 in nsXBLWindowKeyHandler::WalkHandlersAndExecute (
    this=0x7f00fff8b1c0, aKeyEvent=0x7f00fe591c98, aEventType=0x7f01083840a0, 
    aHandler=0x7f00f9571780, aCharCode=114, aIgnoreShiftKey=false)
    at /home/bjacob/mozilla-central/content/xbl/src/nsXBLWindowKeyHandler.cpp:566
#54 0x00007f011708469a in nsXBLWindowKeyHandler::WalkHandlersInternal (
    this=0x7f00fff8b1c0, aKeyEvent=0x7f00fe591c98, aEventType=0x7f01083840a0, 
    aHandler=0x7f00f9571780)
    at /home/bjacob/mozilla-central/content/xbl/src/nsXBLWindowKeyHandler.cpp:483
#55 0x00007f0117083f21 in nsXBLWindowKeyHandler::WalkHandlers (
    this=0x7f00fff8b1c0, aKeyEvent=0x7f00fe591c98, aEventType=0x7f01083840a0)
---Type <return> to continue, or q <return> to quit---
    at /home/bjacob/mozilla-central/content/xbl/src/nsXBLWindowKeyHandler.cpp:358
#56 0x00007f0117084343 in nsXBLWindowKeyHandler::HandleEvent (
    this=0x7f00fff8b1c0, aEvent=0x7f00fe591c00)
    at /home/bjacob/mozilla-central/content/xbl/src/nsXBLWindowKeyHandler.cpp:404
#57 0x00007f0116e81b38 in nsEventListenerManager::HandleEventSubType (
    this=0x7f00f8f27f70, aListenerStruct=0x7f00faff6688, 
    aListener=0x7f00fff8b1c0, aDOMEvent=0x7f00fe591c00, 
    aCurrentTarget=0x7f00f8fcd000, aPhaseFlags=514, aPusher=0x7fff169f12d0)
    at /home/bjacob/mozilla-central/content/events/src/nsEventListenerManager.cpp:735
#58 0x00007f0116e81e04 in nsEventListenerManager::HandleEventInternal (
    this=0x7f00f8f27f70, aPresContext=0x7f00f86f5800, aEvent=0x7fff169f1c20, 
    aDOMEvent=0x7fff169f12b0, aCurrentTarget=0x7f00f8fcd000, aFlags=514, 
    aEventStatus=0x7fff169f12b8, aPusher=0x7fff169f12d0)
    at /home/bjacob/mozilla-central/content/events/src/nsEventListenerManager.cpp:793
#59 0x00007f0116eaf768 in nsEventListenerManager::HandleEvent (
    this=0x7f00f8f27f70, aPresContext=0x7f00f86f5800, aEvent=0x7fff169f1c20, 
    aDOMEvent=0x7fff169f12b0, aCurrentTarget=0x7f00f8fcd000, aFlags=514, 
    aEventStatus=0x7fff169f12b8, aPusher=0x7fff169f12d0)
    at /home/bjacob/mozilla-central/content/events/src/nsEventListenerManager.h:---Type <return> to continue, or q <return> to quit---
168
#60 0x00007f0116eafc98 in nsEventTargetChainItem::HandleEvent (
    this=0x7f00ffe08118, aVisitor=..., aFlags=514, 
    aMayHaveNewListenerManagers=false, aPusher=0x7fff169f12d0)
    at /home/bjacob/mozilla-central/content/events/src/nsEventDispatcher.cpp:215
#61 0x00007f0116eb02b3 in nsEventTargetChainItem::HandleEventTargetChain (
    this=0x7f00ffe08620, aVisitor=..., aFlags=518, aCallback=0x7fff169f1400, 
    aMayHaveNewListenerManagers=false, aPusher=0x7fff169f12d0)
    at /home/bjacob/mozilla-central/content/events/src/nsEventDispatcher.cpp:370
#62 0x00007f0116eb03df in nsEventTargetChainItem::HandleEventTargetChain (
    this=0x7f00ffe08620, aVisitor=..., aFlags=6, aCallback=0x7fff169f1400, 
    aMayHaveNewListenerManagers=false, aPusher=0x7fff169f12d0)
    at /home/bjacob/mozilla-central/content/events/src/nsEventDispatcher.cpp:402
#63 0x00007f0116eb13a0 in nsEventDispatcher::Dispatch (aTarget=0x7f00f87f0e80, 
    aPresContext=0x7f00f86f5800, aEvent=0x7fff169f1c20, aDOMEvent=0x0, 
    aEventStatus=0x7fff169f1a4c, aCallback=0x7fff169f1400, aTargets=0x0)
    at /home/bjacob/mozilla-central/content/events/src/nsEventDispatcher.cpp:681
#64 0x00007f0116a0a809 in PresShell::HandleEventInternal (this=0x7f00f86f7000, 
    aEvent=0x7fff169f1c20, aStatus=0x7fff169f1a4c)
---Type <return> to continue, or q <return> to quit---
    at /home/bjacob/mozilla-central/layout/base/nsPresShell.cpp:6647
#65 0x00007f0116a09a83 in PresShell::HandleEvent (this=0x7f00f86f7000, aFrame=
    0x7f00f87d1860, aEvent=0x7fff169f1c20, aDontRetargetEvents=true, 
    aEventStatus=0x7fff169f1a4c)
    at /home/bjacob/mozilla-central/layout/base/nsPresShell.cpp:6228
#66 0x00007f0116a0881b in PresShell::HandleEvent (this=0x7f00f8f9e800, 
    aFrame=0x7f00f8fd7c38, aEvent=0x7fff169f1c20, aDontRetargetEvents=false, 
    aEventStatus=0x7fff169f1a4c)
    at /home/bjacob/mozilla-central/layout/base/nsPresShell.cpp:5882
#67 0x00007f01170c40f3 in nsViewManager::DispatchEvent (this=0x7f00f8fa61f0, 
    aEvent=0x7fff169f1c20, aView=0x7f00f86ff060, aStatus=0x7fff169f1a4c)
    at /home/bjacob/mozilla-central/view/src/nsViewManager.cpp:917
#68 0x00007f01170be0bc in HandleEvent (aEvent=0x7fff169f1c20)
    at /home/bjacob/mozilla-central/view/src/nsView.cpp:158
#69 0x00007f0117adeb4d in nsWindow::DispatchEvent (this=0x7f00fe2f9b80, 
    aEvent=0x7fff169f1c20, aStatus=@0x7fff169f1cac)
    at /home/bjacob/mozilla-central/widget/gtk2/nsWindow.cpp:575
#70 0x00007f0117ae54f2 in nsWindow::OnKeyPressEvent (this=0x7f00fe2f9b80, 
    aWidget=0x7f00f8f2c580, aEvent=0x7f010222d7f0)
    at /home/bjacob/mozilla-central/widget/gtk2/nsWindow.cpp:3227
#71 0x00007f0117aec50b in key_press_event_cb (widget=0x7f00f8f2c580, 
    event=0x7f010222d7f0)
    at /home/bjacob/mozilla-central/widget/gtk2/nsWindow.cpp:5970
---Type <return> to continue, or q <return> to quit---
#72 0x00007f01132e2828 in ?? ()
   from /usr/lib/x86_64-linux-gnu/libgtk-x11-2.0.so.0
#73 0x00007f0114d840a4 in g_closure_invoke ()
   from /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
#74 0x00007f0114d9602a in ?? ()
   from /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
#75 0x00007f0114d9f483 in g_signal_emit_valist ()
   from /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
#76 0x00007f0114d9f852 in g_signal_emit ()
   from /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
#77 0x00007f01133fcdc1 in ?? ()
   from /usr/lib/x86_64-linux-gnu/libgtk-x11-2.0.so.0
#78 0x00007f011341262b in gtk_window_propagate_key_event ()
   from /usr/lib/x86_64-linux-gnu/libgtk-x11-2.0.so.0
#79 0x00007f011341512b in ?? ()
   from /usr/lib/x86_64-linux-gnu/libgtk-x11-2.0.so.0
#80 0x00007f01132e2828 in ?? ()
   from /usr/lib/x86_64-linux-gnu/libgtk-x11-2.0.so.0
#81 0x00007f0114d840a4 in g_closure_invoke ()
   from /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
#82 0x00007f0114d95e5f in ?? ()
   from /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
#83 0x00007f0114d9f483 in g_signal_emit_valist ()
---Type <return> to continue, or q <return> to quit---
   from /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
#84 0x00007f0114d9f852 in g_signal_emit ()
   from /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
#85 0x00007f01133fcdc1 in ?? ()
   from /usr/lib/x86_64-linux-gnu/libgtk-x11-2.0.so.0
#86 0x00007f01132e0af7 in gtk_propagate_event ()
   from /usr/lib/x86_64-linux-gnu/libgtk-x11-2.0.so.0
#87 0x00007f01132e0d83 in gtk_main_do_event ()
   from /usr/lib/x86_64-linux-gnu/libgtk-x11-2.0.so.0
#88 0x00007f0112d3309c in ?? ()
   from /usr/lib/x86_64-linux-gnu/libgdk-x11-2.0.so.0
#89 0x00007f01146b6a5d in g_main_context_dispatch ()
   from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#90 0x00007f01146b7258 in ?? () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#91 0x00007f01146b7429 in g_main_context_iteration ()
   from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#92 0x00007f0117aefc59 in nsAppShell::ProcessNextNativeEvent (
    this=0x7f0108278cf0, mayWait=true)
    at /home/bjacob/mozilla-central/widget/gtk2/nsAppShell.cpp:162
#93 0x00007f0117b1524a in nsBaseAppShell::DoProcessNextNativeEvent (
    this=0x7f0108278cf0, mayWait=true)
    at /home/bjacob/mozilla-central/widget/xpwidgets/nsBaseAppShell.cpp:171
#94 0x00007f0117b1567e in nsBaseAppShell::OnProcessNextEvent (
---Type <return> to continue, or q <return> to quit---
    this=0x7f0108278cf0, thr=0x7f011b25dc60, mayWait=true, recursionDepth=0)
    at /home/bjacob/mozilla-central/widget/xpwidgets/nsBaseAppShell.cpp:324
#95 0x00007f0117e2013b in nsThread::ProcessNextEvent (this=0x7f011b25dc60, 
    mayWait=true, result=0x7fff169f2aaf)
    at /home/bjacob/mozilla-central/xpcom/threads/nsThread.cpp:619
#96 0x00007f0117db2a57 in NS_ProcessNextEvent_P (thread=0x7f011b25dc60, 
    mayWait=true)
    at /home/bjacob/mozilla-central/obj-firefox-debug/xpcom/build/nsThreadUtils.cpp:245
#97 0x00007f0117c7f2ac in mozilla::ipc::MessagePump::Run (this=0x7f010cc54200, 
    aDelegate=0x7f011b2da8f0)
    at /home/bjacob/mozilla-central/ipc/glue/MessagePump.cpp:134
#98 0x00007f0117e6ee13 in MessageLoop::RunInternal (this=0x7f011b2da8f0)
    at /home/bjacob/mozilla-central/ipc/chromium/src/base/message_loop.cc:208
#99 0x00007f0117e6eda4 in MessageLoop::RunHandler (this=0x7f011b2da8f0)
    at /home/bjacob/mozilla-central/ipc/chromium/src/base/message_loop.cc:201
#100 0x00007f0117e6ed7d in MessageLoop::Run (this=0x7f011b2da8f0)
    at /home/bjacob/mozilla-central/ipc/chromium/src/base/message_loop.cc:175
#101 0x00007f0117b152d2 in nsBaseAppShell::Run (this=0x7f0108278cf0)
    at /home/bjacob/mozilla-central/widget/xpwidgets/nsBaseAppShell.cpp:189
#102 0x00007f01178529a4 in nsAppStartup::Run (this=0x7f01082ea830)
    at /home/bjacob/mozilla-central/toolkit/components/startup/nsAppStartup.cpp:220
---Type <return> to continue, or q <return> to quit---
#103 0x00007f0116664c40 in XRE_main (argc=4, argv=0x7fff169f5768, 
    aAppData=0x623c60)
    at /home/bjacob/mozilla-central/toolkit/xre/nsAppRunner.cpp:3537
#104 0x00000000004023f9 in do_main (
    exePath=0x7fff169f4660 "/home/bjacob/mozilla-central/obj-firefox-debug/dist/bin/", argc=4, argv=0x7fff169f5768)
    at /home/bjacob/mozilla-central/browser/app/nsBrowserApp.cpp:205
#105 0x0000000000402660 in main (argc=4, argv=0x7fff169f5768)
    at /home/bjacob/mozilla-central/browser/app/nsBrowserApp.cpp:295
Comment 16 :Ms2ger 2012-01-29 06:19:49 PST
Created attachment 592497 [details] [diff] [review]
Patch v2
Comment 17 :Ms2ger 2012-01-29 06:20:14 PST
Comment on attachment 592464 [details] [diff] [review]
add ImageData interface

f-, crashes ;)
Comment 18 :Ms2ger 2012-02-09 10:06:35 PST
Created attachment 595793 [details] [diff] [review]
Patch v3

This fails pretty badly on try; any idea what's wrong?

https://tbpl.mozilla.org/?tree=Try&rev=da3f5e4f8156
Comment 19 Blake Kaplan (:mrbkap) 2012-02-10 08:29:43 PST
Comment on attachment 595793 [details] [diff] [review]
Patch v3

I caught the crash here in a debugger and found that the problem is that ImageData calls PreserveWrapper on itself before it itself has a JSObject. Then, when we actually wrap the C++ object for JS, we overwrite the "preserving bit" making nsContentUtils::ReleaseWrapper become a no-op, leaving stale ImageData wrappers in the preserved wrapper map and crashing.

Ms2ger is on the case!
Comment 20 :Ms2ger 2012-02-10 12:06:25 PST
More importantly, mrbkap and bent were on the case, and figured out that khuey's claim that I needed to inherit from nsWrapperCache was bogus.

bjacob, could you write a patch for the WebGL failures at <https://tbpl.mozilla.org/?tree=Try&rev=1d3a2a5e9e0c>?
Comment 21 Benoit Jacob [:bjacob] (mostly away) 2012-02-10 13:56:35 PST
(In reply to Ms2ger from comment #20)
> bjacob, could you write a patch for the WebGL failures at
> <https://tbpl.mozilla.org/?tree=Try&rev=1d3a2a5e9e0c>?

These WebGL failures aren't trivial to debug. They're 3 WebGL test pages timing out, it's not clear why. The typical reason would be that some exception occurred. Need the patch to test locally.
Comment 22 Benoit Jacob [:bjacob] (mostly away) 2012-02-22 12:21:11 PST
Ping! This was on such a good track.

Regarding the WebGL failures, can you give me your current patch so I test locally?

What's the status on the other issues?

Sorry for being pushy, but this is really super important for me (WebGL conformance; 4.5 weeks until the end of the 1.0.1 ratification period, then all browsers must pass 100% or look really bad).
Comment 23 :Ms2ger 2012-02-22 13:24:29 PST
Created attachment 599751 [details] [diff] [review]
Patch v4

Latest patch
Comment 24 Benoit Jacob [:bjacob] (mostly away) 2012-02-22 13:57:17 PST
With this patch, I get an assertion failure in the GC: bug 729680. Marked as invalid bug, because from #jsapi discussion that looks like a bug in the patch:

[16:52] <billm> bjacob: are you sure mData is always initialized?
[16:53] <mccr8> ah okay.  the trace function is the thing in NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN
[16:53] <mccr8> it sounds like mData is uninitialized or was freed already
[16:56] <mccr8> bjacob: the cycle collector gunk looks okay to me otherwise.
Comment 25 Benoit Jacob [:bjacob] (mostly away) 2012-02-22 13:58:23 PST
How to reproduce in GDB:

EXTRA_TEST_ARGS='--debugger=gdb' TEST_PATH=content/canvas/test/webgl/test_webgl_conformance_test_suite.html make -C obj-firefox-debug/ mochitest-plain
Comment 26 Benoit Jacob [:bjacob] (mostly away) 2012-02-22 14:13:38 PST
Anyway, that explains the timeouts. Over to Ms2ger for debugging.
Comment 27 Benoit Jacob [:bjacob] (mostly away) 2012-02-22 15:52:08 PST
Created attachment 599799 [details] [diff] [review]
attempt to use HoldScriptObject, and print debug info

This patch goes on top of Patch v4. It attempts to fix the crash mentioned above, by using HoldScriptObject. It also prints some debug info about ImageData objects.

I get this new crash below. Here is the end of stderr output, and a backtrace:

ImageData ctor 0x2aaac5c85af0, mData=0x2aaabeac22e0
ImageData ctor 0x2aaac5c85c70, mData=0x2aaabeac2340
ImageData ctor 0x2aaac2bf4e20, mData=0x2aaabea76580
ImageData ctor 0x2aaac2bf4ee0, mData=0x2aaabea765e0
ImageData ctor 0x2aaac2f768b0, mData=0x2aaabea76dc0
ImageData ctor 0x2aaac2f76a60, mData=0x2aaabea76e20
TEST-PASS | unknown test url | Successful test page (URL: conformance/canvas/canvas-test.html)
WebGL test page successful: conformance/canvas/canvas-test.html
WebGL mochitest: starting page conformance/canvas/canvas-zero-size.html
NS_IMPL_CYCLE_COLLECTION_TRACE_JS_CALLBACK, tmp=0x2aaac5c85c70, mData=0x2aaabeac2340
NS_IMPL_CYCLE_COLLECTION_TRACE_JS_CALLBACK, tmp=0x2aaac2bf4ee0, mData=0x2aaabea765e0
NS_IMPL_CYCLE_COLLECTION_TRACE_JS_CALLBACK, tmp=0x2aaac2bf4e20, mData=0x2aaabea76580
NS_IMPL_CYCLE_COLLECTION_TRACE_JS_CALLBACK, tmp=0x2aaac2f76a60, mData=0x2aaabea76e20
NS_IMPL_CYCLE_COLLECTION_TRACE_JS_CALLBACK, tmp=0x2aaac2f768b0, mData=0x2aaabea76dc0
NS_IMPL_CYCLE_COLLECTION_TRACE_JS_CALLBACK, tmp=0x2aaac5c85af0, mData=0x2aaabeac22e0
--DOMWINDOW == 13 (0x2aaabee5a878) [serial = 30] [outer = (nil)] [url = http://mochi.test:8888/tests/content/canvas/test/webgl/conformance/canvas/buffer-offscreen-test.html]
ImageData destructor 0x2aaac2f76a60, mData=0x2aaabea76e20
ImageData destructor 0x2aaac2f768b0, mData=0x2aaabea76dc0
ImageData destructor 0x2aaac2bf4ee0, mData=0x2aaabea765e0
ImageData destructor 0x2aaac2bf4e20, mData=0x2aaabea76580
ImageData destructor 0x2aaac5c85c70, mData=0x2aaabeac2340
ImageData destructor 0x2aaac5c85af0, mData=0x2aaabeac22e0

Program received signal SIGSEGV, Segmentation fault.
0x00002aaaae0e21ab in nsXPCOMCycleCollectionParticipant::CheckForRightISupports (this=0x2aaaafa7a300, s=
    0x2aaac5c85c70)
    at /home/bjacob/mozilla-central/obj-firefox-debug/xpcom/build/nsCycleCollectionParticipant.cpp:103
103	                      reinterpret_cast<void**>(&foo));
(gdb) bt
#0  0x00002aaaae0e21ab in nsXPCOMCycleCollectionParticipant::CheckForRightISupports (this=0x2aaaafa7a300, 
    s=0x2aaac5c85c70)
    at /home/bjacob/mozilla-central/obj-firefox-debug/xpcom/build/nsCycleCollectionParticipant.cpp:103
#1  0x00002aaaad204285 in mozilla::dom::ImageData::cycleCollection::Trace (this=0x2aaaafa7a300, p=0x2aaac5c85c70, 
    aCallback=0x2aaaad98614e <CheckParticipatesInCycleCollection(PRUint32, void*, char const*, void*)>, 
    aClosure=0x7fffffff0350) at /home/bjacob/mozilla-central/content/canvas/src/ImageData.cpp:29
#2  0x00002aaaad9861fc in NoteJSHolder (table=0x2aaabcf2ce28, hdr=0x2aaac508d980, number=187, arg=0x7fffffff0350)
    at /home/bjacob/mozilla-central/js/xpconnect/src/XPCJSRuntime.cpp:464
#3  0x00002aaaae6cb0c1 in JS_DHashTableEnumerate (table=0x2aaabcf2ce28, 
    etor=0x2aaaad98619f <NoteJSHolder(JSDHashTable*, JSDHashEntryHdr*, uint32_t, void*)>, arg=0x7fffffff0350)
    at /home/bjacob/mozilla-central/js/src/jsdhash.cpp:745
#4  0x00002aaaad986808 in XPCJSRuntime::AddXPConnectRoots (this=0x2aaabcf2cc00, cx=0x2aaabcf30c00, cb=...)
    at /home/bjacob/mozilla-central/js/xpconnect/src/XPCJSRuntime.cpp:575
#5  0x00002aaaad9526fc in nsXPConnect::BeginCycleCollection (this=0x2aaabc7e4e20, cb=..., 
    explainLiveExpectedGarbage=false) at /home/bjacob/mozilla-central/js/xpconnect/src/nsXPConnect.cpp:601
#6  0x00002aaaae16721d in nsCycleCollector::BeginCollection (this=0x2aaab8a89000, aListener=0x0)
    at /home/bjacob/mozilla-central/xpcom/base/nsCycleCollector.cpp:3157
#7  0x00002aaaae168163 in nsCycleCollectorRunner::Collect (this=0x2aaaabd1ad30, aListener=0x0)
    at /home/bjacob/mozilla-central/xpcom/base/nsCycleCollector.cpp:3953
#8  0x00002aaaae16867a in nsCycleCollector_collect (aListener=0x0)
    at /home/bjacob/mozilla-central/xpcom/base/nsCycleCollector.cpp:4051
#9  0x00002aaaad46688b in nsJSContext::CycleCollectNow (aListener=0x0, aExtraForgetSkippableCalls=0)
    at /home/bjacob/mozilla-central/dom/base/nsJSEnvironment.cpp:3305
#10 0x00002aaaad4572dd in nsDOMWindowUtils::GarbageCollect (this=0x2aaac5488400, aListener=0x0, 
    aExtraForgetSkippableCalls=0) at /home/bjacob/mozilla-central/dom/base/nsDOMWindowUtils.cpp:821
#11 0x00002aaaae171733 in NS_InvokeByIndex_P (that=0x2aaac5488400, methodIndex=22, paramCount=2, 
    params=0x7fffffff8790)
    at /home/bjacob/mozilla-central/xpcom/reflect/xptcall/src/md/unix/xptcinvoke_x86_64_unix.cpp:195
#12 0x00002aaaad9b1627 in CallMethodHelper::Invoke (this=0x7fffffff8750)
    at /home/bjacob/mozilla-central/js/xpconnect/src/XPCWrappedNative.cpp:2881
#13 0x00002aaaad9af46d in CallMethodHelper::Call (this=0x7fffffff8750)
    at /home/bjacob/mozilla-central/js/xpconnect/src/XPCWrappedNative.cpp:2212
#14 0x00002aaaad9af304 in XPCWrappedNative::CallMethod (ccx=..., mode=XPCWrappedNative::CALL_METHOD)
    at /home/bjacob/mozilla-central/js/xpconnect/src/XPCWrappedNative.cpp:2178
#15 0x00002aaaad9bc730 in XPC_WN_CallMethod (cx=0x2aaac78dc400, argc=0, vp=0x2aaabe100508)
    at /home/bjacob/mozilla-central/js/xpconnect/src/XPCWrappedNativeJSOps.cpp:1542
#16 0x00002aaaae750369 in js::CallJSNative (cx=0x2aaac78dc400, 
    native=0x2aaaad9bc4d2 <XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*)>, args=...)
    at /home/bjacob/mozilla-central/js/src/jscntxtinlines.h:311
---Type <return> to continue, or q <return> to quit---
#17 0x00002aaaae737acb in js::InvokeKernel (cx=0x2aaac78dc400, args=..., construct=js::NO_CONSTRUCT)
    at /home/bjacob/mozilla-central/js/src/jsinterp.cpp:499
#18 0x00002aaaae698997 in js::Invoke (cx=0x2aaac78dc400, args=..., construct=js::NO_CONSTRUCT)
    at /home/bjacob/mozilla-central/js/src/jsinterp.h:157
#19 0x00002aaaae6e23f2 in js_fun_apply (cx=0x2aaac78dc400, argc=2, vp=0x2aaabe1004e8)
    at /home/bjacob/mozilla-central/js/src/jsfun.cpp:1692
#20 0x00002aaaae750369 in js::CallJSNative (cx=0x2aaac78dc400, 
    native=0x2aaaae6e2168 <js_fun_apply(JSContext*, unsigned int, JS::Value*)>, args=...)
    at /home/bjacob/mozilla-central/js/src/jscntxtinlines.h:311
#21 0x00002aaaae737acb in js::InvokeKernel (cx=0x2aaac78dc400, args=..., construct=js::NO_CONSTRUCT)
    at /home/bjacob/mozilla-central/js/src/jsinterp.cpp:499
#22 0x00002aaaae743a47 in js::Interpret (cx=0x2aaac78dc400, entryFrame=0x2aaabe100478, 
    interpMode=js::JSINTERP_NORMAL) at /home/bjacob/mozilla-central/js/src/jsinterp.cpp:2694
#23 0x00002aaaae73787f in js::RunScript (cx=0x2aaac78dc400, script=0x2aaac5172438, fp=0x2aaabe100478)
    at /home/bjacob/mozilla-central/js/src/jsinterp.cpp:454
#24 0x00002aaaae737bbc in js::InvokeKernel (cx=0x2aaac78dc400, args=..., construct=js::NO_CONSTRUCT)
    at /home/bjacob/mozilla-central/js/src/jsinterp.cpp:517
#25 0x00002aaaae698997 in js::Invoke (cx=0x2aaac78dc400, args=..., construct=js::NO_CONSTRUCT)
    at /home/bjacob/mozilla-central/js/src/jsinterp.h:157
#26 0x00002aaaae737d9d in js::Invoke (cx=0x2aaac78dc400, thisv=..., fval=..., argc=0, argv=0x2aaabe1003f8, 
    rval=0x7fffffff9bd8) at /home/bjacob/mozilla-central/js/src/jsinterp.cpp:549
#27 0x00002aaaae7acae5 in js::ProxyHandler::call (this=0x2aaaafa80dd0, cx=0x2aaac78dc400, proxy=0x2aaacbc598e0, 
    argc=0, vp=0x2aaabe1003e8) at /home/bjacob/mozilla-central/js/src/jsproxy.cpp:334
#28 0x00002aaaae81a159 in js::Wrapper::call (this=0x2aaaafa80dd0, cx=0x2aaac78dc400, wrapper=0x2aaacbc598e0, 
    argc=0, vp=0x2aaabe1003e8) at /home/bjacob/mozilla-central/js/src/jswrapper.cpp:262
#29 0x00002aaaae81c5fb in js::CrossCompartmentWrapper::call (this=0x2aaaafa80dd0, cx=0x2aaac78dc400, 
    wrapper=0x2aaacbc598e0, argc=0, vp=0x2aaabe1003e8) at /home/bjacob/mozilla-central/js/src/jswrapper.cpp:740
#30 0x00002aaaae7afc36 in js::Proxy::call (cx=0x2aaac78dc400, proxy=0x2aaacbc598e0, argc=0, vp=0x2aaabe1003e8)
    at /home/bjacob/mozilla-central/js/src/jsproxy.cpp:909
#31 0x00002aaaae7b12ec in proxy_Call (cx=0x2aaac78dc400, argc=0, vp=0x2aaabe1003e8)
    at /home/bjacob/mozilla-central/js/src/jsproxy.cpp:1436
#32 0x00002aaaae750369 in js::CallJSNative (cx=0x2aaac78dc400, 
    native=0x2aaaae7b128a <proxy_Call(JSContext*, uintN, JS::Value*)>, args=...)
    at /home/bjacob/mozilla-central/js/src/jscntxtinlines.h:311
#33 0x00002aaaae737a55 in js::InvokeKernel (cx=0x2aaac78dc400, args=..., construct=js::NO_CONSTRUCT)
    at /home/bjacob/mozilla-central/js/src/jsinterp.cpp:492
#34 0x00002aaaae743a47 in js::Interpret (cx=0x2aaac78dc400, entryFrame=0x2aaabe100030, 
    interpMode=js::JSINTERP_NORMAL) at /home/bjacob/mozilla-central/js/src/jsinterp.cpp:2694
#35 0x00002aaaae73787f in js::RunScript (cx=0x2aaac78dc400, script=0x2aaabea699a8, fp=0x2aaabe100030)
---Type <return> to continue, or q <return> to quit---
    at /home/bjacob/mozilla-central/js/src/jsinterp.cpp:454
#36 0x00002aaaae7383c0 in js::ExecuteKernel (cx=0x2aaac78dc400, script=0x2aaabea699a8, scopeChain=..., thisv=..., 
    type=js::EXECUTE_GLOBAL, evalInFrame=0x0, result=0x0) at /home/bjacob/mozilla-central/js/src/jsinterp.cpp:657
#37 0x00002aaaae7385fd in js::Execute (cx=0x2aaac78dc400, script=0x2aaabea699a8, scopeChainArg=..., rval=0x0)
    at /home/bjacob/mozilla-central/js/src/jsinterp.cpp:698
#38 0x00002aaaae674ec2 in EvaluateUCScriptForPrincipalsCommon (cx=0x2aaac78dc400, obj=0x2aaacbc53100, 
    principals=0x2aaabe089308, originPrincipals=0x2aaac4acdb88, chars=0x2aaac63f5108, length=118, 
    filename=0x2aaabeed7648 "http://mochi.test:8888/tests/content/canvas/test/webgl/resources/js-test-post.js", 
    lineno=1, rval=0x0, compileVersion=JSVERSION_DEFAULT) at /home/bjacob/mozilla-central/js/src/jsapi.cpp:5311
#39 0x00002aaaae6750cb in JS_EvaluateUCScriptForPrincipalsVersionOrigin (cx=0x2aaac78dc400, obj=0x2aaacbc53100, 
    principals=0x2aaabe089308, originPrincipals=0x2aaac4acdb88, chars=0x2aaac63f5108, length=118, 
    filename=0x2aaabeed7648 "http://mochi.test:8888/tests/content/canvas/test/webgl/resources/js-test-post.js", 
    lineno=1, rval=0x0, version=JSVERSION_DEFAULT) at /home/bjacob/mozilla-central/js/src/jsapi.cpp:5348
#40 0x00002aaaad4617b8 in nsJSContext::EvaluateString (this=0x2aaac74f82e0, aScript=..., 
    aScopeObject=0x2aaacbc53100, aPrincipal=0x2aaabe089300, aOriginPrincipal=0x2aaac4acdb80, 
    aURL=0x2aaabeed7648 "http://mochi.test:8888/tests/content/canvas/test/webgl/resources/js-test-post.js", 
    aLineNo=1, aVersion=0, aRetValue=0x0, aIsUndefined=0x7fffffffb05e)
    at /home/bjacob/mozilla-central/dom/base/nsJSEnvironment.cpp:1511
#41 0x00002aaaad18aa25 in nsScriptLoader::EvaluateScript (this=0x2aaabc78bc00, aRequest=0x2aaabeed77c0, 
    aScript=...) at /home/bjacob/mozilla-central/content/base/src/nsScriptLoader.cpp:903
#42 0x00002aaaad18a2c5 in nsScriptLoader::ProcessRequest (this=0x2aaabc78bc00, aRequest=0x2aaabeed77c0)
    at /home/bjacob/mozilla-central/content/base/src/nsScriptLoader.cpp:796
#43 0x00002aaaad18ae99 in nsScriptLoader::ProcessPendingRequests (this=0x2aaabc78bc00)
    at /home/bjacob/mozilla-central/content/base/src/nsScriptLoader.cpp:963
#44 0x00002aaaad18bbb9 in nsScriptLoader::OnStreamComplete (this=0x2aaabc78bc00, aLoader=0x2aaabfa172c0, 
    aContext=0x2aaabeed77c0, aStatus=0, aStringLen=118, 
    aString=0x2aaac4acda80 "shouldBeTrue(\"successfullyParsed\");\ndebug('<br /><span class=\"pass\">TEST COMPLETE</span>');\nnotifyFinishedToHarness()\n\245\245\245\245\245\245\245\245\245\245\360q\207\257\252*")
    at /home/bjacob/mozilla-central/content/base/src/nsScriptLoader.cpp:1182
#45 0x00002aaaacad3e7c in nsStreamLoader::OnStopRequest (this=0x2aaabfa172c0, request=0x2aaacbf11858, 
    ctxt=0x2aaabeed77c0, aStatus=0) at /home/bjacob/mozilla-central/netwerk/base/src/nsStreamLoader.cpp:127
#46 0x00002aaaacba8150 in nsHttpChannel::OnStopRequest (this=0x2aaacbf11800, request=0x2aaac4acd980, 
    ctxt=0x2aaabeed77c0, status=0) at /home/bjacob/mozilla-central/netwerk/protocol/http/nsHttpChannel.cpp:4377
#47 0x00002aaaaca9987f in nsInputStreamPump::OnStateStop (this=0x2aaac4acd980)
    at /home/bjacob/mozilla-central/netwerk/base/src/nsInputStreamPump.cpp:583
#48 0x00002aaaaca990ae in nsInputStreamPump::OnInputStreamReady (this=0x2aaac4acd980, stream=0x2aaaccb5a558)
    at /home/bjacob/mozilla-central/netwerk/base/src/nsInputStreamPump.cpp:405
#49 0x00002aaaae12b8bd in nsInputStreamReadyEvent::Run (this=0x2aaabfa0e7c0)
    at /home/bjacob/mozilla-central/xpcom/io/nsStreamUtils.cpp:114
---Type <return> to continue, or q <return> to quit---
#50 0x00002aaaae14d2e7 in nsThread::ProcessNextEvent (this=0x2aaaabd5dc60, mayWait=false, result=0x7fffffffb5bf)
    at /home/bjacob/mozilla-central/xpcom/threads/nsThread.cpp:657
#51 0x00002aaaae0e1f28 in NS_ProcessNextEvent_P (thread=0x2aaaabd5dc60, mayWait=false)
    at /home/bjacob/mozilla-central/obj-firefox-debug/xpcom/build/nsThreadUtils.cpp:245
#52 0x00002aaaadf9e95a in mozilla::ipc::MessagePump::Run (this=0x2aaab8a582c0, aDelegate=0x2aaaabdd58f0)
    at /home/bjacob/mozilla-central/ipc/glue/MessagePump.cpp:110
#53 0x00002aaaae19a1eb in MessageLoop::RunInternal (this=0x2aaaabdd58f0)
    at /home/bjacob/mozilla-central/ipc/chromium/src/base/message_loop.cc:208
#54 0x00002aaaae19a17c in MessageLoop::RunHandler (this=0x2aaaabdd58f0)
    at /home/bjacob/mozilla-central/ipc/chromium/src/base/message_loop.cc:201
#55 0x00002aaaae19a155 in MessageLoop::Run (this=0x2aaaabdd58f0)
    at /home/bjacob/mozilla-central/ipc/chromium/src/base/message_loop.cc:175
#56 0x00002aaaade39ab4 in nsBaseAppShell::Run (this=0x2aaabc775e80)
    at /home/bjacob/mozilla-central/widget/xpwidgets/nsBaseAppShell.cpp:189
#57 0x00002aaaadb7d31c in nsAppStartup::Run (this=0x2aaabc7e6970)
    at /home/bjacob/mozilla-central/toolkit/components/startup/nsAppStartup.cpp:295
#58 0x00002aaaaca45d4f in XRE_main (argc=5, argv=0x7fffffffe258, aAppData=0x423c40)
    at /home/bjacob/mozilla-central/toolkit/xre/nsAppRunner.cpp:3564
#59 0x0000000000402340 in do_main (argc=5, argv=0x7fffffffe258)
    at /home/bjacob/mozilla-central/browser/app/nsBrowserApp.cpp:189
#60 0x0000000000402596 in main (argc=5, argv=0x7fffffffe258)
    at /home/bjacob/mozilla-central/browser/app/nsBrowserApp.cpp:276
Comment 28 :Ms2ger 2012-02-23 01:16:16 PST
Comment on attachment 599799 [details] [diff] [review]
attempt to use HoldScriptObject, and print debug info

Note: can use HoldJSObjects(). Don't we need DropJSObjects() as well?
Comment 29 :Ms2ger 2012-02-23 01:22:02 PST
Oh, and house of card much? It would be great if someone who understood this stuff wrote up some documentation...
Comment 30 :Ms2ger 2012-02-23 11:45:55 PST
Created attachment 600118 [details] [diff] [review]
Patch v5

bjacob, could you try this?
Comment 31 Benoit Jacob [:bjacob] (mostly away) 2012-02-23 12:27:26 PST
No crash! \o/

I get regressions though: 3 test pages that run fine without the patch, time out with the patch:

failed | Unexpected timeout in this test page (URL: conformance/context/context-lost.html)
failed | Unexpected timeout in this test page (URL: conformance/textures/tex-image-and-sub-image-2d-with-image-data.html)
failed | Unexpected timeout in this test page (URL: conformance/textures/tex-image-with-format-and-type.html)

Investigating.
Comment 32 Benoit Jacob [:bjacob] (mostly away) 2012-02-23 12:54:28 PST
We are dying at CustomQS_WebGL.h:427:

    if (argc > 5 &&
        !JSVAL_IS_PRIMITIVE(argv[5]))
    {
        // implement the variants taking a DOMElement as argv[5]
        GET_UINT32_ARG(argv2, 2);
        GET_UINT32_ARG(argv3, 3);
        GET_UINT32_ARG(argv4, 4);

        nsIDOMElement *elt;
        xpc_qsSelfRef eltRef;
        rv = xpc_qsUnwrapArg<nsIDOMElement>(cx, argv[5], &elt, &eltRef.ptr, &argv[5]);
        if (NS_FAILED(rv)) return JS_FALSE;  /// ### !!!! WE ARE DYING HERE!!! #######

        rv = self->TexImage2D_dom(argv0, argv1, argv2, argv3, argv4, elt);

        // NS_ERROR_DOM_SECURITY_ERR indicates we tried to load a cross-domain element, so
        // bail out immediately, don't try to interprete as ImageData
        if (rv == NS_ERROR_DOM_SECURITY_ERR) {
            xpc_qsThrowBadArg(cx, rv, vp, 5);
            return JS_FALSE;
        }

        if (NS_FAILED(rv)) {
            // failed to interprete argv[5] as a DOMElement, now try to interprete it as ImageData
Comment 33 :Ms2ger 2012-02-25 02:14:47 PST
Created attachment 600639 [details] [diff] [review]
Part a: Tighten up support for fake ImageData objects

We've got a couple of functions that try to interpret objects as ImageData; this consolidates the code in one place, and only accepts those objects that have integer width/height and an object data property.
Comment 34 :Ms2ger 2012-02-25 02:17:12 PST
Created attachment 600640 [details] [diff] [review]
Part b: Allow custom quickstubs that return JS::Values

I have no idea why it was done this way, but it breaks my code.
Comment 35 :Ms2ger 2012-02-25 03:58:19 PST
Created attachment 600646 [details] [diff] [review]
Part c: Implement ImageData

Code that actually creates these objects coming in later patches.
Comment 36 :Ms2ger 2012-02-25 04:01:00 PST
Created attachment 600647 [details] [diff] [review]
Part d: Try unwrapping to nsIDOMImageData in GetImageData()
Comment 37 :Ms2ger 2012-02-25 04:39:20 PST
Created attachment 600650 [details] [diff] [review]
Part e: Remove custom quickstub for getImageData and return an actual ImageData
Comment 38 :Ms2ger 2012-02-25 04:51:13 PST
Created attachment 600652 [details] [diff] [review]
Part f: Return real ImageData from CreateImageData
Comment 39 :Ms2ger 2012-02-25 05:35:28 PST
Created attachment 600656 [details] [diff] [review]
Part e: Remove custom quickstub for getImageData and return an actual ImageData

Now with test fix.
Comment 40 :Ms2ger 2012-02-25 05:36:09 PST
Created attachment 600657 [details] [diff] [review]
Part f: Return real ImageData from CreateImageData

And here
Comment 41 Bobby Holley (:bholley) (busy with Stylo) 2012-02-26 21:32:46 PST
Comment on attachment 600640 [details] [diff] [review]
Part b: Allow custom quickstubs that return JS::Values

> I have no idea why it was done this way, but it breaks my code.

Transferring review to jorendorff, since he wrote this code and presumably knows why.
Comment 42 Benoit Jacob [:bjacob] (mostly away) 2012-02-27 14:19:01 PST
Can you please rebase your patches off current m-c? I would love to try them, but don't feel confident resolving some of the conflicts.
Comment 43 :Ms2ger 2012-02-28 02:01:42 PST
You need to apply the patches from bug 730554 first.
Comment 44 Benoit Jacob [:bjacob] (mostly away) 2012-03-01 12:21:37 PST
Great. I made a build with your patches, ran the WebGL conformance tests, and the only remaining test failure around these WebGL tests turned out to be a problem with the test itself, now fixed in the Khronos repository.

Please review and land!
Comment 45 Benoit Jacob [:bjacob] (mostly away) 2012-03-04 19:29:01 PST
If you need to decide which reviews to prioritize, here's a tip: I care about these ones!
Comment 46 Boris Zbarsky [:bz] 2012-03-08 20:17:32 PST
Comment on attachment 600639 [details] [diff] [review]
Part a: Tighten up support for fake ImageData objects

>+++ b/content/canvas/src/CustomQS.h
>+  if (!JS_GetProperty(cx, &dataObject, "width", &temp)) {
....
>+  if (!temp.isInt32()) {

I don't think this is right.  If the imagedata object has a width of 5.0, say, I don't think we should be throwing here, should we?  Seems to me like the old code's handling of "width" via JS_ValueToECMAInt32 was more correct....

>+++ b/content/canvas/src/CustomQS_Canvas2D.h
>-    if (wi <= 0 || hi <= 0)
>-        return xpc_qsThrow(cx, NS_ERROR_DOM_INDEX_SIZE_ERR);

This check seems to have gotten lost, or rather replaced by handling negative values by adding 2^32 to them.  Was that on purpose?  Seems odd.

>@@ -365,41 +335,35 @@ nsIDOMCanvasRenderingContext2D_PutImageD
>+    JSObject* darray;

I believe this is a GC hazard.  The old code rooted the darray, and I think you need to as well.  A JS::Anchor is probably enough.

In particular, consider the codepath where darray is a uint8 array.  We'll grab the tsrc from it, but not anchor it, and then get the typed array data from _that_ and pass it in... so at that point darray and tsrc can both get optimized away and possibly GCed.

Maybe the right thing is to anchor tsrc and just nix tsrc_tvr and not worry about anchoring darray...

>+++ b/content/canvas/src/CustomQS_WebGL.h
>+++ b/content/canvas/src/Makefile.in
>+	CustomQS.h \

Since this is exported, please call it CustomQS_Canvas.h
Comment 47 Boris Zbarsky [:bz] 2012-03-08 20:18:41 PST
Comment on attachment 600646 [details] [diff] [review]
Part c: Implement ImageData

This seems fine, but unwrapping will involve a QI.  Would it make any sense to make this a castable interface?

Maybe it just doesn't matter much given upcoming binding work...

r=me
Comment 48 Boris Zbarsky [:bz] 2012-03-08 20:19:00 PST
Comment on attachment 600647 [details] [diff] [review]
Part d: Try unwrapping to nsIDOMImageData in GetImageData()

>-GetImageData(JSContext* cx, const JS::Value& imageData,
>+GetImageData(JSContext* cx, JS::Value& imageData,

Hmm.  Why is this change needed?  Passing a |const JS::Value&| as a |jsval| (pass by value, note) should work, I'd think.

>+  nsIDOMImageData* domID;

The fact that "id" stands for "imagedata" here is not obvious.  Please just spell it out, perhaps?

r=me with those nits fixed.
Comment 49 Boris Zbarsky [:bz] 2012-03-08 20:19:15 PST
Comment on attachment 600656 [details] [diff] [review]
Part e: Remove custom quickstub for getImageData and return an actual ImageData

>+++ b/content/canvas/src/nsCanvasRenderingContext2D.cpp
>+nsCanvasRenderingContext2D::GetImageDataArray(JSContext* aCx,
>+    JS::AutoObjectRooter rd(aCx, darray);

I don't think you need to root darray here; the stack scanner should handle it.

>+++ b/content/canvas/src/nsCanvasRenderingContext2DAzure.cpp

Same here.  I wish we'd just finish nixing the non-Azure version.....

r=me with the nit fixed.
Comment 50 Boris Zbarsky [:bz] 2012-03-08 20:19:47 PST
Comment on attachment 600657 [details] [diff] [review]
Part f: Return real ImageData from CreateImageData

This is probably fine, but can we get rid of the custom quickstub for createImageData altogether?

Again, may not be worth worrying about pending new bindings.  If you do want to worry about it, please file a followup bug.

r=me either way.
Comment 51 :Ms2ger 2012-03-08 23:26:10 PST
(In reply to Boris Zbarsky (:bz) from comment #46)
> Comment on attachment 600639 [details] [diff] [review]
> Part a: Tighten up support for fake ImageData objects
> 
> >+++ b/content/canvas/src/CustomQS.h
> >+  if (!JS_GetProperty(cx, &dataObject, "width", &temp)) {
> ....
> >+  if (!temp.isInt32()) {
> 
> I don't think this is right.  If the imagedata object has a width of 5.0,
> say, I don't think we should be throwing here, should we?  Seems to me like
> the old code's handling of "width" via JS_ValueToECMAInt32 was more
> correct....

I dunno.. This is already the case where you're passing a fake ImageData object, so we might as well require that you pass in one that matches the interface. Either way works, though.

(In reply to Boris Zbarsky (:bz) from comment #48)
> Comment on attachment 600647 [details] [diff] [review]
> Part d: Try unwrapping to nsIDOMImageData in GetImageData()
> 
> >-GetImageData(JSContext* cx, const JS::Value& imageData,
> >+GetImageData(JSContext* cx, JS::Value& imageData,
> 
> Hmm.  Why is this change needed?  Passing a |const JS::Value&| as a |jsval|
> (pass by value, note) should work, I'd think.

Because xpc_qsUnwrapArg wants a non-const jsval* for its last argument.

(In reply to Boris Zbarsky (:bz) from comment #49)
> Comment on attachment 600656 [details] [diff] [review]
> Part e: Remove custom quickstub for getImageData and return an actual
> ImageData
> 
> >+++ b/content/canvas/src/nsCanvasRenderingContext2D.cpp
> >+nsCanvasRenderingContext2D::GetImageDataArray(JSContext* aCx,
> >+    JS::AutoObjectRooter rd(aCx, darray);
> 
> I don't think you need to root darray here; the stack scanner should handle
> it.

OK. This is all black magic to me...

(In reply to Boris Zbarsky (:bz) from comment #50)
> Comment on attachment 600657 [details] [diff] [review]
> Part f: Return real ImageData from CreateImageData
> 
> This is probably fine, but can we get rid of the custom quickstub for
> createImageData altogether?
> 
> Again, may not be worth worrying about pending new bindings.  If you do want
> to worry about it, please file a followup bug.

And making it take two jsvals in the IDL? I guess that works, and would remove the xpc_qsXPCOMObjectToJsval goop... Filed bug 734334.

Will address your other comments.
Comment 52 Boris Zbarsky [:bz] 2012-03-09 09:32:56 PST
> This is already the case where you're passing a fake ImageData object

How so?  I thought right now we used JS_ValueToECMAInt32?

> Because xpc_qsUnwrapArg wants a non-const jsval* for its last argument.

Ah, ok.
Comment 53 :Ms2ger 2012-03-09 09:36:27 PST
(In reply to Boris Zbarsky (:bz) from comment #52)
> > This is already the case where you're passing a fake ImageData object
> 
> How so?  I thought right now we used JS_ValueToECMAInt32?

Should have phrased this differently; my point was that, if you pass in a duck-typed ImageData, I would be happy to require that it quacks *exactly* like a duck.
Comment 54 Benoit Jacob [:bjacob] (mostly away) 2012-03-09 09:45:03 PST
Should we even accept duck-typed objects at all? Does the spec say something about that?
Comment 55 :Ms2ger 2012-03-09 09:46:05 PST
No; yes; bug 625804.
Comment 56 Boris Zbarsky [:bz] 2012-03-09 09:49:07 PST
You can't require quacking like a JSVAL_IS_INT duck, because that's not under the control of the script.  The JIT uses heuristics for when values should be ints and when they should be doubles, based on other code that's running and so forth.
Comment 57 :Ms2ger 2012-03-09 09:53:13 PST
Ugh. I just found isNumber() / toNumber(), though; does that work for you?
Comment 58 Boris Zbarsky [:bz] 2012-03-09 13:20:50 PST
It'd still fail in situations where a "native" imagedata succeeds.  For example, passing "5" as a width to createImageData and then using the return value would work, but having a duck quacking { width: "5" } would fail.  This is especially a problem in cases where the "5" started out as 5 but then got round-tripped through localStorage or some such...
Comment 59 :Ms2ger 2012-03-10 00:51:05 PST
I'm not terribly convinced, but I'll just implement your preference for this and throwing/wrapping negative numbers. Let me know?
Comment 60 Boris Zbarsky [:bz] 2012-03-10 07:33:51 PST
Sounds good to me.
Comment 61 :Ms2ger 2012-03-10 08:04:24 PST
So, ToInt32 and throw?
Comment 62 Boris Zbarsky [:bz] 2012-03-10 08:09:39 PST
Yeah, let's do that for now.
Comment 63 :Ms2ger 2012-03-10 12:49:09 PST
Created attachment 604680 [details] [diff] [review]
Part a: Tighten up support for fake ImageData objects (v2)
Comment 64 Boris Zbarsky [:bz] 2012-03-10 13:23:26 PST
Comment on attachment 604680 [details] [diff] [review]
Part a: Tighten up support for fake ImageData objects (v2)

> +  if (!JS_GetProperty(cx, &obj, "width", &temp) ||

s/"width"/name/

r=me with that.
Comment 65 :Ms2ger 2012-03-10 13:25:35 PST
Er, yes, thanks.
Comment 66 :Ms2ger 2012-03-12 02:06:17 PDT
jorendorff, I'd appreciate it if you could review somewhere this week.
Comment 67 :Ms2ger 2012-03-15 00:18:53 PDT
I'll land the reviewed patches tomorrow morning.
Comment 68 Jason Orendorff [:jorendorff] 2012-03-15 12:17:16 PDT
Comment on attachment 600640 [details] [diff] [review]
Part b: Allow custom quickstubs that return JS::Values

Review of attachment 600640 [details] [diff] [review]:
-----------------------------------------------------------------

I would rip out all three custom quick stubs, unless bz or peterv thinks they're worthwhile. Premature optimization and all that.

Without the custom quick stub for ImageData.data, this patch isn't necessary, and I would tend toward *not* taking it just because it doesn't actually make the code any shorter (and it makes the generated code slightly longer and less typical of idiomatic JSAPI usage). But r=me if this is deemed necessary--with the one problem below fixed.

::: js/xpconnect/src/XPCQuickStubs.h
@@ +722,5 @@
> +inline bool
> +xpc_qsSameResult(const JS::Value &result1, const JS::Value &result2)
> +{
> +    return result1 == result2;
> +}

Though this will work for the particular case at hand, it isn't really correct. JS_SameValue would be the right thing, and it requires a cx.

I don't mind the rest of the changes--every little bit of sanity helps--but this should be fixed, and the only way I see to fix it is by adding some gross code at the point where we spit out that assertion. :-P
Comment 70 Benoit Jacob [:bjacob] (mostly away) 2012-10-10 07:39:33 PDT
*** Bug 637077 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.