401328 - when opening new tab, small white rectangle on top left of page

Status: RESOLVED FIXED

(Camino Graveyard :: Tabbed Browsing, defect, P4)

Description

[philippe (part-time)]

STR
1. Open page with dark(er) background.
2. command T to open new tab
3. a small white background flashes briefly a the top left of the page (content).

Regressed very recently (I had first seen that yesterday with a home made build, pull time 20071025 ~ 20:30 tinderbox time - just before Josh checked-in bug 401184)

Now with Version 2007102617 (2.0a1pre) - PPC 10.4.10

Comment 1

[philippe (part-time)]

Apparently, this is caused by the patch in bug 362549. I backed it out locally, and no more little rectangle.

The patch in bug 362549 did not fix the issue  (bleeding into loading tabs) as far as Camino is concerned, btw.

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Someone running Camino needs to debug this. It shouldn't be hard, set a breakpoint in the code I added in bug 362549 and look at the state of the Cocoa view tree.

I think it must be some latent bug that's being exposed now, because the patch in bug 362549 is pretty clearly correct.

Comment 3

[philippe (part-time)]

When opening a link from within an email client (Mail.app here), the small rectangle is also visible, the whole time, until the page has loaded in its own tab.

Comment 4

[Stuart Morgan]

I've played with this some, and I noticed that the rectangle has exactly the aspect ratio of the content area, but much, much smaller, so I strongly suspect that this and bug 401383 are actually the same bug: "Background of unloaded page is drawing at a comically small size". nsChildView appears believe it's drawing the entire content area as far as I can tell (the actual drawing isn't though, as evidenced by bug 401383)

Interestingly, it appears that the scale factor is around 60 (I'm eyeballing the size of the rect relative to chrome features), which is my pixel-to-appunit ratio. It may be a coincidence, but combined with nsChildView appearing to have sane sizes, I'm wondering if the drawing is somehow happening without going through the pixel/appunit conversion at this point.

Robert, any suggestions on how to test that theory? I have no idea where in the code that transform should be happening.

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Sounds very plausible.

Try setting layout.css.dpi to something like 200. That should change your appunits-per-pixel to 30.

Comment 6

[Stuart Morgan]

Trying with some ridiculously large layout.css.dpi values, the dimensions of the square are indeed 1/appunits-per-pixel.

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Okay. Then we just need to get the problematic drawing operation in the debugger so we can get a call stack and figure out why the correct rect hasn't been set. Setting a conditional breakpoint in the right place with the "bad" width value should do the trick.

Comment 8

[Stuart Morgan]

#0  nsThebesRenderingContext::FillRect (this=0x28eeaed0, aRect=<incomplete type>) at /Volumes/CaminoTrunk/mozilla/gfx/src/thebes/nsThebesRenderingContext.cpp:562
#1  0x200f2c46 in nsWebBrowser::HandleEvent (aEvent=0xbfffcf58) at /Volumes/CaminoTrunk/mozilla/embedding/browser/webBrowser/nsWebBrowser.cpp:1677
#2  0x20501fcc in nsChildView::DispatchEvent (this=0x28ee0ef0, event=0xbfffcf58, aStatus=@0xbfffceec) at /Volumes/CaminoTrunk/mozilla/widget/src/cocoa/nsChildView.mm:1347
#3  0x204fc6f8 in nsChildView::DispatchWindowEvent (this=0x28ee0ef0, event=@0xbfffcf58) at /Volumes/CaminoTrunk/mozilla/widget/src/cocoa/nsChildView.mm:1360
#4  0x205053c4 in -[ChildView drawRect:] (self=0x28ec8f80, _cmd=0x9542a630, aRect={origin = {x = 0, y = 0}, size = {width = 912, height = 708}}) at /Volumes/CaminoTrunk/mozilla/widget/src/cocoa/nsChildView.mm:2234
...

For reasons I haven't been able to track down gdb really doesn't like to let me do useful debugging of C++ code these days, so the FillRect rect isn't shown in the stack, but I turned on the thebes gfx logging and got the following from this invocation:

-1603797152[140d790]: ## 28eeaed0 nsTRC::FillRect [0,0,912,708]
-1603797152[140d790]: ## 28eeaed0 nsTRC::FillRect raw [0.000000,0.000000,15.200000,11.800000]

So FillRect should be getting called with appunits, but is instead getting called with pixels. I would guess that /embedding/.../nsWebBrowser should be doing the conversion in the paint handler?

CCing bsmedberg for advice on who should look at this (and whether it should be moved to one of the core embedding components).

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

> I would guess that /embedding/.../nsWebBrowser should be
> doing the conversion in the paint handler?

Yep.

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix?

Try this.

Comment 11

[Stuart Morgan]

Yep, that fixes it.

Comment 13

[Stuart Morgan]

Is there anything else you need from me before getting this reviewed and landed?

Comment 14

[Stuart Morgan]

Comment on attachment 290902 [details] [diff] [review]
fix?

r=me

Comment 15

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 290902 [details] [diff] [review]
fix?

sr=bzbarsky

Comment 16

[Stuart Morgan]

Landed, thanks for all the help!