490535 - Passing a valid CSS font property value to context 2d's font property throws an error

Status: RESOLVED DUPLICATE of bug 571287

(Core :: Graphics: Canvas2D, defect)

Description

[Ben Galbraith]

On Firefox 3.5b4, this code:

var ctx = // context retrieved elsewhere
ctx.font = "9pt Tahoma";

will throw this error:

Error: uncaught exception: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIDOMCanvasRenderingContext2D.font]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: http://localhost:8080/js/th/th.all.js :: anonymous :: line 4499"  data: no]

The error is intermittent but can be reproduced fairly consistently by going to the site in the URL field of this issue, creating an account, logging in and arrived at the dashboard screen, and then playing with the resize bars and generally clicking anywhere that causes a canvas repaint (driven by in-page JS).

NOTE: We will be rolling out a fix to that URL that wraps assignments to ctx.font with a try / catch in the next few days, so ping me if you need another test case.

This behavior did not occur in the previous 3.1/3.5 beta releases nor does it occur in Safari 4.

Comment 1

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

We need to figure out what's going on here for 1.9.1.

Comment 2

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

So I can trigger this, and the NS_ERROR_FAILURE is the one from the start of SetFont, where we do:

nsCOMPtr<nsIContent> content = do_QI(mCanvasElement);
if (!content)
  return NS_ERROR_FAILURE;

mCanvasElement is NULL.

The only way that I can see that this could happen is if you have a reference to a context whose <canvas> element has already been destroyed.  I can see this happening if you do something like:

var canvas = createElement("canvas");
var context = canvas.getContext("2d");

and then let the canvas element itself get GC'd, while still using context.  Because there's GC involved, this is why it's only intermittent.

We should fix this internally to make the context invalid in that case, since you have to make sure the canvas sticks around.  But, we should also check the internal refcounting; currently the context does not hold a strong ref to the canvas element, just a weak ref, so the context by itself won't keep the element around.  Maybe that's bad, though at that point they have to be cycle collection participants and I'll have to figure out how the heck to do that.

(Not a regression -- this same behaviour would be in previous releases, we just didn't have things on the context that explicitly depended on the element being available like SetFont does.)

Comment 3

[Ben Galbraith]

Yup, this is the problem in Bespin. I create a hidden canvas with no parent node to perform text measurements, etc. The canvas goes out of scope rather quickly but the context instance does not. I assumed the context would hold a strong reference to the canvas. Easily fixed for us by keeping the canvas in scope as well.

Comment 4

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

On second thought, this really shouldn't block.  Should still fix it, though.

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Not going to block 1.9.2, but should be fixed. Vlad, who should own this? (And why is "Canvas" under Layout?)

Comment 7

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Looks like this was fixed in bug 571287.