193317 - 100% CPU viewing news w/ a background image

Status: RESOLVED FIXED

(MailNews Core :: Backend, defect)

Description

[[e-mail]]

Spun off from bug 98626.
As bienvenu commented the problem still exists for news.

Comment 1

[Scott MacGregor]

taking

Comment 2

[Joe Sabash [:JoeS1]]

98626 was deemed critical for mail..
The same situation exists for news
There are many newsgroups where tiled
backgrounds are used.
This bug should be considered crital
Joe

Comment 3

[(not reading, please use [e-mail] instead)]

I'm not seeing this hang on win2k, debug, trunk or 1.4 branch bits.

from bug #98626, bienvenu wrote:

> this fix works for imap and local, but not news - news seems to be munging its
> uri's by chopping off the ?part=1.2 kind of thing so the comparison still fails,
> leaving us in the situation we're currently in. The patch is still valid, but we
> need to fix the news code in order to fix this bug for news.

I'll see if something has changed in the news code, that would explain this.

I'm using the 1/23/2003 10:07 post to n.p.m.wishlist
(news://news.mozilla.org:119/b0panv$[e-mail]) to test.

Comment 4

[Scott MacGregor]

Here's the mozilla zine thread talking about this issue:

http://forums.mozillazine.org/viewtopic.php?p=95944#95944

I'm not seeing this using the latest set of builds on Win XP. 

I'm using the following test message:

Joe's post to netscape.test.multimedia on June 14th titled "Background bug
update | by with moz 

Comment 5

[(not reading, please use [e-mail] instead)]

ok, I see the CPU peg.

I was looking for a freeze / hang, not a 100% cpu usage, my fault.

both

snews://secnews.netscape.com:563/bcfktq$[e-mail]
and 
news://news.mozilla.org:119/b0panv$[e-mail]

show the problem.

I'll investigate

Comment 6

[Francis Chin]

I can reproduce the 99% CPU peg with
news://news.mozilla.org:119/b0panv$[e-mail] on 1.4 RC3, but also
on this web page:

http://placenamehere.com/neuralustmirror/200202/fun.php?sheet=13

In both the news and the web page instances, the mouse pointer flickers while
over the page. The key feature appears to be the background image.

This is with:
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.4) Gecko/20030624
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.4b) Gecko/20030611 Mozilla
Firebird/0.6

My system: Duron 1.2GHz, W2K SP3, Matrox G400.

Firebird becomes so unresponsive I can't scroll by clicking on buttons or
scrollbar recess.
Mozilla 1.4 RC3 is also very difficult to scroll, and scrolling auto-repeat
doesn't happen.


On Mozilla 1.4 RC3, there is a memory leak - usage goes from 20M to 30M in one
minute, exceeds 60M four minutes later, and doesn't come back down.
The same leak occurs on Firebird - usage goes from 18M to 25M in one minute.
Going to another HTML page and minimising the browser, CPU usage is still 50%
for a couple of minutes, then it goes back to 0% and memory goes back to 18M.

I don't think this bug is specific to MailNews, so please can the Summary,
Product & Component fields be changed to reflect this?

Comment 7

[Scott MacGregor]

My half ass understanding of what is going on here:


before loading an image url, the image loader checks to see if we are already
loading the url:

  if (mRequest) {
    nsCOMPtr<nsIURI> oldURI;
    mRequest->GetURI(getter_AddRefs(oldURI));
    PRBool eq = PR_FALSE;
    aURI->Equals(oldURI, &eq);
    if (eq) {
      return NS_OK;
    }

mRequest->GetURI turns around and calls GetOriginalURI on the channel loading
the url:

nsresult imgRequest::GetURI(nsIURI **aURI)
{
  if (mChannel)
    return mChannel->GetOriginalURI(aURI);
....
}

For news this url never refers to the part.

When I load the same message under imap, the url contains the part
(?part=1.2&filename=grid100.gif).

This difference causes the image loader to think the urls are different and so
we try to load the image again (and we do so in an infinite loop).

Comment 8

[Scott MacGregor]

Attachment: a possible fix

This patch certainly fixes the problem. I'm just not sure what the
ramifications are of saying NNTP no longer honors Get/SetOriginalURI on a nntp
channel. (Note this code was copied from the imap channel which works fine
without implementing these two methods).

The infinite loop was happening because the image loader asks the channel for
its original URI when determining whether the image has already been loaded.
For imap, we always returned the real url for the image because it does not
support GetOriginalURI. News was supporting this method so the channel was
giving back the actual url used to load the news message and not the specific
background image url. 

I'll try this patch out in my next weekly thunderbird build and we can see what
side effects pop up.

Comment 9

[Scott MacGregor]

This patch has held up well for news in a mini thunderbird respin I did for the
multimedia newsgroup testers.

I believe it is safe to ignore calls to SetOriginalURI for news channels because:
1) SetOriginalURI is primarily used to keep track of redirects, so someone can
always get back to the original URI. That is not a problem for news.

2) We do the same thing for the imap channel and we have not had any problems
because of it. Although Imap is a little different because we create a mock
channel for each individual load request. For news, we just have the one channel
for the connection.

The risk of this fix is that somewhere in the news code we rely on being able to
get the original news article url that was run to load the current message. I
don't believe this is really an issue though from briefly looking at the code. 

I'd like to get this into the trunk for some testing and then maybe have it find
its way into 1.5 final.

Comment 10

[David :Bienvenu]

Comment on attachment 131424 [details] [diff] [review]
a possible fix

+, if you remove the bogus comment in SetOriginalURI.

I don't know if this is going to cause problems or not - my guess is not, but
one thing I'd try is clicking on a link to a message...

Comment 11

[Scott MacGregor]

Checked in. I'm going to mark this as blocking 1.5. IF this bug tests okay after
some trunk testing, then I think we should consider taking it for 1.5. I know I
will be putting it in the thunderbird 0.3 build based on 1.5 final.

Comment 12

[Asa Dotzler [:asa]]

Comment on attachment 131424 [details] [diff] [review]
a possible fix

nominating for 1.5

Comment 13

[Asa Dotzler [:asa]]

Scott, how's this looking on the trunk? We're running out of time on 1.5.

Comment 14

[Scott MacGregor]

Asa: Have not heard of any problems yet that resulted from this change.

Comment 15

[Scott MacGregor]

FYI, fixed in thunderbird 0.3 branch.

Comment 16

[Asa Dotzler [:asa]]

Comment on attachment 131424 [details] [diff] [review]
a possible fix

a=asa (on behalf of drivers) for checkin to the 1.5 branch. Please add the
fixed1.5 keyword when this fix lands on the branch.

Comment 17

[Scott MacGregor]

checked into the 1.5 branch

Comment 18

[David :Bienvenu]

*** Bug 170853 has been marked as a duplicate of this bug. ***