Allow to set src=null to clear data

[rochoa]

This still invokes onerror callback if binded, which is usual browser behaviour.

Error is specific for this case, using CAIRO_STATUS_INVALID_CONTENT.
Not sure if error should be CAIRO_STATUS_NULL_POINTER instead.

From Chrome's Developer Tools:

> var img = document.createElement('img')
< undefined
> img.onerror = function handler(evt) { console.log('img onload/onerror handler: %s', evt.type); }
< handler(evt)
> img.src = null
< null
< img onload/onerror handler: error

[LinusU]

Hmm, I don't think that this is quite correct behaviour. Chrome actually tries to load the same image as if you had passed in the string 'null', which works perfectly fine if there is a file called null in the same directory.

Safari on the other hand doesn't do anything when setting the src to null. It doesn't clear the current image, nor does it fire onload/onerror.

I'm not sure where the spec for Image is actually, I would love to find it and than we can follow that one.

This is what I used for testing:

a = new Image()

a.onload = console.log.bind(console, 'onload')
a.onerror = console.log.bind(console, 'onerror')

a.src = null

[LinusU]

Seems like there is a specification but it doesn't really say anything about it... 😭

http://stackoverflow.com/questions/15233483/is-there-a-specification-for-javascript-image-object

Safari actually sets it to an empty string. At least it seems like all browsers coerce the value into a string which seems sensible.

To be honest I would actually prefer a TypeError being thrown if the value passed in is anything but a string, how does that sound?

[rochoa]

I just updated the branch with latest master to remove conflicts.

I didn't realise about Chrome trying to load 'null' 😩! So you are right, my proposal has not the desired/correct behaviour.

I'm OK with your proposal, let me push something for that.

[okcoker]

So I just ran into the issue of Images taking up tons of memory. It seems this PR will still cause an error to be thrown when doing img.src = null. If that's the case, what would be the recommended way to free up memory? Is exposing the clearData method asking too much? Probably, right?

Currently I ignore the error but setting the src to null made a difference of about 6gb of memory in my case (1000s of images). I'd wanna just know that I'm doing the right thing moving forward but setting src = null is my best choice right now it seems.

[LinusU]

Hmm, can't you drop any references to the actual image and then V8 should garbage collect for you?

[okcoker]

Hmm, can't you drop any references to the actual image and then V8 should garbage collect for you?

That's what I would think. I have images being created and written to disk within a fairly large loop. My initial code involved setting img = null but that had no effect. Tried looking through the source code to figure out why and came across the clearData function which led me to img.src = null. I saw instant savings from that as memory was being freed while the loop was executing.

Maybe v8 would normally garbage collect after the loop is done? Is there something that needs to be added in the source code to free memory which doing img = null?

[chearon]

@okcoker have you seen #785? sounds like what you're describing, I remember looking into it a while back and there were definitely some missing free()s in the setters but I never got around to trying to fix it

[okcoker]

@chearon I actually did see that issue but ended up here as a means to find the recommended way of freeing memory. Looking at the other ticket, I think setting the callback handlers to null works even better. I don't need to set img = null or img.src = null as it seems V8 collects memory appropriately.

My code looks something like this:

function getImage(buffer) {
    return new Promise(function(resolve, reject) {
        const img = new Canvas.Image();

        img.onload = function() {
            img.onload = null;
            img.onerror = null;
            resolve(img);
        };

        img.onerror = function(err) {
            img.onload = null;
            img.onerror = null;
            reject(err);
        };

        img.src = buffer;
    });
}

Seems to work pretty well although it still feels like this isn't something one should have to worry about.