summaryrefslogtreecommitdiff
path: root/core/fxge/skia
diff options
context:
space:
mode:
authordsinclair <dsinclair@chromium.org>2016-09-19 08:37:10 -0700
committerCommit bot <commit-bot@chromium.org>2016-09-19 08:37:10 -0700
commitc48089977dc4d2a63d37e6668382c10b42e22a72 (patch)
tree931e8a2f17d997e55c9bead576f7f672196844c0 /core/fxge/skia
parentea3c3be83dae12ef682c68fc7cf906d790fd9f84 (diff)
downloadpdfium-c48089977dc4d2a63d37e6668382c10b42e22a72.tar.xz
Revert of Pdfium: Fix fonts leaking on ClosePage. (patchset #10 id:180001 of https://codereview.chromium.org/2158023002/ )
Reason for revert: Causes heap-use-after-free. See crbug.com/647612. Original issue's description: > Fix memory leaking on ClosePage. > CFX_FontCache refactoring: > after this CL: Only one global CFX_FontCache used. Any cached items from it, are released, when its are not used. > > BUG=79367,48791 > > The fonts was not cleared after unloading pages. > > Test pdf: > > http://www.nasa.gov/pdf/750614main_NASA_FY_2014_Budget_Estimates-508.pdf > > For this file, we have ~5 fonts per page, which equal ~1 Mb per page. > In this PDF we have 670 pages, as result after slow scrolling(reading) full document we have ~600 Mb fonts data in memory. > > memory usage of PDF Plugin: > before this CL: ~660 Mb > after this CL: ~100 Mb > > Committed: https://pdfium.googlesource.com/pdfium/+/cde5101eb15b24519e89fa500fe37038bc8e2201 TBR=tsepez@chromium.org,brucedawson@chromium.org,npm@chromium.org,art-snake@yandex-team.ru # Not skipping CQ checks because original CL landed more than 1 days ago. BUG=79367,48791 Review-Url: https://codereview.chromium.org/2350763002
Diffstat (limited to 'core/fxge/skia')
-rw-r--r--core/fxge/skia/fx_skia_device.cpp27
-rw-r--r--core/fxge/skia/fx_skia_device.h2
-rw-r--r--core/fxge/skia/fx_skia_device_unittest.cpp10
3 files changed, 27 insertions, 12 deletions
diff --git a/core/fxge/skia/fx_skia_device.cpp b/core/fxge/skia/fx_skia_device.cpp
index b1c47bfedf..d2e4abce92 100644
--- a/core/fxge/skia/fx_skia_device.cpp
+++ b/core/fxge/skia/fx_skia_device.cpp
@@ -14,6 +14,7 @@
#include "core/fpdfapi/fpdf_parser/include/cpdf_array.h"
#include "core/fpdfapi/fpdf_parser/include/cpdf_dictionary.h"
#include "core/fpdfapi/fpdf_parser/include/cpdf_stream_acc.h"
+#include "core/fxge/include/cfx_fontcache.h"
#include "core/fxge/include/cfx_fxgedevice.h"
#include "core/fxge/include/cfx_gemodule.h"
#include "core/fxge/include/cfx_graphstatedata.h"
@@ -485,6 +486,7 @@ class SkiaState {
// mark all cached state as uninitialized
SkiaState()
: m_pFont(nullptr),
+ m_pCache(nullptr),
m_fontSize(0),
m_fillColor(0),
m_strokeColor(0),
@@ -578,6 +580,7 @@ class SkiaState {
bool DrawText(int nChars,
const FXTEXT_CHARPOS* pCharPos,
CFX_Font* pFont,
+ CFX_FontCache* pCache,
const CFX_Matrix* pMatrix,
FX_FLOAT font_size,
uint32_t color,
@@ -588,12 +591,13 @@ class SkiaState {
FlushCommands(pDriver);
if (m_drawPath)
FlushPath(pDriver);
- if (m_drawText && FontChanged(pFont, pMatrix, font_size, color))
+ if (m_drawText && FontChanged(pFont, pCache, pMatrix, font_size, color))
FlushText(pDriver);
if (!m_drawText) {
m_positions.setCount(0);
m_glyphs.setCount(0);
m_pFont = pFont;
+ m_pCache = pCache;
m_fontSize = font_size;
m_fillColor = color;
m_drawMatrix = *pMatrix;
@@ -622,8 +626,8 @@ class SkiaState {
SkPaint skPaint;
skPaint.setAntiAlias(true);
skPaint.setColor(m_fillColor);
- if (m_pFont->GetFace()) { // exclude placeholder test fonts
- sk_sp<SkTypeface> typeface(SkSafeRef(m_pFont->GetDeviceCache()));
+ if (m_pFont->GetFace() && m_pCache) { // exclude placeholder test fonts
+ sk_sp<SkTypeface> typeface(SkSafeRef(m_pCache->GetDeviceCache(m_pFont)));
skPaint.setTypeface(typeface);
}
skPaint.setTextEncoding(SkPaint::kGlyphID_TextEncoding);
@@ -779,11 +783,13 @@ class SkiaState {
}
bool FontChanged(CFX_Font* pFont,
+ CFX_FontCache* pCache,
const CFX_Matrix* pMatrix,
FX_FLOAT font_size,
uint32_t color) {
- return pFont != m_pFont || MatrixChanged(pMatrix, m_drawMatrix) ||
- font_size != m_fontSize || color != m_fillColor;
+ return pFont != m_pFont || pCache != m_pCache ||
+ MatrixChanged(pMatrix, m_drawMatrix) || font_size != m_fontSize ||
+ color != m_fillColor;
}
bool MatrixChanged(const CFX_Matrix* pMatrix, const CFX_Matrix& refMatrix) {
@@ -866,6 +872,7 @@ class SkiaState {
CFX_GraphStateData m_drawState;
CFX_Matrix m_clipMatrix;
CFX_Font* m_pFont;
+ CFX_FontCache* m_pCache;
FX_FLOAT m_fontSize;
uint32_t m_fillColor;
uint32_t m_strokeColor;
@@ -998,14 +1005,18 @@ void CFX_SkiaDeviceDriver::Flush() {
FX_BOOL CFX_SkiaDeviceDriver::DrawDeviceText(int nChars,
const FXTEXT_CHARPOS* pCharPos,
CFX_Font* pFont,
+ CFX_FontCache* pCache,
const CFX_Matrix* pObject2Device,
FX_FLOAT font_size,
uint32_t color) {
- if (m_pCache->DrawText(nChars, pCharPos, pFont, pObject2Device, font_size,
- color, this)) {
+ if (!pCache)
+ pCache = CFX_GEModule::Get()->GetFontCache();
+ if (m_pCache->DrawText(nChars, pCharPos, pFont, pCache, pObject2Device,
+ font_size, color, this)) {
return TRUE;
}
- sk_sp<SkTypeface> typeface(SkSafeRef(pFont->GetDeviceCache()));
+ sk_sp<SkTypeface> typeface(
+ SkSafeRef(pCache ? pCache->GetDeviceCache(pFont) : nullptr));
SkPaint paint;
paint.setAntiAlias(true);
paint.setColor(color);
diff --git a/core/fxge/skia/fx_skia_device.h b/core/fxge/skia/fx_skia_device.h
index 78d8e32ebc..8a1bf690b2 100644
--- a/core/fxge/skia/fx_skia_device.h
+++ b/core/fxge/skia/fx_skia_device.h
@@ -10,6 +10,7 @@
#include "core/fxge/include/cfx_pathdata.h"
#include "core/fxge/include/ifx_renderdevicedriver.h"
+class CFX_FontCache;
class SkCanvas;
class SkMatrix;
class SkPaint;
@@ -121,6 +122,7 @@ class CFX_SkiaDeviceDriver : public IFX_RenderDeviceDriver {
FX_BOOL DrawDeviceText(int nChars,
const FXTEXT_CHARPOS* pCharPos,
CFX_Font* pFont,
+ CFX_FontCache* pCache,
const CFX_Matrix* pObject2Device,
FX_FLOAT font_size,
uint32_t color) override;
diff --git a/core/fxge/skia/fx_skia_device_unittest.cpp b/core/fxge/skia/fx_skia_device_unittest.cpp
index 5666feea9f..5162e9cbde 100644
--- a/core/fxge/skia/fx_skia_device_unittest.cpp
+++ b/core/fxge/skia/fx_skia_device_unittest.cpp
@@ -2,6 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
+#include "core/fxge/include/cfx_fontcache.h"
#include "core/fxge/include/cfx_fxgedevice.h"
#include "core/fxge/include/cfx_graphstatedata.h"
#include "core/fxge/include/cfx_pathdata.h"
@@ -38,6 +39,7 @@ void CommonTest(CFX_SkiaDeviceDriver* driver, const State& state) {
FXTEXT_CHARPOS charPos[] = {{{0, 0, 0, 0}, 0, 1, 1, 4, false, false}};
CFX_Font font;
FX_FLOAT fontSize = 1;
+ CFX_FontCache cache;
CFX_PathData clipPath, clipPath2;
clipPath.AppendRect(0, 0, 3, 1);
clipPath2.AppendRect(0, 0, 2, 1);
@@ -57,8 +59,8 @@ void CommonTest(CFX_SkiaDeviceDriver* driver, const State& state) {
driver->DrawPath(&path1, &matrix, &graphState, 0xFF112233, 0,
FXFILL_WINDING, 0);
} else if (state.m_graphic == State::Graphic::kText) {
- driver->DrawDeviceText(SK_ARRAY_COUNT(charPos), charPos, &font, &matrix,
- fontSize, 0xFF445566);
+ driver->DrawDeviceText(SK_ARRAY_COUNT(charPos), charPos, &font, &cache,
+ &matrix, fontSize, 0xFF445566);
}
if (state.m_save == State::Save::kYes)
driver->RestoreState(true);
@@ -80,8 +82,8 @@ void CommonTest(CFX_SkiaDeviceDriver* driver, const State& state) {
driver->DrawPath(&path2, &matrix2, &graphState, 0xFF112233, 0,
FXFILL_WINDING, 0);
} else if (state.m_graphic == State::Graphic::kText) {
- driver->DrawDeviceText(SK_ARRAY_COUNT(charPos), charPos, &font, &matrix2,
- fontSize, 0xFF445566);
+ driver->DrawDeviceText(SK_ARRAY_COUNT(charPos), charPos, &font, &cache,
+ &matrix2, fontSize, 0xFF445566);
}
if (state.m_save == State::Save::kYes)
driver->RestoreState(false);