The Canvas debugger should handle cases where there are no animation loop

RESOLVED FIXED in Firefox 39

Status

defect
RESOLVED FIXED
5 years ago
4 months ago

People

(Reporter: pbro, Assigned: jsantell)

Tracking

unspecified
Firefox 39
Dependency tree / graph

Firefox Tracking Flags

(firefox39 fixed)

Details

Attachments

(2 attachments, 2 obsolete attachments)

STR:

- Go to http://codepen.io/captainbrosset/full/lHpnK
- Open the canvas debugger
- Hit the record button

==> A snapshot appears, with the label "loading..." that never goes away
==> The record button remains in active state
==> Clicking on the snapshot produces errors in the console:

*************************
A coding exception was thrown and uncaught in a Task.

Full message: TypeError: screenshot is null
Full stack: CallsListView<.showScreenshot@chrome://browser/content/devtools/canvasdebugger.js:697:1
SnapshotsListView<._onSelect/<@chrome://browser/content/devtools/canvasdebugger.js:324:7
TaskImpl_run@resource://gre/modules/Task.jsm:282:1
resolve@resource://gre/modules/commonjs/sdk/core/promise.js:118:11
then@resource://gre/modules/commonjs/sdk/core/promise.js:43:43
resolve@resource://gre/modules/commonjs/sdk/core/promise.js:185:11
setTimeout_timer@resource://gre/modules/Timer.jsm:30:5

*************************

We should somehow find a way to handle these cases and let the user know there's no animation in the page.
Depends on: 917226
See Also: → 978948
I think some things would help this:

* Change the message "Loading..." to something like "Waiting for a requestAnimationFrame call", which indicates that rAF is needed for this, as well as that the tools aren't "loading" anything.
* After 5-10s, stop the actor (need a new method on the actor), and display a message saying "no requestAnimationFrame loops found" or something.

This clarifies the restrictions, and explains how it works a bit.

Thoughts?
Flags: needinfo?(vporof)
Assignee: nobody → jsantell
Status: NEW → ASSIGNED
Flags: needinfo?(vporof)
Here's a little demo of what I got cookin'.
Depends on: 1122766
Sorry that this is huge, Victor.
Attachment #8566889 - Flags: review?(vporof)
The text for this should change in lieu of bug 978948, supporting setTimeout.

Should we explicitly say "waiting for requestAnimationFrame/setTimeout cycle"? Or just not mention setTimeout at all? Two messages need to be considered, the "recording" state, as well as the "failed" state, which can contain more debugging information (Detected no setTimeout/rAF cycles, maybe make the timeout a pref and mention to increase that if needed?)
Comment on attachment 8566889 [details] [diff] [review]
985488-canvas-debugger-refactor.patch

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

NIIIICE

::: browser/devtools/canvasdebugger/callslist.js
@@ +1,1 @@
> +/**

use strict.

::: browser/devtools/canvasdebugger/snapshotslist.js
@@ +1,1 @@
> +/**

use strict?

@@ +94,5 @@
> +
> +  /**
> +   * Removes the last snapshot added, in the event no requestAnimationFrame loop was found.
> +   */
> +  removeSnapshot: function () {

s/removeSnapshot/removeLastSnapshot/

::: browser/devtools/canvasdebugger/test/doc_no-canvas.html
@@ +8,5 @@
> +    <title>Canvas inspector test page</title>
> +  </head>
> +
> +  <body>
> +  </body>

Would be cool to have a few tests where there's a setTimeout and/or rAF and no canvas as well. Ideally, this case will still show 'waiting for a canvas etc..'

::: browser/devtools/canvasdebugger/test/head.js
@@ +39,5 @@
>  waitForExplicitFinish();
>  
>  let gToolEnabled = Services.prefs.getBoolPref("devtools.canvasdebugger.enabled");
>  
> +gDevTools.testing = true;

I giggle every time I see this.
Attachment #8566889 - Flags: review?(vporof) → review+
Attachment #8566889 - Attachment is obsolete: true
Attachment #8567545 - Flags: review+
Blocks: 978948
Made bug 1135403 to check the scenario of rAF/sT existing but no canvas/render calls
No longer blocks: 978948
Depends on: 978948
rebased
Attachment #8567545 - Attachment is obsolete: true
Attachment #8568340 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/acf0699ae844
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 39
Product: Firefox → DevTools
Product: DevTools → DevTools Graveyard
You need to log in before you can comment on or make changes to this bug.