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

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

[Carsten Book [:Tomcat]]

Attachment: 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

Comment 1

[Carsten Book [:Tomcat]]

[Tracking Requested - why for this release]:

daoshengmu, jeff: could you take a look ?

Comment 2

[Vincent Liu[:vliu]]

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

Comment 3

[Daosheng Mu[:daoshengmu]]

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.

Comment 4

[Kelsey Gilbert [:jgilbert]]

(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.

Comment 5

[Daosheng Mu[:daoshengmu]]

Attachment: Bug 1358053 - Remove DataSourceSurface assertion when doing snapshot in WebGL;

Review commit: https://reviewboard.mozilla.org/r/134270/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/134270/

Comment 6

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8862351 [details]
Bug 1358053 - Remove DataSourceSurface assertion when doing snapshot in WebGL;

https://reviewboard.mozilla.org/r/134270/#review137246

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fe62e230c76e
Remove DataSourceSurface assertion when doing snapshot in WebGL; r=jgilbert

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/fe62e230c76e

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Seems like we can just let this ride the 55 train?

Comment 10

[Daosheng Mu[:daoshengmu]]

(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.

Comment 11

[Gerry Chang [:gchang]]

Track 54- as this is an assertion for debugging.