From f4a9f83b38a0a45cda3205ad50747e6a7719f8ab Mon Sep 17 00:00:00 2001 From: Cary Clark Date: Fri, 24 Mar 2017 11:41:24 -0400 Subject: fix new tab crash in skia clip stack The crash on the new tab page is triggered by processing a transparency. This creates a new Skia device in CPDF_RenderStatus::LoadSMask(): // cpdf_renderstatus.cpp # 2557 if (!bitmap_device.Create(width, height, format, nullptr)) which sets the Skia clip stack to empty. It then calls RenderObjectList() RenderSingleObject() ProcessClipPath() which resets the clip stack; // cpdf_renderstatus.cpp # 1882 m_LastClipPath = ClipPath; m_pDevice->RestoreState(true); At this point m_LastClipPath contains {m_Ref={m_pObject={m_pObj=empty } } } The impelemntation in CFX_AggDeviceDriver::RestoreState() is // fx_agg_driver.cpp # 1283 if (m_StateStack.empty()) return; This hides unbalanced save/restores, but reworking PDFium to balance is nontrivial. R=dsinclair@chromium.org BUG=chromium:704442 Bug: Change-Id: Ia70d4dd7bd118e40adc5c029acbaa0b66372d3aa Reviewed-on: https://pdfium-review.googlesource.com/3191 Commit-Queue: dsinclair Reviewed-by: dsinclair --- core/fxge/skia/fx_skia_device.cpp | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/core/fxge/skia/fx_skia_device.cpp b/core/fxge/skia/fx_skia_device.cpp index 1071f85a3b..208b763b8f 100644 --- a/core/fxge/skia/fx_skia_device.cpp +++ b/core/fxge/skia/fx_skia_device.cpp @@ -830,6 +830,8 @@ class SkiaState { m_type = Accumulator::kNone; } + bool IsEmpty() { return !m_commands.count(); } + bool SetClipFill(const CFX_PathData* pPathData, const CFX_Matrix* pMatrix, int fill_mode) { @@ -938,13 +940,6 @@ class SkiaState { bool ClipRestore() { if (m_debugDisable) return false; - - // TODO(dsinclair): This check works around crbug.com/704442 where - // it looks like we have a ClipRestore without a corresponding ClipSave. - // We need to track down the imbalance and fix correctly. - if (m_commandIndex == 0) - return true; - Dump(__func__); while (Clip::kSave != m_commands[--m_commandIndex]) { SkASSERT(m_commandIndex > 0); @@ -1128,9 +1123,14 @@ class SkiaState { bool m_debugDisable; // turn off cache for debugging #if SHOW_SKIA_PATH mutable int m_debugSaveCounter; + static int m_debugInitCounter; #endif }; +#if SHOW_SKIA_PATH +int SkiaState::m_debugInitCounter; +#endif + // convert a stroking path to scanlines void CFX_SkiaDeviceDriver::PaintStroke(SkPaint* spaint, const CFX_GraphStateData* pGraphState, @@ -1362,16 +1362,21 @@ void CFX_SkiaDeviceDriver::SaveState() { } void CFX_SkiaDeviceDriver::RestoreState(bool bKeepSaved) { - if (!m_pCache->ClipRestore()) - m_pCanvas->restore(); - if (bKeepSaved && !m_pCache->ClipSave()) - m_pCanvas->save(); #ifdef _SKIA_SUPPORT_PATHS_ m_pClipRgn.reset(); if (m_StateStack.empty()) return; +#else + if (m_pCache->IsEmpty()) + return; +#endif + if (!m_pCache->ClipRestore()) + m_pCanvas->restore(); + if (bKeepSaved && !m_pCache->ClipSave()) + m_pCanvas->save(); +#ifdef _SKIA_SUPPORT_PATHS_ if (bKeepSaved) { if (m_StateStack.back()) m_pClipRgn = pdfium::MakeUnique(*m_StateStack.back()); -- cgit v1.2.3