1122766 - Canvas debugger doesn't work when recordAnimationFrame doesn't pick up draw calls within 1 frame

Status: RESOLVED FIXED

(DevTools Graveyard :: Canvas Debugger, defect)

Description

[Marco Castelluccio [:marco]]

I've tried to collect a trace, I'm seeing the following error in the browser console:

> A promise chain failed to handle a rejection. Did you forget to '.catch', or did you forget to 'return'?
> See https://developer.mozilla.org/Mozilla/JavaScript_code_modules/Promise.jsm/Promise
> 
> Date: Fri Jan 16 2015 23:34:52 GMT+0100 (ora solare Europa occidentale)
> Full Message: TypeError: functionCalls is null
> Full Stack: CallsListView<.showCalls@chrome://browser/content/devtools/canvasdebugger.js:614:21
> SnapshotsListView<._onSelect/<@chrome://browser/content/devtools/canvasdebugger.js:329:7
> TaskImpl_run@resource://gre/modules/Task.jsm:314:40
> Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:870:23
> this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:749:7
> this.PromiseWalker.scheduleWalkerLoop/<@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:691:37

Comment 1

[Victor Porof [:vporof][:vp]]

STR?

Comment 2

[Marco Castelluccio [:marco]]

1) Open the canvas debugger
2) Start recording the trace

You can't stop recording

Comment 3

[Victor Porof [:vporof][:vp]]

Are you sure this isn't a dupe of 978948 or bug 981748? What website are you testing on?

Comment 4

[Marco Castelluccio [:marco]]

I've tested on a few different pages, one does indeed have a canvas in an iframe.

This demo page doesn't work too, and it doesn't seem to be using an iframe or setTimeout for animating: http://s.codepen.io/boomerang/e551c185f0be13ed515944918218dff01421453998881/index.html

Comment 5

[Victor Porof [:vporof][:vp]]

Hmm, that page is empty for me.

Comment 6

[Marco Castelluccio [:marco]]

http://codepen.io/matt-west/pen/bGdEC (then open the frame in another tab)

Comment 7

[Victor Porof [:vporof][:vp]]

If I do that, after reloading the page from the canvas debugger, it's blank.

Comment 8

[Jordan Santell [:jsantell] [@jsantell]]

Attachment: canvasbug.html

Ported codepen to html file, when recording, and starting the animation on this file, get the following error, causing canvas debugger to stop working altogether:

 Error writing response to: getOverview
console.error: 
  Message: Error: undefined passed where a value is required
  Stack:
    identityWrite@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/protocol.js:116:1
types.addDictType/<.write@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/protocol.js:234:23
types.addDictType/<.write@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/protocol.js:234:23
RetVal<.write@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/protocol.js:530:12
Response<.write/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/protocol.js:683:1
Response<.write@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/protocol.js:681:23
actorProto/</handler/sendReturn@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/protocol.js:1016:24
Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:867:23
this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:746:7
this.PromiseWalker.scheduleWalkerLoop/<@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:688:37

identityWrite@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/protocol.js:116:1
types.addDictType/<.write@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/protocol.js:234:23
types.addDictType/<.write@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/protocol.js:234:23
RetVal<.write@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/protocol.js:530:12
Response<.write/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/protocol.js:683:1
Response<.write@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/protocol.js:681:23
actorProto/</handler/sendReturn@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/protocol.js:1016:24
Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:867:23
this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:746:7
this.PromiseWalker.scheduleWalkerLoop/<@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:688:37
console.error: 
  Protocol error (unknownError): Error: undefined passed where a value is required

Comment 9

[Jordan Santell [:jsantell] [@jsantell]]

This is due to requestAnimationFrame being called when the recording starts, and then rAF is called at the start of the `animate` function, resulting in an empty rendering cycle. This is also broken in Fx35.

Comment 10

[Jordan Santell [:jsantell] [@jsantell]]

How common are the scenarios where we have a rAF cycle start at the beginning of a function? Seems really strange, but if it's common, it's something we should deal with. Would this skip the first (or any) empty cycle? I can do this patch, but want more insight on a smart way to handle this potentially common scenario.

Comment 11

[Jordan Santell [:jsantell] [@jsantell]]

Attachment: 1122766-canvas-raf.patch

This keeps checking rAF until a draw call is found. In fixes the scenario linked where a loop is like:

```
let animate = () => {
  rAF(animate);
  /* render */
}
rAF(animate);
```

Where we have 2 rAF calls without any drawing. This will continue looping indefinitely, like all other invalid recordings, but that is hope to be fixed in bug 985488

 https://treeherder.mozilla.org/#/jobs?repo=try&revision=ac8ca767978f

Comment 12

[Victor Porof [:vporof][:vp]]

Comment on attachment 8566351 [details] [diff] [review]
1122766-canvas-raf.patch

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

::: browser/devtools/canvasdebugger/test/browser_canvas-actor-test-04.js
@@ +21,5 @@
>    ok(snapshotActor,
>      "A snapshot actor was sent after recording.");
>  
>    let animationOverview = yield snapshotActor.getOverview();
> +  ok(animationOverview,

!

::: toolkit/devtools/server/actors/canvas.js
@@ +296,5 @@
> +   */
> +  isRecording: method(function() {
> +    return !!this._callWatcher.isRecording();
> +  }, {
> +    response: { recording: RetVal("boolean") } 

whitespace

Comment 13

[Jordan Santell [:jsantell] [@jsantell]]

Attachment: 1122766-canvas-raf.patch

nits fixed

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/3310c2450553

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/3310c2450553