Changeset 105393 in webkit

Timestamp:

Jan 19, 2012 1:08:12 AM (12 years ago)

Author:

commit-queue@webkit.org

Message:

[Chromium] Random characters got rendered as empty boxes or with incorrect glyphs even when a font is present
​https://bugs.webkit.org/show_bug.cgi?id=76508

Patch by Kazuhiro Inaba <​kinaba@chromium.org> on 2012-01-19
Reviewed by Kent Tamura.

Wrapped GetGlyphIndices() API calls so that when they failed we trigger font
loading outside the sandbox and retry the call.

No new auto tests since the bug involves the system's occasional cache behavior
and thus there's no reliable way to reproduce and test the situation.

• platform/graphics/chromium/GlyphPageTreeNodeChromiumWin.cpp:

(WebCore::getGlyphIndices):
GDI call wrapper ensuring fonts to be loaded.
(WebCore::initSpaceGlyph):
Changed to use the wrapper function.
(WebCore::fillBMPGlyphs):
Changed to use the wrapper function.
Introduced scoped HDC management by HWndDC.
(WebCore::GlyphPage::fill):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/graphics/chromium/GlyphPageTreeNodeChromiumWin.cpp (modified) (9 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-01-19  Kazuhiro Inaba  <kinaba@chromium.org>
+
+        [Chromium] Random characters got rendered as empty boxes or with incorrect glyphs even when a font is present
+        https://bugs.webkit.org/show_bug.cgi?id=76508
+
+        Reviewed by Kent Tamura.
+
+        Wrapped GetGlyphIndices() API calls so that when they failed we trigger font
+        loading outside the sandbox and retry the call.
+
+        No new auto tests since the bug involves the system's occasional cache behavior
+        and thus there's no reliable way to reproduce and test the situation.
+
+        * platform/graphics/chromium/GlyphPageTreeNodeChromiumWin.cpp:
+        (WebCore::getGlyphIndices):
+        GDI call wrapper ensuring fonts to be loaded.
+        (WebCore::initSpaceGlyph):
+        Changed to use the wrapper function.
+        (WebCore::fillBMPGlyphs):
+        Changed to use the wrapper function.
+        Introduced scoped HDC management by HWndDC.
+        (WebCore::GlyphPage::fill):
+
 2012-01-19  Adam Barth  <abarth@webkit.org>
 

trunk/Source/WebCore/platform/graphics/chromium/GlyphPageTreeNodeChromiumWin.cpp

@@ -1 +1 @@
 /*
- * Copyright (c) 2008, 2009 Google Inc. All rights reserved.
+ * Copyright (c) 2008, 2009, 2012 Google Inc. All rights reserved.
  * 
  * Redistribution and use in source and binary forms, with or without
@@ -35 +35 @@
 #include "Font.h"
 #include "GlyphPageTreeNode.h"
+#include "HWndDC.h"
 #include "PlatformSupport.h"
 #include "SimpleFontData.h"
@@ -50 +51 @@
 }
 
-// Lazily initializes space glyph
-static Glyph initSpaceGlyph(HDC dc, Glyph* spaceGlyph)
-{
-    if (*spaceGlyph)
-        return *spaceGlyph;
-
+// Convert characters to glyph ids by GetGlyphIndices(), during which, we
+// ensure the font is loaded in memory to make it work in a sandboxed process.
+static bool getGlyphIndices(HFONT font, HDC dc, const UChar* characters, unsigned charactersLength, WORD* glyphBuffer, DWORD flag)
+{
+    if (GetGlyphIndices(dc, characters, charactersLength, glyphBuffer, flag) != GDI_ERROR)
+        return true;
+    if (PlatformSupport::ensureFontLoaded(font)) {
+        if (GetGlyphIndices(dc, characters, charactersLength, glyphBuffer, flag) != GDI_ERROR)
+            return true;
+        // FIXME: Handle gracefully the error if this call also fails.
+        // See http://crbug.com/6401
+        LOG_ERROR("Unable to get the glyph indices after second attempt");
+    }
+    return false;
+}
+
+// Initializes space glyph
+static bool initSpaceGlyph(HFONT font, HDC dc, Glyph* spaceGlyph)
+{
     static wchar_t space = ' ';
-    GetGlyphIndices(dc, &space, 1, spaceGlyph, 0);
-    return *spaceGlyph;
+    return getGlyphIndices(font, dc, &space, 1, spaceGlyph, 0);
 }
 
@@ -69 +82 @@
                           UChar* buffer,
                           GlyphPage* page,
-                          const SimpleFontData* fontData,
-                          bool recurse)
-{
-    HDC dc = GetDC((HWND)0);
+                          const SimpleFontData* fontData)
+{
+    HWndDC dc((HWND)0);
     HGDIOBJ oldFont = SelectObject(dc, fontData->platformData().hfont());
 
     TEXTMETRIC tm = {0};
     if (!GetTextMetrics(dc, &tm)) {
-        SelectObject(dc, oldFont);
-        ReleaseDC(0, dc);
-
-        if (recurse) {
-            if (PlatformSupport::ensureFontLoaded(fontData->platformData().hfont()))
-                return fillBMPGlyphs(offset, length, buffer, page, fontData, false);
-
-            fillEmptyGlyphs(page);
-            return false;
+        if (PlatformSupport::ensureFontLoaded(fontData->platformData().hfont())) {
+            if (!GetTextMetrics(dc, &tm)) {
+                // FIXME: Handle gracefully the error if this call also fails.
+                // See http://crbug.com/6401
+                LOG_ERROR("Unable to get the text metrics after second attempt");
+
+                SelectObject(dc, oldFont);
+                fillEmptyGlyphs(page);
+                return false;
+            }
         } else {
-            // FIXME: Handle gracefully the error if this call also fails.
-            // See http://crbug.com/6401
-            LOG_ERROR("Unable to get the text metrics after second attempt");
+            SelectObject(dc, oldFont);
             fillEmptyGlyphs(page);
             return false;
@@ -129 +140 @@
     // we treat turetype and raster Font as different way when windows version
     // is less than Vista.
-    GetGlyphIndices(dc, buffer, length, localGlyphBuffer, GGI_MARK_NONEXISTING_GLYPHS);
+    if (!getGlyphIndices(fontData->platformData().hfont(), dc, buffer, length, localGlyphBuffer, GGI_MARK_NONEXISTING_GLYPHS)) {
+        SelectObject(dc, oldFont);
+        fillEmptyGlyphs(page);
+        return false;
+    }
 
     // Copy the output to the GlyphPage
@@ -139 +154 @@
 
     Glyph spaceGlyph = 0;  // Glyph for a space. Lazily filled.
+    bool spaceGlyphInitialized = false;
 
     for (unsigned i = 0; i < length; i++) {
@@ -150 +166 @@
             // Hard code the glyph indices for characters that should be
             // treated like spaces.
-            glyph = initSpaceGlyph(dc, &spaceGlyph);
+            if (!spaceGlyphInitialized) {
+                // If initSpaceGlyph fails, spaceGlyph stays 0 (= glyph is not present).
+                initSpaceGlyph(fontData->platformData().hfont(), dc, &spaceGlyph);
+                spaceGlyphInitialized = true;
+            }
+            glyph = spaceGlyph;
         } else if (glyph == invalidGlyph) {
             // WebKit expects both the glyph index and FontData
@@ -162 +183 @@
 
     SelectObject(dc, oldFont);
-    ReleaseDC(0, dc);
     return haveGlyphs;
 }
@@ -223 +243 @@
     // or entirely in non-BMP. 
     if (bufferLength == length)
-        return fillBMPGlyphs(offset, length, characterBuffer, this, fontData, true);
+        return fillBMPGlyphs(offset, length, characterBuffer, this, fontData);
 
     if (bufferLength == 2 * length) {