@cyanglaz
An empty layer tree is a very special case and in theory it shouldn't happen. Here I filed another Issue to illustrate the specific scene it is currently happening.
flutter/flutter#98959

Then let's take a look at the call stacks of following scenarios.

1. a normal frame

Rasterizer::Draw
  -> Rasterizer::DoDraw
    -> Rasterizer::DrawToSurface
      -> Rasterizer::DrawToSurfaceUnsafe
        -> ExternalViewEmbedder::BeginFrame
        -> ExternalViewEmbedder:: SubmitFrame
  -> ExternalViewEmbedder::EndFrame

2. a frame without surface

Rasterizer::Draw
  // do noting

3. a frame with empty layer tree

Rasterizer::Draw
  -> ExternalViewEmbedder::EndFrame

4. redraw last layer tree

Rasterizer::DrawLastLayerTree
  -> Rasterizer::DrawToSurface
      -> Rasterizer::DrawToSurfaceUnsafe
        -> ExternalViewEmbedder::BeginFrame
        -> ExternalViewEmbedder:: SubmitFrame
  -> ExternalViewEmbedder::EndFrame

5. redraw last layer tree but last layer tee is not exists

Rasterizer::DrawLastLayerTree
  // do noting

From the above scenarios, It seems that Rasterizer::Draw or Rasterizer::DrawLastLayerTree is not always call ExternalViewEmbedder::EndFrame. So I think when we draw an empty layer tee, we should do nothing, just like scene 2 (a frame without surface) and scene 5 (redraw last layer tree but last layer tee is not exists) above.

Calling EndFrame with an empty layer tree seems unnecessary and has side effects. At the same time, this also adds unnecessary work to the platform to correctly implement EndFrame

@ColdPaleLight Ok, sounds good to me.
I double checked the endFrame implementations for all the platforms, there doesn't seem to be any side effect of not calling endFrame. So I'm ok with it.

Another way to fix this issue is to pass consume_result to EndFrame, and update the docs, so implementors are aware of this behavior.

I think conditionally calling methods like this can be harder to reason about a few years from now. I'd definitely prefer something more explicit.

@blasten
The original intention of EndFrame design is not to call back every time in Rasterizer::Draw or Rasterizer::DrawLastLayerTree, otherwise it should also be called back when Rasterizer::surface_ does not exist.

For this scene of the empty layer tree, I think we should do the same thing as the scene where Rasterizer::surface_ does not exist. Because in this scenario, the Rasterizer::Draw method actually does nothing except call EndFrame. I think the current implementation itself is incorrect.

@ColdPaleLight actually, looking at the pattern when EndFrame "should" be called. Maybe we can add a DCHECK that crashes if EndFrame is called without a BeginFrame? Does it make more sense?

@ColdPaleLight actually, looking at the pattern when EndFrame "should" be called. Maybe we can add a DCHECK that crashes if EndFrame is called without a BeginFrame? Does it make more sense?

If we are sure that BeginFrame and EndFrame should be paired. Maybe we can set a flag when BeginFrame is called, and then only call EndFrame when this flag is set.

If we are sure that BeginFrame and EndFrame should be paired. Maybe we can set a flag when BeginFrame is called, and then only call EndFrame when this flag is set.

Yes, that's what the pattern you listed tells me and that's what I think we should do.

@cyanglaz
Can we continue to land this PR when @dnfield 's comment solved ? I think returning early in case of PipelineConsumeResult::NoneAvailable will make the code more robust.

I 'll file another PR to pair the BeginFrame and EndFrame

Yes I have approved this PR from my end! ;)