Ignore maxWidth in fillText and strokeText if it is undefined

[Hakerh400]

Fixes #1454

As it turned out that Chrome's behavior is correct, we can now safely update our implementation.

• Have you updated CHANGELOG.md?

[Hakerh400]

The Travis failure is not related to this PR (it also fails on master with the same error as of Node.js v12.6.0 or earlier).

[zbjornson]

Yay bugfix! 👍

I can't repro that crash locally on Linux with Node.js 12.3 or 12.6 :-. Did you repro it locally? Rather than remove that build entirely, could you allow it to fail instead? See this for example: 9e6a610#diff-354f30a63fb0907d4ad57269548329e3

[Hakerh400]

Can't build locally at the moment, so I'm not sure if it would reproduce the crash. But I think it is a bug with cairo pattern allocation/deallocation, which was partially fixed by #1422, but after new V8's GC implementation in Node.js v12.5.0 the order of destructing V8 objects changed, revealing the bug (the bug was probably present earlier too, but didn't consistently cause crash).

[Hakerh400]

Confirmed that the bug is with cairo pattern deallocation.
The following change makes the test succeed (see Travis build):

diff --git a/index.js b/index.js
index 3c9c826..0a4b177 100644
--- a/index.js
+++ b/index.js
@@ -11,8 +11,24 @@ const JPEGStream = require('./lib/jpegstream')
 const DOMMatrix = require('./lib/DOMMatrix').DOMMatrix
 const DOMPoint = require('./lib/DOMMatrix').DOMPoint

+const set = new Set()
+
+CanvasRenderingContext2D.prototype.createLinearGradient = ((f) => function (...args) {
+  const a = f.apply(this, args)
+  set.add(a)
+  return a
+})(CanvasRenderingContext2D.prototype.createLinearGradient)
+
+CanvasRenderingContext2D.prototype.createPattern = ((f) => function (...args) {
+  const a = f.apply(this, args)
+  set.add(a)
+  return a
+})(CanvasRenderingContext2D.prototype.createPattern)
+
 function createCanvas (width, height, type) {
-  return new Canvas(width, height, type)
+  const canvas = new Canvas(width, height, type)
+  set.add(canvas)
+  return canvas
 }

Of course this patch is not a good solution, as it keeps references to all patterns to prevent them from being deallocated. The real issue is, I think, related to canvas resurfacing. It deserves more investigation and a new PR, because it is not relevant to this PR.

[zbjornson]

Ah thanks for digging into that. When I was fixing segfaults a few weeks ago I left some existing code in the destructors that looked wrong and that I expected to cause crashes because for the life of me I couldn't get the destructors to run, and wanted the noisy crashes to reveal if they ever did start running. I'll try to repro it again with that in mind.