Closed Bug 47095 Opened 24 years ago Closed 24 years ago

leaks up 10k after layout/base/src/nsDocumment Viewer Change

Categories

(Core :: Layout, defect, P3)

x86
Windows 95
defect

Tracking

()

VERIFIED FIXED

People

(Reporter: chofmann, Assigned: hyatt)

References

()

Details

Attachments

(1 file)

Tracking bug for a new leak that appeared on tinderbox Friday night.
Lets figure out what to do...

From e-mail thread:

Chris Hofmann wrote: 
  tinderbox reports leaks up 10k after this checkin last night...

Valeski wrote:

Wierd. First glance would make you think leaks would have gone down. 
I suspect that because the doc is no longer registered as an observer, 
it doesn't know when to clean some things up? 
What bug were you guys fixing? I'm inclined to back this out. 

Jud 


This decreases the time to bring up windows.  I'd argue that the speed 
improvement is worth 10k of
leaks.  We should obviously find the leaks, but backing it out (given the small 
size of the leak)
would be overkill IMO. 

dave 

That's 10k, not including network buffers which aren't
getting freed. I say back it out until it works.

Warren

Judson Valeski wrote:

> Judson Valeski wrote:
>
> > 10k of leaks during our stripped down smoketest. Is this per document or
> > per window?
>
> Here are the new leaks introduced as a result of the said change, these look
> like per document leaks to me which scares me.
>
> --NEW-LEAKS--------------------------------
> nsAuthURLParser              12          -
> JPGDecoder                   16          -
> ImageURLImpl                192     700.00%
> NetReaderImpl               128     700.00%
> ImageNetContextImpl         256     700.00%
> ImgDCallbk                  128     700.00%
> ImageConsumer               512     700.00%
> nsFileTransport             980     600.00%
> nsAsyncStreamObserver        140     600.00%
> nsFileIO                    224     600.00%
> nsAsyncStreamListener        168     600.00%
> nsDocumentOpenInfo          392     600.00%
> nsProxyEventObject         2632     600.00%
> nsFileStream                140     600.00%
> nsFileChannel               840     600.00%
> nsProxyObject               392     600.00%
> nsLocalFile                 812     600.00%
> ImageRendererImpl            60     400.00%
> GIFDecoder                   48     200.00%
> nsPipe                      792     200.00%
> nsResChannel                300     200.00%
> nsImageGTK                  288     200.00%
> nsStringKey                3432     120.00%
> nsHashKey                    92     109.09%
> nsLoadGroup                   8     100.00%
> nsDocLoaderImpl             264     100.00%
> nsStdURL                     92      91.67%
> nsStr                      1500      70.45%
> nsVoidArray                 248      47.62%
> nsSupportsArray             560      16.67%
>
> Jud
add url to the diff for the change.
I hate to keep beating this dead horse, but I just noticed that the change in 
the URL was not quite right. The call to BeginObservingDocument was commented 
out, but there is another later call that has to be *uncommented* (the one after 
the test for if(!makeCX). The change should have been:

    // Now make the shell for the document
    rv = mDocument->CreateShell(mPresContext, mViewManager, styleSet,
                                getter_AddRefs(mPresShell));
    NS_RELEASE(styleSet);
    if (NS_FAILED(rv)) return rv;
    // ***COMMENT THIS OUT *** mPresShell->BeginObservingDocument();

      // Initialize our view manager
      nsRect bounds;
      mWindow->GetBounds(bounds);
      nscoord width = bounds.width;
      nscoord height = bounds.height;
      float p2t;
      mPresContext->GetPixelsToTwips(&p2t);
      width = NSIntPixelsToTwips(width, p2t);
      height = NSIntPixelsToTwips(height, p2t);
      mViewManager->DisableRefresh();
      mViewManager->SetWindowDimensions(width, height);

      if (!makeCX) {
        // Make shell an observer for next time
        // *** REMOVE THE COMMENT FROM THE FOLLOWING LINE ***
        mPresShell->BeginObservingDocument();

Without that we probably never observe the document... Anyway, it might be worth 
looking into before (or after) you backout the change.
Cc'ing Edward.
backed out.
Status: NEW → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
David,

I don't think QA can verify this one since it's code related. Can you please mark 
as verified ?
Marking verified in the May 31 build.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: