Closed Bug 1358053 Opened 4 years ago Closed 4 years ago

Assertion failure: surface->IsDataSourceSurface() (The snapshot SourceSurface from WebGL rendering contest is not DataSourceSurface.), at ImageBitmap.cpp:885

Categories

(Core :: Canvas: WebGL, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox-esr52 --- wontfix
firefox53 --- wontfix
firefox54 - wontfix
firefox55 + fixed

People

(Reporter: cbook, Assigned: daoshengmu)

References

()

Details

(Keywords: assertion, Whiteboard: [gfx-noted])

Attachments

(2 files)

Attached file crash stack
Found via bughunter and reproduced on latest trunk windows debug build 

Steps to reproduce:
-> Load https://speaktogo.withgoogle.com/
--> Assertion failure


Assertion failure: surface->IsDataSourceSurface() (The snapshot SourceSurface from WebGL rendering contest is not                DataSourceSurface.), at /home/worker/workspace/build/src/dom/canvas/ImageBitmap.cpp:885
#01: mozilla::dom::ImageBitmap::Create [dom/canvas/ImageBitmap.cpp:1471]
#02: nsGlobalWindow::CreateImageBitmap [dom/base/nsGlobalWindow.cpp:14699]
#03: mozilla::dom::WindowBinding::createImageBitmap [obj-firefox/dom/bindings/WindowBinding.cpp:15318]
#04: mozilla::dom::WindowBinding::createImageBitmap_promiseWrapper [obj-firefox/dom/bindings/WindowBinding.cpp:15399]
#05: mozilla::dom::WindowBinding::genericPromiseReturningMethod [obj-firefox/dom/bindings/WindowBinding.cpp:15695]
#06: js::CallJSNative [js/src/jscntxtinlines.h:282]
#07: js::InternalCallOrConstruct [js/src/vm/Interpreter.cpp:448]
#08: Interpret [js/src/vm/Interpreter.cpp:2990]
#09: js::RunScript [js/src/vm/Interpreter.cpp:365]
#10: js::InternalCallOrConstruct [js/src/vm/Interpreter.cpp:466]
#11: Interpret [js/src/vm/Interpreter.cpp:2990]
#12: js::RunScript [js/src/vm/Interpreter.cpp:365]
#13: js::InternalCallOrConstruct [js/src/vm/Interpreter.cpp:466]
#14: js::Call [js/src/vm/Interpreter.cpp:512]
#15: PromiseReactionJob [js/src/builtin/Promise.cpp:921]
#16: js::CallJSNative [js/src/jscntxtinlines.h:282]
#17: js::InternalCallOrConstruct [js/src/vm/Interpreter.cpp:448]
#18: js::Call [js/src/vm/Interpreter.cpp:512]
#19: JS::Call [js/src/jsapi.cpp:2887]
#20: mozilla::dom::PromiseJobCallback::Call [obj-firefox/dom/bindings/PromiseBinding.cpp:21]
#21: mozilla::dom::PromiseJobCallback::Call [obj-firefox/dist/include/mozilla/dom/PromiseBinding.h:89]
#22: PromiseJobRunnable::Run [xpcom/base/nsISupportsImpl.h:58]
#23: mozilla::dom::Promise::PerformMicroTaskCheckpoint [dom/promise/Promise.cpp:557]
#24: mozilla::CycleCollectedJSContext::AfterProcessTask [xpcom/base/CycleCollectedJSContext.cpp:1416]
#25: XPCJSContext::AfterProcessTask [js/xpconnect/src/XPCJSContext.cpp:3685]
#26: nsThread::ProcessNextEvent [xpcom/threads/nsThread.cpp:1282]
#27: NS_ProcessNextEvent [xpcom/threads/nsThreadUtils.cpp:389]
#28: mozilla::ipc::MessagePump::Run [ipc/glue/MessagePump.cpp:97]
#29: MessageLoop::RunInternal [ipc/chromium/src/base/message_loop.cc:239]
#30: MessageLoop::Run [ipc/chromium/src/base/message_loop.cc:502]
#31: nsBaseAppShell::Run [widget/nsBaseAppShell.cpp:158]
#32: nsAppStartup::Run [toolkit/components/startup/nsAppStartup.cpp:284]
#33: XREMain::XRE_mainRun [toolkit/xre/nsAppRunner.cpp:4500]
#34: XREMain::XRE_main [toolkit/xre/nsAppRunner.cpp:4677]
#35: XRE_main [toolkit/xre/nsAppRunner.cpp:4768]
#36: do_main [browser/app/nsBrowserApp.cpp:236]
#37: main [browser/app/nsBrowserApp.cpp:309]
#38: libc.so.6 + 0x20830
#39: _start
[Tracking Requested - why for this release]:

daoshengmu, jeff: could you take a look ?
Flags: needinfo?(jgilbert)
Flags: needinfo?(dmu)
Flags: needinfo?(jgilbert) → needinfo?(jgilbert)
A first look for this bug was.

1. DrawTarget with software backend was created in [1] for snapshot.
2. A type check was mismatched in [2]. The check returns true only if the surface type was SurfaceType::DATA or SurfaceType::DATA_SHARED. 

[1]: http://searchfox.org/mozilla-central/rev/7aa21f3b531ddee90a353215bd86e97d6974e25b/dom/canvas/WebGLContext.cpp#1982
[2]: http://searchfox.org/mozilla-central/rev/7aa21f3b531ddee90a353215bd86e97d6974e25b/dom/canvas/ImageBitmap.cpp#872
On Windows, it is a SourceSurfaceSkia and its type is not equal to SurfaceType::DATA or SurfaceType::DATA_SHARED. 

As Comment 2 mentioned, the SourceSurfaceSkia is created by the Skia backend. I am think if we should override IsDataSourceSurface() to allow SourceSurfaceSkia/SourceSurfaceCairo can return true because they derive from DataSourceSurface. I have tried to modify it locally and it works well.

Might Jeff would have more ideas about this.
Flags: needinfo?(dmu)
(In reply to Daosheng Mu[:daoshengmu] from comment #3)
> On Windows, it is a SourceSurfaceSkia and its type is not equal to
> SurfaceType::DATA or SurfaceType::DATA_SHARED. 
> 
> As Comment 2 mentioned, the SourceSurfaceSkia is created by the Skia
> backend. I am think if we should override IsDataSourceSurface() to allow
> SourceSurfaceSkia/SourceSurfaceCairo can return true because they derive
> from DataSourceSurface. I have tried to modify it locally and it works well.
> 
> Might Jeff would have more ideas about this.

I think we should just remove this assert.
Flags: needinfo?(jgilbert)
Whiteboard: [gfx-noted]
Comment on attachment 8862351 [details]
Bug 1358053 - Remove DataSourceSurface assertion when doing snapshot in WebGL;

https://reviewboard.mozilla.org/r/134270/#review137246
Attachment #8862351 - Flags: review?(jgilbert) → review+
Pushed by dmu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/fe62e230c76e
Remove DataSourceSurface assertion when doing snapshot in WebGL; r=jgilbert
https://hg.mozilla.org/mozilla-central/rev/fe62e230c76e
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Seems like we can just let this ride the 55 train?
Assignee: nobody → dmu
Flags: needinfo?(dmu)
(In reply to Ryan VanderMeulen [:RyanVM] from comment #9)
> Seems like we can just let this ride the 55 train?

yep. I think so. This is just an assertion for debugging and will not have any effect on the release build.
Flags: needinfo?(dmu)
Track 54- as this is an assertion for debugging.
Duplicate of this bug: 1453199
Duplicate of this bug: 1461725
You need to log in before you can comment on or make changes to this bug.