From kfm-devel Tue Aug 31 16:48:02 1999 From: Waldo Bastian Date: Tue, 31 Aug 1999 16:48:02 +0000 To: kfm-devel Subject: KDE1.1.2/PATCH Was: Bug#1812: KFM memory leak: updated patch for khtmlw. X-MARC-Message: https://marc.info/?l=kfm-devel&m=93611880720817 MIME-Version: 1 Content-Type: multipart/mixed; boundary="--Boundary-=_XiqVKuYtDLcgDewHTacIOHORdtqK" --Boundary-=_XiqVKuYtDLcgDewHTacIOHORdtqK Content-Type: text/plain Content-Transfer-Encoding: 8bit On Tue, 31 Aug 1999, Bjarni R. Einarsson wrote: > On Tue, Aug 31, 1999 at 05:53:30PM +0200, Waldo Bastian wrote: > > On Tue, 31 Aug 1999, you wrote: > > > Sorry if this doesn't thread correctly, I'm pasting from > > > http://www.lists.kde.org/, since I'm not actually subscribed to kfm-devel > > > (I'm just a power user, not a KDE developer). :-) > > > > No problem. Do you want CC's? > > Yes please. That makes quoting much easier. :-) > > I understand your resistance to my patch - it's a bit bigger than you'd > like, right before release time, and you aren't sure what it is fixing. > You want to know which is doing what, I guess. Yes. > So... my patch explained: > > The patch implements the following changes. > > 1. pCache cleanup code > 2. cosmetics > 3. A GIF/Gif98a typo fixed > 4. The movieCache one-liner. > 5. Minor paranoia. > > 1. Consists of the following changes: > > a) A global definition (PCACHESIZE) replacing the constant 503. > > b) Use of pCache->setAutoDelete(TRUE) where appropriate. > A destructor for the HTMLCachedImage type, which deletes the pixmap. > Brute force cache-flushing, via pCache->count() and pCache->clear() > > c) The "cached" boolean variable in the HTMLImage type is ignored, > destroying the pixmap whether it is cached or not. This is a good > thing, because the pixmap is already implicitly shared by Qt, and > /must/ always be deleted by this destructor, for b) to work. > This means the "cached" variable isn't really used. > > d) I added checks of whether pixmap was actually unassigned before > calling pixmap = new QPixmap. > > I think that if you're correct that the pCache is never used on web pages, > that d) or c) might be the one-liner you are looking for. I just discovered it is c) :-) > But.. I feel compelled to harp on this some more - how can you be sure that > everybody uses cacheImage() the right way? The changes in b) are very > straightforward, and should cause no adverse effects other than a minor > performance hit - if that. cacheImage() is available to anyone using the > HTML widget, so fixing this is protecting us from abuse by future KDE 1.1.x > compatible programs which may not "do the right thing", no matter what kfm > is doing today. > > 2. Is just in the way. > > 3. Makes handling of GIF images consistant throughout the program, only > implementing movie-related stuff on GIF89a images. > > 4. The one-liner everyone seems happy with. :-) > > 5. I changed a "--refcount == 0" comparison to "--refcount <= 0", just > to be on the safe side. It probably doesn't help, probably doesn't > matter, but it made me feel better. Yes this does help. I have attached a patch that consists of 1c) and 4. If you can check that this patch gives the same test-results as patch7 then we should apply this. That solves bug 1812 I think. Then we can have a look at the pCache stuff. It would be nice if we could fix it, but it isn't very urgent. It is clear that it leaks but not very hard. I think it is worse if we change something there and break something. I have commited 5) since it doesn't hurt. Is 3) related to a bug? Cheers, Waldo --Boundary-=_XiqVKuYtDLcgDewHTacIOHORdtqK Content-Type: text/x-c++; name="kdelibs.khtmlw.31-08-99.diff2" Content-Transfer-Encoding: base64 Content-Disposition: attachment; filename="kdelibs.khtmlw.31-08-99.diff" SW5kZXg6IGh0bWxvYmouY3BwCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KUkNTIGZpbGU6IC9ob21lL2tkZS9rZGVsaWJz L2todG1sdy9BdHRpYy9odG1sb2JqLmNwcCx2CnJldHJpZXZpbmcgcmV2aXNpb24gMS42OQpkaWZm IC11IC1yMS42OSBodG1sb2JqLmNwcAotLS0gaHRtbG9iai5jcHAJMTk5OS8wMS8xOCAxMDo1Nzo1 NAkxLjY5CisrKyBodG1sb2JqLmNwcAkxOTk5LzA4LzMxIDE2OjU3OjE3CkBAIC0xMzA0LDggKzEz MDQsNiBAQAogICAgIHVybCA9IF91cmw7CiAgICAgdGFyZ2V0ID0gX3RhcmdldDsKICAgICAKLSAg ICBjYWNoZWQgPSBUUlVFOwotCiAgICAgcHJlZGVmaW5lZFdpZHRoID0gKCBfd2lkdGggPCAwICYm ICFfcGVyY2VudCApID8gZmFsc2UgOiB0cnVlOwogICAgIHByZWRlZmluZWRIZWlnaHQgPSBfaGVp Z2h0IDwgMCA/IGZhbHNlIDogdHJ1ZTsKIApAQCAtMTQzMSw3ICsxNDI5LDYgQEAKICAgICBlbHNl CiAgICAgewogICAgICAgcGl4bWFwLT5sb2FkKCB1LnBhdGgoKSApOwotICAgICAgY2FjaGVkID0g ZmFsc2U7CiAgICAgfQogCiAgICAgYkNvbXBsZXRlID0gdHJ1ZTsKQEAgLTE0ODcsNyArMTQ4NCw2 IEBACiAgIHsKICAgICBwaXhtYXAgPSBuZXcgUVBpeG1hcCgpOwogICAgIHBpeG1hcC0+bG9hZEZy b21EYXRhKCBfYnVmZmVyLmJ1ZmZlcigpICk7CSAgICAKLSAgICBjYWNoZWQgPSBmYWxzZTsKIAog ICAgIGlmICggcGl4bWFwID09IDAgfHwgcGl4bWFwLT5pc051bGwoKSApCiAgICAgICByZXR1cm4g ZmFsc2U7CkBAIC0xNTU2LDcgKzE1NTIsNiBAQAogICAgIHsKIAlwaXhtYXAgPSBuZXcgUVBpeG1h cCgpOwogCXBpeG1hcC0+bG9hZCggX2ZpbGVuYW1lICk7CSAgICAKLQljYWNoZWQgPSBmYWxzZTsK IAogCWlmICggcGl4bWFwID09IDAgfHwgcGl4bWFwLT5pc051bGwoKSApCiAJICAgIHJldHVybjsK QEAgLTE4MTYsOCArMTgxMSwxMCBAQAogICAgIC8vIGlmICggIWltYWdlVVJMLmlzRW1wdHkoKSAm JiAhcGl4bWFwICkKICAgICAvLyBodG1sV2lkZ2V0LT5jYW5jZWxSZXF1ZXN0RmlsZSggdGhpcyAp OwogCi0gICAgaWYgKCBwaXhtYXAgJiYgIWNhY2hlZCApCisgICAgaWYgKCBwaXhtYXAgKQogCWRl bGV0ZSBwaXhtYXA7CisgICAgaWYgKCBtb3ZpZUNhY2hlICkKKyAgICAgICAgZGVsZXRlIG1vdmll Q2FjaGU7CiAjaWZkZWYgVVNFX1FNT1ZJRQogICAgIGlmICggbW92aWUgKQogICAgIHsK --Boundary-=_XiqVKuYtDLcgDewHTacIOHORdtqK--