From af3ab9cb4f492f72cf4cfced7cd309319465ae44 Mon Sep 17 00:00:00 2001 From: "commit-queue@webkit.org" Date: Thu, 19 Jan 2012 09:08:12 +0000 Subject: [PATCH] [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 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): git-svn-id: http://svn.webkit.org/repository/webkit/trunk@105393 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- Source/WebCore/ChangeLog | 23 +++++++ .../chromium/GlyphPageTreeNodeChromiumWin.cpp | 70 ++++++++++++++-------- 2 files changed, 68 insertions(+), 25 deletions(-) diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog index 53de2f1..84ee594 100644 --- a/Source/WebCore/ChangeLog +++ b/Source/WebCore/ChangeLog @@ -1,3 +1,26 @@ +2012-01-19 Kazuhiro Inaba + + [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 createAttributeNS should understand that "xmlns" is allowed in the http://www.w3.org/2000/xmlns/ diff --git a/Source/WebCore/platform/graphics/chromium/GlyphPageTreeNodeChromiumWin.cpp b/Source/WebCore/platform/graphics/chromium/GlyphPageTreeNodeChromiumWin.cpp index f5ab221..42f45ab 100644 --- a/Source/WebCore/platform/graphics/chromium/GlyphPageTreeNodeChromiumWin.cpp +++ b/Source/WebCore/platform/graphics/chromium/GlyphPageTreeNodeChromiumWin.cpp @@ -1,5 +1,5 @@ /* - * 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 * modification, are permitted provided that the following conditions are @@ -34,6 +34,7 @@ #include "Font.h" #include "GlyphPageTreeNode.h" +#include "HWndDC.h" #include "PlatformSupport.h" #include "SimpleFontData.h" #include "SystemInfo.h" @@ -49,15 +50,27 @@ static void fillEmptyGlyphs(GlyphPage* page) page->setGlyphDataForIndex(i, 0, 0); } -// Lazily initializes space glyph -static Glyph initSpaceGlyph(HDC dc, Glyph* 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 (*spaceGlyph) - return *spaceGlyph; + 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); } // Fills |length| glyphs starting at |offset| in a |page| in the Basic @@ -68,27 +81,25 @@ static bool fillBMPGlyphs(unsigned offset, unsigned length, UChar* buffer, GlyphPage* page, - const SimpleFontData* fontData, - bool recurse) + const SimpleFontData* fontData) { - HDC dc = GetDC((HWND)0); + 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 (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"); - if (recurse) { - if (PlatformSupport::ensureFontLoaded(fontData->platformData().hfont())) - return fillBMPGlyphs(offset, length, buffer, page, fontData, false); - - fillEmptyGlyphs(page); - return false; + 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; } @@ -128,7 +139,11 @@ static bool fillBMPGlyphs(unsigned offset, // Also according to Jungshik and Hironori's suggestion and modification // 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 bool haveGlyphs = false; @@ -138,6 +153,7 @@ static bool fillBMPGlyphs(unsigned offset, invalidGlyph = 0x1F; Glyph spaceGlyph = 0; // Glyph for a space. Lazily filled. + bool spaceGlyphInitialized = false; for (unsigned i = 0; i < length; i++) { UChar c = buffer[i]; @@ -149,7 +165,12 @@ static bool fillBMPGlyphs(unsigned offset, if (Font::treatAsSpace(c)) { // 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 // pointer to be 0 if the glyph is not present @@ -161,7 +182,6 @@ static bool fillBMPGlyphs(unsigned offset, } SelectObject(dc, oldFont); - ReleaseDC(0, dc); return haveGlyphs; } @@ -222,7 +242,7 @@ bool GlyphPage::fill(unsigned offset, unsigned length, UChar* characterBuffer, // FIXME: Add assertions to make sure that buffer is entirely in BMP // 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) { // A non-BMP input buffer will be twice as long as output glyph buffer -- 2.7.4