[iOS] Skip unnecessary canvas operations

[zhongwuzw]

Description

After #20671 merged, ExternalViewEmbedder always has value,

engine/shell/common/rasterizer.cc

Lines 472 to 474 in b22809b

	if (external_view_embedder != nullptr) {
	FML_DCHECK(!frame->IsSubmitted());
	external_view_embedder->SubmitFrame(surface_->GetContext(),

so we always call platform_views_controller_'s SubmitFrame even if not exit platform view, we need to bypass these unnecessary operations.

engine/shell/platform/darwin/ios/ios_external_view_embedder.mm

Line 79 in b22809b

platform_views_controller_->SubmitFrame(std::move(context), ios_context_, std::move(frame));

Related Issues

#20671 cc @cyanglaz .

Tests

I added the following tests:

N/A.

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

• I read the contributor guide and followed the process outlined there for submitting PRs.
• I signed the CLA.
• I read and followed the C++, Objective-C, Java style guides for the engine.
• I read the tree hygiene wiki page, which explains my responsibilities.
• I updated/added relevant documentation.
• All existing and new tests are passing.
• I am willing to follow-up on review comments in a timely manner.

Reviewer Checklist

• I have submitted any presubmit flakes in this PR using the engine presubmit flakes form before re-triggering the failure.

Breaking Change

Did any tests fail when you ran them? Please read handling breaking changes.

• No, no existing tests failed, so this is not a breaking change.
• Yes, this is a breaking change. If not, delete the remainder of this section.
  • I wrote a design doc: https://flutter.dev/go/template Replace this with a link to your design doc's short link
  • I got input from the developer relations team, specifically from: Replace with the names of who gave advice
  • I wrote a migration guide: https://flutter.dev/go/breaking-changes-template Replace this with a link to a pull request that adds the migration guide to https://flutter.dev/docs/release/breaking-changes

[flutter-dashboard]

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

[gaaclarke]

This looks good to me. @blasten or @cyanglaz do you want to verify? The diff is really weird for this PR but more simply put it moves the following calls under a if (num_platform_views > 0) check:

    SkCanvas* background_canvas = frame->SkiaCanvas();

    // Resolve all pending GPU operations before allocating a new surface.
    background_canvas->flush();
    // Clipping the background canvas before drawing the picture recorders requires to
    // save and restore the clip context.
    SkAutoCanvasRestore save(background_canvas, /*doSave=*/true);

I verified that the failures in CI below were infra flakes and I've filed them with the form.

I was doing performance checks last week and was surprised to find platform channels showing up in my evaluations despite having none so this is a real problem (cc @xster you probably saw this, too).

[blasten]

cool. Android has this check too. Any chance we add a test for this?

[zhongwuzw]

No idea about how to add a test. :)

[chinmaygarde]

@blasten Can you answer this query so progress is unblocked here?

[gaaclarke]

#22275 will alleviate some of the problems this PR addresses since we won't be going into this code if we don't have merged raster/platform threads. There probably aren't many clients that have merged raster/platform threads and don't have platform views I'd imagine. You can check out that PR for examples on adding tests for platform views as well.

[chinmaygarde]

There probably aren't many clients that have merged raster/platform threads and don't have platform views I'd imagine.

Correct. There are some platforms that use the embedder API but that is not relevant to this commit. I agree that #22275 is the right way of going about achieve this.