[canvaskit] Fix freeing of memory in computeTonalColors

Change-Id: I82317cf762362a4fb74933d7d7145555d397c850
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/294700
Reviewed-by: Nathaniel Nifong <nifong@google.com>

modules/canvaskit/CHANGELOG.md

@@ -9,7 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Fixed
  - A bug where loading fonts (and other memory intensive calls) would cause CanvasKit
    to infrequently crash with
-   `TypeError: Cannot perform %TypedArray%.prototype.set on a neutered ArrayBuffer`
+   `TypeError: Cannot perform %TypedArray%.prototype.set on a neutered ArrayBuffer`.
+ - Incorrectly freeing Malloced colors passed into computeTonalColors.
 
 ## [0.16.1] - 2020-06-04
 

modules/canvaskit/helper.js

@@ -739,7 +739,7 @@ CanvasKit.Free = function(mallocObj) {
 // This helper will free the given pointer unless the provided array is one
 // that was returned by CanvasKit.Malloc.
 function freeArraysThatAreNotMallocedByUsers(ptr, arr) {
-  if (!arr || !arr['_ck']) {
+  if (!arr['_ck']) {
     CanvasKit._free(ptr);
   }
 }

modules/canvaskit/interface.js

@@ -1201,7 +1201,7 @@ CanvasKit.onRuntimeInitialized = function() {
                                                   colors.length, mode, flags, localMatrixPtr, colorSpace);
 
     CanvasKit._free(colorPtr);
-    freeArraysThatAreNotMallocedByUsers(posPtr, pos);
+    pos && freeArraysThatAreNotMallocedByUsers(posPtr, pos);
     return lgs;
   }
 
@@ -1216,7 +1216,7 @@ CanvasKit.onRuntimeInitialized = function() {
                                                   colors.length, mode, flags, localMatrixPtr, colorSpace);
 
     CanvasKit._free(colorPtr);
-    freeArraysThatAreNotMallocedByUsers(posPtr, pos);
+    pos && freeArraysThatAreNotMallocedByUsers(posPtr, pos);
     return rgs;
   }
 
@@ -1235,7 +1235,7 @@ CanvasKit.onRuntimeInitialized = function() {
                                                  localMatrixPtr, colorSpace);
 
     CanvasKit._free(colorPtr);
-    freeArraysThatAreNotMallocedByUsers(posPtr, pos);
+    pos && freeArraysThatAreNotMallocedByUsers(posPtr, pos);
     return sgs;
   }
 
@@ -1252,7 +1252,7 @@ CanvasKit.onRuntimeInitialized = function() {
                           colorPtr, posPtr, colors.length, mode, flags, localMatrixPtr, colorSpace);
 
     CanvasKit._free(colorPtr);
-    freeArraysThatAreNotMallocedByUsers(posPtr, pos);
+    pos && freeArraysThatAreNotMallocedByUsers(posPtr, pos);
     return rgs;
   }
 
@@ -1275,17 +1275,23 @@ CanvasKit.onRuntimeInitialized = function() {
 //    ambient: {r, g, b, a},
 //    spot: {r, g, b, a},
 // }
-// Returns the same format
+// Returns the same format. Note, if malloced colors are passed in, the memory
+// housing the passed in colors passed in will be overwritten with the computed
+// tonal colors.
 CanvasKit.computeTonalColors = function(tonalColors) {
+    // copy the colors into WASM
     var cPtrAmbi = copyColorToWasmNoScratch(tonalColors['ambient']);
     var cPtrSpot = copyColorToWasmNoScratch(tonalColors['spot']);
+    // The output of this function will be the same pointers we passed in.
     this._computeTonalColors(cPtrAmbi, cPtrSpot);
+    // Read the results out.
     var result =  {
       'ambient': copyColorFromWasm(cPtrAmbi),
       'spot': copyColorFromWasm(cPtrSpot),
     }
-    freeArraysThatAreNotMallocedByUsers(cPtrAmbi);
-    freeArraysThatAreNotMallocedByUsers(cPtrSpot);
+    // If the user passed us malloced colors in here, we don't want to clean them up.
+    freeArraysThatAreNotMallocedByUsers(cPtrAmbi, tonalColors['ambient']);
+    freeArraysThatAreNotMallocedByUsers(cPtrSpot, tonalColors['spot']);
     return result;
 }
 

modules/canvaskit/tests/core.spec.js

@@ -66,6 +66,25 @@ describe('Core canvas behavior', () => {
         expect(out.spot[3]).toBeCloseTo(expectedSpot[3], 3);
     });
 
+    it('can compute tonal colors with malloced values', () => {
+        const ambientColor = CanvasKit.Malloc(Float32Array, 4);
+        ambientColor.toTypedArray().set(CanvasKit.BLUE);
+        const spotColor = CanvasKit.Malloc(Float32Array, 4);
+        spotColor.toTypedArray().set(CanvasKit.RED);
+        const input = {
+            ambient: ambientColor,
+            spot: spotColor,
+        };
+        const out = CanvasKit.computeTonalColors(input);
+        expect(new Float32Array(out.ambient)).toEqual(CanvasKit.BLACK);
+        const expectedSpot = [0.173, 0, 0, 0.969];
+        expect(out.spot.length).toEqual(4);
+        expect(out.spot[0]).toBeCloseTo(expectedSpot[0], 3);
+        expect(out.spot[1]).toBeCloseTo(expectedSpot[1], 3);
+        expect(out.spot[2]).toBeCloseTo(expectedSpot[2], 3);
+        expect(out.spot[3]).toBeCloseTo(expectedSpot[3], 3);
+    });
+
     // This helper is used for all the MakeImageFromEncoded tests.
     // TODO(kjlubick): rewrite this and callers to use gm
     function decodeAndDrawSingleFrameImage(imgName, goldName, done) {