586436 - Fix mistakes in editor/libeditor/html/tests/test_bug520189.html

Status: NEW

(Core :: DOM: Editor, defect)

Description

[(no longer active)]

I landed this fix in bug :

http://hg.mozilla.org/mozilla-central/rev/40ad71a60486

Which apparently caused leaks on Linux Md3:

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1281492510.1281494162.8440.gz

I'm filing this to investigate why the leaks happened, and land the test fixes again.

Comment 1

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Given that the test patch for Bug 572290 causes leaks there's an underlying bug here somewhere, and leaks are bad.

Comment 2

[(no longer active)]

The leak is coming from loading an image which a javascript URI which doesn't return any data (<img src="javascript:void('data');">).  Bug 587902 might help us there.

Joe, do you think you have an idea how an imgRequestProxy (and its friends) are normally deallocated?  I sat down with Jeff and tried to figure this out, but that only caused sweat and tears, and no useful results.

Comment 3

[Joe Drew (not getting mail)]

Bug 587902 won't help; it's an imagelib-internal detail.

Proxies (and requests) are deallocated once they have no more references. References come from:
- the image cache, and are removed when either the imgRequest is cancelled (due to it not being an image, for example), or there are no more users & it's timed out, or when Firefox shuts down; or
- things that call imgLoader::LoadImage, like nsImageLoadingContent, which are cycle collected/etc.

Given that we're leaking both an imgRequest and imgRequestProxy, this implies to me that whoever is loading the image is not calling Cancel on the imgRequestProxy it gets back from LoadImage. If you don't call Cancel, we never break the cycle between Requests and Proxies.

Comment 4

[Benjamin Smedberg]

Is this a new regression? It's bad, and should be fixed, but doesn't sound bad enough to block.

Comment 5

[(no longer active)]

(In reply to comment #4)
> Is this a new regression? It's bad, and should be fixed, but doesn't sound bad
> enough to block.

It's a problem which was uncovered when I landed a test fix.  We have no idea how long it's been happening.

Comment 6

[(no longer active)]

(In reply to comment #3)
> Given that we're leaking both an imgRequest and imgRequestProxy, this implies
> to me that whoever is loading the image is not calling Cancel on the
> imgRequestProxy it gets back from LoadImage. If you don't call Cancel, we never
> break the cycle between Requests and Proxies.

So, what the test does is that it parses something like "<img src='javascript:void("foo");'>" as a document fragment, and pastes it in a document.  Unfortunately I don't know enough about the imagelib APIs to know where Cancel is (supposed to be) called on this image load, but what we know for sure is that this is an image which doesn't have a mimetype and also doesn't have any data.  Joe, would you happen to know what happens to an image load like that?

Comment 7

[Joe Drew (not getting mail)]

From the point of view of imagelib, a totally invalid image load like that's totally fine. It's got to be the thing holding on to the image, i.e., whatever is pasted in.