Make sure RecordingImageBufferSurface's current frame is valid

third_party/WebKit/Source/platform/graphics/RecordingImageBufferSurface.cpp

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "platform/graphics/RecordingImageBufferSurface.h"
 
 #include "platform/Histogram.h"
 #include "platform/graphics/CanvasMetrics.h"
 #include "platform/graphics/ExpensiveCanvasHeuristicParameters.h"
 #include "platform/graphics/GraphicsContext.h"
@@ skipping 252 matching lines @@
     ASSERT(m_currentFrame->getRecordingCanvas());
     ASSERT(fallbackReason);
     ASSERT(*fallbackReason == FallbackReasonUnknown);
 
     if (!m_imageBuffer->isDirty()) {
         if (!m_previousFrame) {
             // Create an initial blank frame
             m_previousFrame = fromSkSp(m_currentFrame->finishRecordingAsPicture());
             initializeCurrentFrame();
         }
+        if (!m_currentFrame.get())
+            *fallbackReason = FallbackReasonCurrentFrameNull;

[Justin Novosad, 2016/07/04 14:01:05]

I don't think this is right. If m_currentFrame is not set, it means that a
fallback has already occured, in which case finalizeFrameInternal should not be
called (ASSERTs above check that).  If this code is reachable, we might be in an
invalid state.  Do you have a repro case that exercises this case?

[xidachen, 2016/07/04 14:06:36]

I don't have a repro case. But from the histogram of canary channel, we still
see 0.2% that is Unkonwn. This seems to be the only place that could happen.
Could this happen when browser crashes, or some un-expected behavior?

[Justin Novosad, 2016/07/04 14:16:57]

Let's do an experiment on the canary channel then: Change the two first ASSERTs
at the top of this method into "CHECK", and right here, do:
CHECK(m_currentFrame);
return true;

[xidachen, 2016/07/04 14:36:37]

Okay. Let's do the experiment then. I remove the category in the histogram and
replace the ASSERTs by CHECKs. My understanding is that CHECK won't do anything
if it is not hit, is that correct?

         return m_currentFrame.get();
     }
 
     if (!m_frameWasCleared) {
         *fallbackReason = FallbackReasonCanvasNotClearedBetweenFrames;
         return false;
     }
 
     if (m_fallbackFactory && m_currentFrame->getRecordingCanvas()->getSaveCount() > ExpensiveCanvasHeuristicParameters::ExpensiveRecordingStackDepth) {
         *fallbackReason = FallbackReasonRunawayStateStack;
@@ skipping 76 matching lines @@
 
 void RecordingImageBufferSurface::setIsHidden(bool hidden)
 {
     if (m_fallbackSurface)
         m_fallbackSurface->setIsHidden(hidden);
     else
         ImageBufferSurface::setIsHidden(hidden);
 }
 
 } // namespace blink