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 <dsinclair@chromium.org>
Reviewed-by: dsinclair <dsinclair@chromium.org>

1 files 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<CFX_ClipRgn>(*m_StateStack.back());