Fallback fonts are not used when computing line heights in some cases.
Created attachment 201416 [details] Fix (without change log)
<rdar://problem/12803147>
Created attachment 201441 [details] Fixes one bug
Created attachment 201442 [details] Reverted erroneous test change
Comment on attachment 201442 [details] Reverted erroneous test change View in context: https://bugs.webkit.org/attachment.cgi?id=201442&action=review > Source/WebCore/rendering/RenderBlockLineLayout.cpp:3094 > + if (wordMeasurement.fallbackFonts.isEmpty() && !fallbackFonts.isEmpty()) > + wordMeasurement.fallbackFonts.swap(fallbackFonts); Sure could use a why comment.
Created attachment 203893 [details] More comprehensive WIP fix There was more fundamental flaw in our code.
Created attachment 203991 [details] Fixes the bug
I thought passing a NULL fallbackFonts pointer into floatWidthFor{Simple,Complex}Text was a significant optimization. Have you checked whether this patch has any impact performance?
(In reply to comment #8) > I thought passing a NULL fallbackFonts pointer into floatWidthFor{Simple,Complex}Text was a significant optimization. Have you checked whether this patch has any impact performance? Yeah, I'm getting the numbers now.
Created attachment 203994 [details] A/B test results Thankfully I don't see any regression from running some perf. tests but please let me know if there is some other benchmark or page where this patch may cause a regression.
Comment on attachment 203991 [details] Fixes the bug View in context: https://bugs.webkit.org/attachment.cgi?id=203991&action=review I hope creating and destroying these empty hash sets is inexpensive enough. > Source/WebCore/platform/graphics/Font.cpp:325 > + ASSERT(fallbackFonts); This assertion is really unnecessary. The code that guarantees this is non-zero is just a couple lines up from here.
(In reply to comment #11) > (From update of attachment 203991 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=203991&action=review > > I hope creating and destroying these empty hash sets is inexpensive enough. I think andersca optimized empty hash maps recently to reduce the memory usage, etc... so HashTable only initializes various member variables to 0 now. > > Source/WebCore/platform/graphics/Font.cpp:325 > > + ASSERT(fallbackFonts); > > This assertion is really unnecessary. The code that guarantees this is non-zero is just a couple lines up from here. Removed.
Committed r151327: <http://trac.webkit.org/changeset/151327>