Expect presentation only if swap buffers will happen

There are situations when viz::Display::DrawAndSwap will Draw but will
skip Swap (when resize is happening and we have CopyOutput requests).
For this frame presentation callback will not be called.

This CL moves creation of PresentationGroupTiming for frame only if
|should_swap| is true to avoid waiting for presentation that will
never happen.

Fixed: 1042938
Change-Id: I508679b2e5c7d2dceeba3c915479db7617d432ea
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2008354
Commit-Queue: Vasiliy Telezhnikov <vasilyt@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#734004}

components/viz/service/display/display.cc

@@ -608,7 +608,12 @@ bool Display::DrawAndSwap(base::TimeTicks expected_display_time) {
                                 draw_timer->Elapsed().InMicroseconds());
         break;
     }
+  } else {
+    TRACE_EVENT_INSTANT0("viz", "Draw skipped.", TRACE_EVENT_SCOPE_THREAD);
+  }
 
+  bool should_swap = should_draw && size_matches;
+  if (should_swap) {
     PresentationGroupTiming presentation_group_timing;
     presentation_group_timing.OnDraw(draw_timer->Begin());
 
@@ -624,12 +629,7 @@ bool Display::DrawAndSwap(base::TimeTicks expected_display_time) {
     }
     pending_presentation_group_timings_.emplace_back(
         std::move(presentation_group_timing));
-  } else {
-    TRACE_EVENT_INSTANT0("viz", "Draw skipped.", TRACE_EVENT_SCOPE_THREAD);
-  }
 
-  bool should_swap = should_draw && size_matches;
-  if (should_swap) {
     TRACE_EVENT_ASYNC_STEP_INTO0("viz,benchmark",
                                  "Graphics.Pipeline.DrawAndSwap",
                                  swapped_trace_id_, "WaitForSwap");

components/viz/service/display/display.h

@@ -176,6 +176,7 @@ class VIZ_SERVICE_EXPORT Display : public DisplaySchedulerClient,
   bool HasPendingSurfaces(const BeginFrameArgs& args) const;
 
  private:
+  friend class DisplayTest;
   // PresentationGroupTiming stores rendering pipeline stage timings associated
   // with a call to Display::DrawAndSwap along with a list of
   // Surface::PresentationHelper's for each aggregated Surface that will be

components/viz/service/display/display_unittest.cc

@@ -213,6 +213,10 @@ class DisplayTest : public testing::Test {
 
   void LatencyInfoCapTest(bool over_capacity);
 
+  size_t pending_presentation_group_timings_size() {
+    return display_->pending_presentation_group_timings_.size();
+  }
+
   ServerSharedBitmapManager shared_bitmap_manager_;
   FrameSinkManagerImpl manager_;
   std::unique_ptr<CompositorFrameSinkSupport> support_;
@@ -4142,4 +4146,54 @@ TEST_F(DisplayTest, DisplayTransformHint) {
   TearDownDisplay();
 }
 
+TEST_F(DisplayTest, DisplaySizeMismatch) {
+  RendererSettings settings;
+  settings.partial_swap_enabled = true;
+  settings.auto_resize_output_surface = false;
+  SetUpSoftwareDisplay(settings);
+
+  StubDisplayClient client;
+  display_->Initialize(&client, manager_.surface_manager());
+
+  id_allocator_.GenerateId();
+  display_->SetLocalSurfaceId(
+      id_allocator_.GetCurrentLocalSurfaceIdAllocation().local_surface_id(),
+      1.f);
+  display_->Resize(gfx::Size(100, 100));
+
+  // Pass has copy output request but wrong size so it should be drawn, but not
+  // swapped.
+  {
+    std::unique_ptr<RenderPass> pass = RenderPass::Create();
+    pass->output_rect = gfx::Rect(0, 0, 99, 99);
+    pass->damage_rect = gfx::Rect(10, 10, 0, 0);
+    bool copy_called = false;
+    pass->copy_requests.push_back(std::make_unique<CopyOutputRequest>(
+        CopyOutputRequest::ResultFormat::RGBA_BITMAP,
+        base::BindOnce(&CopyCallback, &copy_called)));
+    pass->id = 1u;
+
+    RenderPassList pass_list;
+    pass_list.push_back(std::move(pass));
+
+    SubmitCompositorFrame(
+        &pass_list,
+        id_allocator_.GetCurrentLocalSurfaceIdAllocation().local_surface_id());
+    EXPECT_TRUE(scheduler_->damaged());
+
+    display_->DrawAndSwap(base::TimeTicks::Now());
+
+    // Expect no swap happen
+    EXPECT_EQ(0u, output_surface_->num_sent_frames());
+
+    // Expect draw and copy output request happen
+    EXPECT_TRUE(copy_called);
+
+    // Expect there is no pending
+    EXPECT_EQ(pending_presentation_group_timings_size(), 0u);
+  }
+
+  TearDownDisplay();
+}
+
 }  // namespace viz