Bug 82633 – remove ahem font from flexbox layout tests

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Tony Chang]

remove ahem font from flexbox layout tests

[Tony Chang]

Created attachment 134628 [details]
Patch

[Tony Chang]

http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=css3%2Fflexbox%2Fflex-align-vertical-writing-mode.html

Started failing on the mac bots yesterday.

[Ojan Vafai]

Comment on attachment 134628 [details]
Patch

I don't mind this "fix" but doesn't this point to a genuine bug? It seems that we're not reflowing when we finally load the ahem font, which is totally wrong, no?

[Dimitri Glazkov (Google)]

(In reply to comment #3)
> (From update of attachment 134628 [details])
> I don't mind this "fix" but doesn't this point to a genuine bug? It seems that we're not reflowing when we finally load the ahem font, which is totally wrong, no?

I am pretty sure it's reflowing after the ahem font is loaded. The test doesn't explicitly wait until then -- that's the problem. Ishibashi-san, would you mind confirming this?

[Eric Seidel (no email)]

Comment on attachment 134628 [details]
Patch

Really?  Why should DRT be dumping before the font is loaded?

[Ojan Vafai]

(In reply to comment #5)
> (From update of attachment 134628 [details])
> Really?  Why should DRT be dumping before the font is loaded?

I don't think it is. My running theory is that the font loads halfway through laying out the page. I'm not actively debugging this though, so I'm not sure.

[Ojan Vafai]

(In reply to comment #4)
> (In reply to comment #3)
> > (From update of attachment 134628 [details] [details])
> > I don't mind this "fix" but doesn't this point to a genuine bug? It seems that we're not reflowing when we finally load the ahem font, which is totally wrong, no?
> 
> I am pretty sure it's reflowing after the ahem font is loaded. The test doesn't explicitly wait until then -- that's the problem. Ishibashi-san, would you mind confirming this?

Hm...I think we might have a race. Specifically, we baseline align by calling RenderBlock::firstLineBoxBaseline, which calls style(true)->fontMetrics().ascent(firstRootBox()->baselineType()). If we first layout the flex items before the font has loaded and then reposition them after the font has loaded, then we'll layout the flex items using the fallback font, but align them using the loaded font's ascent.

Not 100% sure that's what's going on, but it still looks like a genuine bug to me. I would expect this to affect any other layout code that uses fontMetrics as well.

[Tony Chang]

(adding dirk since this is gardening related).

I'm going to build on mac and debug this a bit.

[Tony Chang]

We may want to land this in the interim to green the tree.

[Ojan Vafai]

Comment on attachment 134628 [details]
Patch

Regardless, this bug is almost certainly in the mac font code and not in the flexbox code. Tony's going to file a bug with this testcase. Interestingly, this test only fails on the Chromium 10.5 and 10.7 bots. It passes on the Apple Mac bots and the Chromium 10.6 bots.

[Tony Chang]

https://bugs.webkit.org/show_bug.cgi?id=82654 is for tracking this bug.  I'm going to land this patch in an attempt to green the tree.

[Tony Chang]

Committed r112578: <http://trac.webkit.org/changeset/112578>