Closed Bug 550309 Opened 15 years ago Closed 13 years ago

window.ImageData prototype should exist

Categories

(Core :: Graphics: Canvas2D, defect)

Product:
Core
Component:
Graphics: Canvas2D
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla14

People

(Reporter: emk, Assigned: Ms2ger)

References

(
URL
)

Details

(Keywords: dev-doc-complete)

Attachments

(6 files, 9 obsolete files)

Part b: Allow custom quickstubs that return JS::Values
2.93 KB, patch
jorendorff
: review+
Details | Diff | Splinter Review
Part c: Implement ImageData
17.18 KB, patch
bzbarsky
: review+
Details | Diff | Splinter Review
Part d: Try unwrapping to nsIDOMImageData in GetImageData()
1.34 KB, patch
bzbarsky
: review+
Details | Diff | Splinter Review
Part e: Remove custom quickstub for getImageData and return an actual ImageData
26.09 KB, patch
bzbarsky
: review+
Details | Diff | Splinter Review
Part f: Return real ImageData from CreateImageData
5.61 KB, patch
bzbarsky
: review+
Details | Diff | Splinter Review
Part a: Tighten up support for fake ImageData objects (v2)
12.80 KB, patch
bzbarsky
: review+
Details | Diff | Splinter Review
Subject says all.
So what is this bug about, exactly? How are they not exposed?
I believe this is allowing 'new ImageData()' and 'new CanvasPixelArray()'. I don't know if there's any value in having these, though.
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()'.
URL: data:text/html,<script>alert(window.C...
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?
Summary: Expose CanvasPixelArray and ImageData as DOM objects → window.CanvasPixelArray and window.ImageData prototypes should exist
(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.
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.
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.
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.
Blocks: 622842
Blocks: 637077
CanvasPixelArray was removed in favour of Uint8ClampedArray.
Summary: window.CanvasPixelArray and window.ImageData prototypes should exist → window.ImageData prototype should exist
Opera 12, IE 10, Safari 5.1, Chrome 15 have ImageData.prototype Please add it.
Patches welcome.
Assignee: nobody → ehsan
I won't get to this soon realistically...
Assignee: ehsan → nobody
Doing it now: this is blocking our ability to pass the WebGL conformance tests. See bug 637077
Assignee: nobody → bjacob
Attached patch add ImageData interface (obsolete) — — Splinter Review
Attachment #592464 - Flags: feedback?(Ms2ger)
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
Attached patch Patch v2 (obsolete) — — Splinter Review
Comment on attachment 592464 [details] [diff] [review] add ImageData interface f-, crashes ;)
Attachment #592464 - Flags: feedback?(Ms2ger) → feedback-
Attached patch Patch v3 (obsolete) — — Splinter Review
This fails pretty badly on try; any idea what's wrong? https://tbpl.mozilla.org/?tree=Try&rev=da3f5e4f8156
Assignee: bjacob → Ms2ger
Attachment #592464 - Attachment is obsolete: true
Attachment #592497 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #595793 - Flags: feedback?(mrbkap)
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!
Attachment #595793 - Flags: feedback?(mrbkap) → feedback-
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>?
(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.
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).
Attached patch Patch v4 (obsolete) — — Splinter Review
Latest patch
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.
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
Anyway, that explains the timeouts. Over to Ms2ger for debugging.
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 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?
Oh, and house of card much? It would be great if someone who understood this stuff wrote up some documentation...
Attached patch Patch v5 (obsolete) — — Splinter Review
bjacob, could you try this?
Attachment #595793 - Attachment is obsolete: true
Attachment #599751 - Attachment is obsolete: true
Attachment #599799 - Attachment is obsolete: true
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.
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
Depends on: 730554
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.
Attachment #600118 - Attachment is obsolete: true
Attachment #600639 - Flags: review?(bzbarsky)
I have no idea why it was done this way, but it breaks my code.
Attachment #600640 - Flags: review?(bobbyholley+bmo)
Code that actually creates these objects coming in later patches.
Attachment #600646 - Flags: review?(bzbarsky)
Attachment #600652 - Flags: review?(bzbarsky)
Now with test fix.
Attachment #600650 - Attachment is obsolete: true
Attachment #600656 - Flags: review?(bzbarsky)
Attachment #600650 - Flags: review?(bzbarsky)
And here
Attachment #600652 - Attachment is obsolete: true
Attachment #600657 - Flags: review?(bzbarsky)
Attachment #600652 - Flags: review?(bzbarsky)
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.
Attachment #600640 - Flags: review?(bobbyholley+bmo) → review?(jorendorff)
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.
You need to apply the patches from bug 730554 first.
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!
If you need to decide which reviews to prioritize, here's a tip: I care about these ones!
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
Attachment #600639 - Flags: review?(bzbarsky) → review-
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
Attachment #600646 - Flags: review?(bzbarsky) → review+
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.
Attachment #600647 - Flags: review?(bzbarsky) → review+
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.
Attachment #600656 - Flags: review?(bzbarsky) → review+
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.
Attachment #600657 - Flags: review?(bzbarsky) → review+
Depends on: 734334
(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.
> 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.
(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.
Should we even accept duck-typed objects at all? Does the spec say something about that?
No; yes; bug 625804.
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.
Ugh. I just found isNumber() / toNumber(), though; does that work for you?
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...
I'm not terribly convinced, but I'll just implement your preference for this and throwing/wrapping negative numbers. Let me know?
Sounds good to me.
So, ToInt32 and throw?
Yeah, let's do that for now.
Attachment #600639 - Attachment is obsolete: true
Attachment #604680 - Flags: review?(bzbarsky)
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.
Attachment #604680 - Flags: review?(bzbarsky) → review+
Er, yes, thanks.
jorendorff, I'd appreciate it if you could review somewhere this week.
I'll land the reviewed patches tomorrow morning.
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
Attachment #600640 - Flags: review?(jorendorff) → review+
The custom quickstub didn't make a difference in speed, so I left it out. https://hg.mozilla.org/mozilla-central/rev/d2d5ffdd5ef6 https://hg.mozilla.org/mozilla-central/rev/79b86334d0fe https://hg.mozilla.org/mozilla-central/rev/66bed1bee130 https://hg.mozilla.org/mozilla-central/rev/64daf7f88b87 https://hg.mozilla.org/mozilla-central/rev/fdaa44e6f85e
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Keywords: dev-doc-needed
OS: Windows 7 → All
Hardware: x86 → All
Resolution: --- → FIXED
Target Milestone: --- → mozilla14
Depends on: 736752
Depends on: 743615
Depends on: 746773
Depends on: 792561
Depends on: 805210
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: