Proper unicode-range font loading behavior

Source/platform/fonts/GlyphPageTreeNode.cpp

 /*
  * Copyright (C) 2006, 2007, 2008 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  *
  * 1.  Redistributions of source code must retain the above copyright
  *     notice, this list of conditions and the following disclaimer.
  * 2.  Redistributions in binary form must reproduce the above copyright
@@ skipping 205 matching lines @@
                 const SegmentedFontData* segmentedFontData = static_cast<const SegmentedFontData*>(fontData);
                 for (int i = segmentedFontData->numRanges() - 1; i >= 0; i--) {
                     const FontDataRange& range = segmentedFontData->rangeAt(i);
                     // all this casting is to ensure all the parameters to min and max have the same type,
                     // to avoid ambiguous template parameter errors on Windows
                     int from = max(0, static_cast<int>(range.from()) - static_cast<int>(start));
                     int to = 1 + min(static_cast<int>(range.to()) - static_cast<int>(start), static_cast<int>(GlyphPage::size) - 1);
                     if (from >= static_cast<int>(GlyphPage::size) || to <= 0)
                         continue;
 
-                    // If this is a custom font needs to be loaded, kick off
-                    // the load here, and do not fill the page so that
-                    // font fallback is used while loading.
+                    // If this is a custom font needs to be loaded, do not fill
+                    // the page so that font fallback is used while loading.
                     RefPtr<CustomFontData> customData = range.fontData()->customFontData();
                     if (customData && customData->isLoadingFallback()) {
-                        customData->beginLoadIfNeeded();
+                        for (int j = from; j < to; j++) {

[eae, 2014/04/23 18:38:39]

Here and elsewhere shouldn't j (and from) be unsigned?

[Kunihiko Sakamoto, 2014/04/24 01:49:24]

I'd prefer to keep them int, as calculation of |from| and |to| may involve
negative numbers. Also, range.from() and range.to() are UChar32, which is
int32_t, so static_cast somewhere is inevitable.

+                            m_page->setCustomFontToLoad(j, customData.get());
+                            haveGlyphs = true;
+                        }
                         continue;
                     }
 
                     haveGlyphs |= fill(m_page.get(), from, to - from, buffer + from * (start < 0x10000 ? 1 : 2), (to - from) * (start < 0x10000 ? 1 : 2), range.fontData().get());
                 }
             }
 
             if (!haveGlyphs)
                 m_page = nullptr;
         } else if (parentPage && parentPage->owner() != m_parent) {
@@ skipping 23 matching lines @@
                 // parent page.
                 m_page = parentPage;
             } else {
                 // Combine the parent's glyphs and ours to form a new more complete page.
                 m_page = GlyphPage::createForMixedFontData(this);
 
                 // Overlay the parent page on the fallback page. Check if the fallback font
                 // has added anything.
                 bool newGlyphs = false;
                 for (unsigned i = 0; i < GlyphPage::size; i++) {
-                    if (parentPage->glyphAt(i))
+                    if (parentPage->glyphAt(i)) {
                         m_page->setGlyphDataForIndex(i, parentPage->glyphDataForIndex(i));
-                    else  if (fallbackPage->glyphAt(i)) {
+                    } else if (fallbackPage->glyphAt(i)) {
                         m_page->setGlyphDataForIndex(i, fallbackPage->glyphDataForIndex(i));
                         newGlyphs = true;
                     }
+
+                    if (parentPage->customFontToLoadAt(i)) {
+                        m_page->setCustomFontToLoad(i, parentPage->customFontToLoadAt(i));
+                    } else if (fallbackPage->customFontToLoadAt(i) && !parentPage->glyphAt(i)) {
+                        m_page->setCustomFontToLoad(i, fallbackPage->customFontToLoadAt(i));
+                        newGlyphs = true;
+                    }
                 }
 
                 if (!newGlyphs)
                     // We didn't override anything, so our override is just the parent page.
                     m_page = parentPage;
             }
         }
     } else {
         // System fallback. Initialized with the parent's page here, as individual
         // entries may use different fonts depending on character. If the Font
@@ skipping 114 matching lines @@
         printf("\nPage %d:\n", it->key);
         showGlyphPageTree(it->key);
     }
 }
 
 void showGlyphPageTree(unsigned pageNumber)
 {
     WebCore::GlyphPageTreeNode::getRoot(pageNumber)->showSubtree();
 }
 #endif