294415 - Scrambling bug - loading pages can sometimes become distorted

Status: RESOLVED FIXED

(Camino Graveyard :: Page Layout, defect)

Description

[Echidnae]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b2) Gecko/20050516 Camino/0.8+
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b2) Gecko/20050516 Camino/0.8+

This is the scrambling bug that seems to happen randomly, when loading a page. 
Sometimes a page will load and the contents will appear displaced or
"scrambled".  Some of the time, this will happen during the loading of a page
and will be fixed when the page is finished loading.  On occasion, the page will
remain scrambled when done loading.  Parts of the page can also appear to be
missing entirely.  Scrolling and highlighting parts of the page will return the
page to normal.  This problem can be traced back to the 20050401 nightly, which
is when this problem first seemed to appear.  Hard to reproduce as it seems to
happen randomly.  Happening on both Panther and Tiger.

Reproducible: Sometimes

Steps to Reproduce:
1.  Load a page
2.  During loading, a page may become scrambled and distorted
3.  

Actual Results:  
When done loading, a page may shift back into place, or it may finish load with
the contents stuck as scrambled.

Expected Results:  
The page loads correctly without any scrambling or distortion or shifting.

Comment 1

[Echidnae]

Attachment: screenshot of a scrambled page

Comment 2

[Samuel Sidler (old account; do not CC)]

Confirming... Could be related to bug 294335.

Comment 3

[Simon Fraser [no longer active]]

I heard talk that this first appeared in 1 April builds, in which case it could
be a regression from bug 288369.

Comment 4

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Attachment: Scrambling with widgets

Here's a screenshot from my webmail inbox where form widgets are scrambled (but
I assume any part of the page can be scrambled).

Comment 5

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Scrambling seems "more common" on pages with images whose sizes are not
specified in HTML (phpBB-based fora like Mozillazine), but that's certainly not
the only situation in which scrambling happens.

See also possibly related bug 292291 and bug 293343 in addition to the Google
one in comment 2.  All of these (aside from the Google one AKAIK) happen
sometimes but not others, very inconsistently :-(

Comment 6

[stefan livens]

I can reproduce this scrambling on a certain page 5 out of 5 attempts.
how: load page -> scrambled. reload -> ok. empty cache ->reload->scrambled. and so.
unfortunately: it's a https site (this loads slowlier, so it's easier to see).
i'll try to find an public site.

my opinion (for this page): it's because of images without dimensions. i can see
the green ... that changes to the image, and then it gets scrambled.

it only happened to me after the upgrade to tiger, with the same camino (from
between 20050503 and 20050425: camino on 10.3 no scrambling, same camino on
10.4: scrambling 

Comment 7

[stefan livens]

after some testing I can confirm the regression happened between 20050401
(scrambled) and 20050331 (not scrambled).
camino 084 is not affected.

Comment 8

[Samuel Sidler (old account; do not CC)]

I suppose now I should give the example given to me to confirm which is in
Bugzilla and is *not* related to images, as far as I can tell.

Steps to reproduce:
 * Go to the main bugzilla page (Home)
 * Click on the "Report a Bug" link at the top of the page. Be sure not to click
the advanced version, "Enter a new bug report"
 * Select Camino
 * Scroll down to component
 * Select the "Page Layout" component.
 * Note how the green text to the right changes but cuts off (read: whitens out)
the rest of the line.

Reproducible?
Almost always. Giving this example, I couldn't reproduce, but normally I can. ;)

(Oh the irony)

Comment 9

[stefan livens]

i can see this to.
it helps to use the arrow-keys in selecting the component, some are cut off,
others not.

Comment 10

[stefan livens]

I found an http site, on which I can reproduce the scrambling 4 out 4 attempts.
http://www.microsoft.com/mac/
first load:scrambled->reload:fine. empty cache. reload -> scrambled.
maybe this can be used as testcase

Comment 11

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

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

Comment 12

[Chris Lawson (gone)]

Interesting note from bug 292291 -- turning the cache off (both memory and disk
caching) seems to work around that bug. Can anyone confirm similar behaviour here?

cl

Comment 13

[stefan livens]

I used CEP (Camino Extra Prefs) to disable the cache ( this in my user.js :
user_pref("browser.cache.memory.capacity", 0);
user_pref("browser.cache.memory.enable", false);
user_pref("browser.cache.disk.capacity", 0);
user_pref("browser.cache.disk.enable", false);

When I go to my testcase (the https-site) the end result is good. it's NOT
scrambled. 
But when loading the images and adjusting the layout (images smal and without
proper dimensions) it gets scrambled. but as i said: the end result is good.

Same with the microsoft page: during loading:scrambled. after the whole page is
loaded, everything "locks" into place.

Comment 14

[stefan livens]

when i add this to my user.js :user_pref("nglayout.initialpaint.delay", 0);
it's always scrambled, with or without cache.

interesting: when I move another window on top of the scrambled page, everything
what was underneath this window will turnout OK.

hope all this helps.

Comment 15

[Jasper]

This is exactly what I feared was happening. The whole idea behind the initial
paint delay is to wait for all the content to be fetched and then draw. By
setting it to 0 camino won't wait for it all to be there and will repaint
everytime it get's something new. Chances of something going wrong there are
pretty big as the idea was to keep the delay around 250 (miliseconds I believe)
as we have on default. Please try and setting it back to 250 and see if you
problems go away. I for sure don't have any problems with that default setting.

Comment 16

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

I have never messed with the intial paint delay for that very reason, and I
still see the scrambling bug regularly.  I've not messed with cache or fastback
or pipelining or any other fancy-but-buggy option.  That might explain why I
don't see the scrambling bug as often as others, but it won't explain away the
scrambling bug itself.

While it may exacerbate things, the inital paint delay is as much a "red
herring" about the cause of the actual regression here as animated gifs are in
the slow typing bug (bug 272954 comment 33).

Comment 17

[Simon Fraser [no longer active]]

Attachment: Testcase. Click on the components, and note that the div doesn't correctly update.

Comment 18

[Simon Fraser [no longer active]]

Quartz Debug shows that when we set the <div> contents in the testcase, we
refresh a region which is the size of the old contents, not the new contents.

Comment 19

[Boris Zbarsky [:bzbarsky]]

Simon, any way you can debug what regions get passed to the nsIWidget Invalidate
method here?  Are those also wrong?

Comment 20

[Simon Fraser [no longer active]]

What happens in this testcase is that nsListControlFrame::KeyPress does an
immediate invalidate (using the arrow keys to change the selected item). This
goes down into nsChildView::Update(), which calls NSView's -displayIfNeeded
method. In the process of repainting, we hit the new
EndUpdateViewBatch(NS_VMREFRESH_NO_SYNC);, which causes an invalidate on a child
view of the one being drawn. This invalidate gets "lost".

So Cocoa widget basically fails to handle an invalidate inside a draw (in this
case, on different widgets).

Comment 21

[Simon Fraser [no longer active]]

I'm tempted to suggest that any invalidate during painting is a bad thing and
should be avoided. This is especially true when the invalidate occurs on a
different widget to the one being drawn. What's the expected result? If you end
up with a paint that for some reason always causes an invalidate, you have
runaway painting.

Comment 22

[Boris Zbarsky [:bzbarsky]]

> I'm tempted to suggest that any invalidate during painting is a bad thing and
> should be avoided.

The problem here is that we need to reflow when the draw happens (otherwise we
get nasty flicker).  The reflow triggers invalidates.  If we process them once
the reflow is all done, we get the issue here.  If we put processing them off on
an event, we get bug 288369 (see comments there that basically say that
processing invalidate inside draw is a current requirement for our widget
impls.....)

Perhaps what we should have is widget sending two notifications to the
viewmanager?  WILL_PAINT and PAINT?  The former will process pending reflows and
apply any invalidates they generate; the latter will actually do the painting.

Comment 23

[Simon Fraser [no longer active]]

(In reply to comment #22)

> Perhaps what we should have is widget sending two notifications to the
> viewmanager?  WILL_PAINT and PAINT?  The former will process pending reflows and
> apply any invalidates they generate; the latter will actually do the painting.

That seems like the most sane solution. To work around the current problem in
Cocoa widget, I'd have to add code to Invalidate() to walk up the widget
hierarchy asking if any parents is currently drawing, and, if so, stash the
invalidate rect into a "pending" list for which we do the actual invalidates
after the paint has finished. That gets ugly fast.

Comment 24

[Boris Zbarsky [:bzbarsky]]

There's always the problem, though, that WILL_PAINT can cause synchronous paints
(from inside reflow)...  I think we avoid those as much as possible, but I'm
sure they still happen.

Comment 25

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

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

Comment 26

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Requesting blocking 0.9 because the bug I just duped had it (it matches what
Simon describes in comment 18).

Bug 293343 is similar but backwards from what Simon mentions in comment 18, but
it might yet be a dupe.  The Google doesn't draw until scroll bug 294335 also is
this "backwards" case.

Comment 27

[Mike Pinkerton (not reading bugmail)]

though this really is a core bug....

Comment 28

[michael s. jessop]

You could always add some sort of "soft mutex".  :)  Create a global object that
just sits there and all it does is record who is painting, and whenever you go
to .repaint you can check it first. 

:D  Yeah I'm kidding.

Comment 29

[Julian Pellico]

(In reply to comment #10)
> I found an http site, on which I can reproduce the scrambling 4 out 4 attempts.
> http://www.microsoft.com/mac/
> first load:scrambled->reload:fine. empty cache. reload -> scrambled.
> maybe this can be used as testcase

What's scrambled on the page? The layout all appears great to me. I see a small
issue if the page is wider than the window so that there's a horizontal
scrollbar, if you scroll left and right part of the bar at the top isn't
redrawn, but this is probably the same thing as smfr's test. Very enlightening
test case by smfr.

Comment 30

[Simon Fraser [no longer active]]

Attachment: Delay invalidates if painting (basic patch, probably doesn't apply)

This patch changes the invalidate methods to use
-performSelector:withObject:afterDelay: to postpone invalidates, if there is a
focussed view (i.e. if we are painting). This seems to fix the scrambling
issues.

Comment 31

[Simon Fraser [no longer active]]

Attachment: Full patch, contains some other cleanup

Comment 32

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 186417 [details] [diff] [review]
Full patch, contains some other cleanup

sr=pink

shouldn't we still push on bz to separate invalidates while drawing? Seems like
an ugly thing to have to hack around...

Comment 33

[Mike Pinkerton (not reading bugmail)]

landed simon's patch, nightly from the 16th should have it. yay!!!

Comment 34

[Simon Fraser [no longer active]]

(In reply to comment #32)
> shouldn't we still push on bz to separate invalidates while drawing? Seems like
> an ugly thing to have to hack around...

I looked at doing bz's suggestion of having widget call into the view manager
twice, once with a WILL_PAINT message, and once with a PAINT message. That
wouldn't have helped, because we'd still be calling WILL_PAINT from within
-drawRect: (so code could still do invalidates from inside cocoa drawing), so
that's not a solution.

Comment 35

[Samuel Sidler (old account; do not CC)]

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