Getting rid of DelegatedFrameData

cc/surfaces/display.cc

 // 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 "cc/surfaces/display.h"
 
 #include <stddef.h>
 
 #include "base/memory/ptr_util.h"
 #include "base/trace_event/trace_event.h"
@@ skipping 196 matching lines @@
   // overlays.
   bool output_partial_list = renderer_->use_partial_swap() &&
                              !output_surface_->GetOverlayCandidateValidator();
   aggregator_.reset(new SurfaceAggregator(
       surface_manager_, resource_provider_.get(), output_partial_list));
   aggregator_->set_output_is_secure(output_is_secure_);
 }
 
 void Display::UpdateRootSurfaceResourcesLocked() {
   Surface* surface = surface_manager_->GetSurfaceForId(current_surface_id_);
-  bool root_surface_resources_locked =
-      !surface || !surface->GetEligibleFrame().delegated_frame_data;
+  bool root_surface_resources_locked = !surface || !surface->HasFrame();

[Fady Samuel, 2016/11/16 00:38:23]

I think the right check here is:

root_surface_resources_locked = !surface || !surface->HasFrame() ||
surface->GetEligibleFrame().render_pass_list.empty();

[danakj, 2016/11/16 01:23:17]

Why? The old code did not check for an empty render_pass_list.

[Saman Sami, 2016/11/16 01:40:26]

It didn't explicitly, what we believe it meant to check for it. Could you take a
look at John's comment here:
https://bugs.chromium.org/p/chromium/issues/detail?id=663516#c12

[danakj, 2016/11/16 01:56:17]

I see, this should be false if an empty frame was submitted to reclaim
resources?

This is because ForceReclaimResources() now has to way to submit a "null" frame
and is submitting an "empty" frame instead? Perhaps it should be doing something
else that would cause HasFrame to return false.

   if (scheduler_)
     scheduler_->SetRootSurfaceResourcesLocked(root_surface_resources_locked);
 }
 
 void Display::DidLoseContextProvider() {
   if (scheduler_)
     scheduler_->OutputSurfaceLost();
   // WARNING: The client may delete the Display in this method call. Do not
   // make any additional references to members after this call.
   client_->DisplayOutputSurfaceLost();
 }
 
 bool Display::DrawAndSwap() {
   TRACE_EVENT0("cc", "Display::DrawAndSwap");
 
   if (current_surface_id_.is_null()) {
     TRACE_EVENT_INSTANT0("cc", "No root surface.", TRACE_EVENT_SCOPE_THREAD);
     return false;
   }
 
   if (!output_surface_) {
     TRACE_EVENT_INSTANT0("cc", "No output surface", TRACE_EVENT_SCOPE_THREAD);
     return false;
   }
 
   CompositorFrame frame = aggregator_->Aggregate(current_surface_id_);
-  if (!frame.delegated_frame_data) {
+
+  if (frame.render_pass_list.empty()) {
     TRACE_EVENT_INSTANT0("cc", "Empty aggregated frame.",
                          TRACE_EVENT_SCOPE_THREAD);
     return false;
   }
 
   // Run callbacks early to allow pipelining.
   for (const auto& id_entry : aggregator_->previous_contained_surfaces()) {
     Surface* surface = surface_manager_->GetSurfaceForId(id_entry.first);
     if (surface)
       surface->RunDrawCallbacks();
   }
 
-  DelegatedFrameData* frame_data = frame.delegated_frame_data.get();
-
   frame.metadata.latency_info.insert(frame.metadata.latency_info.end(),
                                      stored_latency_info_.begin(),
                                      stored_latency_info_.end());
   stored_latency_info_.clear();
   bool have_copy_requests = false;
-  for (const auto& pass : frame_data->render_pass_list) {
+  for (const auto& pass : frame.render_pass_list) {
     have_copy_requests |= !pass->copy_requests.empty();
   }
 
   gfx::Size surface_size;
   bool have_damage = false;
-  if (!frame_data->render_pass_list.empty()) {
-    RenderPass& last_render_pass = *frame_data->render_pass_list.back();
+  if (!frame.render_pass_list.empty()) {
+    RenderPass& last_render_pass = *frame.render_pass_list.back();
     if (last_render_pass.output_rect.size() != current_surface_size_ &&
         last_render_pass.damage_rect == last_render_pass.output_rect &&
         !current_surface_size_.IsEmpty()) {
       // Resize the output rect to the current surface size so that we won't
       // skip the draw and so that the GL swap won't stretch the output.
       last_render_pass.output_rect.set_size(current_surface_size_);
       last_render_pass.damage_rect = last_render_pass.output_rect;
     }
     surface_size = last_render_pass.output_rect.size();
     have_damage = !last_render_pass.damage_rect.size().IsEmpty();
   }
 
   bool size_matches = surface_size == current_surface_size_;
   if (!size_matches)
     TRACE_EVENT_INSTANT0("cc", "Size mismatch.", TRACE_EVENT_SCOPE_THREAD);
 
   bool should_draw = have_copy_requests || (have_damage && size_matches);
 
   // If the surface is suspended then the resources to be used by the draw are
   // likely destroyed.
   if (output_surface_->SurfaceIsSuspendForRecycle()) {
     TRACE_EVENT_INSTANT0("cc", "Surface is suspended for recycle.",
                          TRACE_EVENT_SCOPE_THREAD);
     should_draw = false;
   }
 
-  client_->DisplayWillDrawAndSwap(should_draw, frame_data->render_pass_list);
+  client_->DisplayWillDrawAndSwap(should_draw, frame.render_pass_list);
 
   if (should_draw) {
     bool disable_image_filtering =
         frame.metadata.is_resourceless_software_draw_with_scroll_or_animation;
     if (software_renderer_) {
       software_renderer_->SetDisablePictureQuadImageFiltering(
           disable_image_filtering);
     } else {
       // This should only be set for software draws in synchronous compositor.
       DCHECK(!disable_image_filtering);
     }
 
-    renderer_->DecideRenderPassAllocationsForFrame(
-        frame_data->render_pass_list);
-    renderer_->DrawFrame(&frame_data->render_pass_list, device_scale_factor_,
+    renderer_->DecideRenderPassAllocationsForFrame(frame.render_pass_list);
+    renderer_->DrawFrame(&frame.render_pass_list, device_scale_factor_,
                          device_color_space_, current_surface_size_);
   } else {
     TRACE_EVENT_INSTANT0("cc", "Draw skipped.", TRACE_EVENT_SCOPE_THREAD);
   }
 
   bool should_swap = should_draw && size_matches;
   if (should_swap) {
     swapped_since_resize_ = true;
     for (auto& latency : frame.metadata.latency_info) {
       TRACE_EVENT_WITH_FLOW1(
@@ skipping 40 matching lines @@
   aggregator_->SetFullDamageForSurface(current_surface_id_);
   if (scheduler_)
     scheduler_->SurfaceDamaged(current_surface_id_);
 }
 
 void Display::OnSurfaceDamaged(const SurfaceId& surface_id, bool* changed) {
   if (aggregator_ &&
       aggregator_->previous_contained_surfaces().count(surface_id)) {
     Surface* surface = surface_manager_->GetSurfaceForId(surface_id);
     if (surface) {
-      const CompositorFrame& current_frame = surface->GetEligibleFrame();
-      if (!current_frame.delegated_frame_data ||
-          current_frame.delegated_frame_data->resource_list.empty()) {
+      if (!surface->HasFrame() ||
+          surface->GetEligibleFrame().resource_list.empty()) {
         aggregator_->ReleaseResources(surface_id);
       }
     }
     if (scheduler_)
       scheduler_->SurfaceDamaged(surface_id);
     *changed = true;
   } else if (surface_id == current_surface_id_) {
     if (scheduler_)
       scheduler_->SurfaceDamaged(surface_id);
     *changed = true;
@@ skipping 10 matching lines @@
 const SurfaceId& Display::CurrentSurfaceId() {
   return current_surface_id_;
 }
 
 void Display::ForceImmediateDrawAndSwapIfPossible() {
   if (scheduler_)
     scheduler_->ForceImmediateSwapIfPossible();
 }
 
 }  // namespace cc