Enforce constant size and device scale factor for surfaces

cc/surfaces/surface.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/surface.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <algorithm>
 
 #include "base/stl_util.h"
 #include "cc/output/compositor_frame.h"
 #include "cc/output/copy_output_request.h"
 #include "cc/surfaces/compositor_frame_sink_support.h"
 #include "cc/surfaces/local_surface_id_allocator.h"
 #include "cc/surfaces/surface_manager.h"
 #include "cc/surfaces/surface_resource_holder_client.h"
 
 namespace cc {
 
 // The frame index starts at 2 so that empty frames will be treated as
 // completely damaged the first time they're drawn from.
 static const int kFrameIndexStart = 2;
 
 Surface::Surface(
-    const SurfaceId& id,
+    const SurfaceInfo& surface_info,
     base::WeakPtr<CompositorFrameSinkSupport> compositor_frame_sink_support)
-    : surface_id_(id),
-      previous_frame_surface_id_(id),
+    : surface_info_(surface_info),
+      previous_frame_surface_id_(surface_info.id()),
       compositor_frame_sink_support_(std::move(compositor_frame_sink_support)),
       surface_manager_(compositor_frame_sink_support_->surface_manager()),
       frame_index_(kFrameIndexStart),
       destroyed_(false) {}
 
 Surface::~Surface() {
   ClearCopyRequests();
   surface_manager_->SurfaceDiscarded(this);
 
   UnrefFrameResourcesAndRunDrawCallback(std::move(pending_frame_data_));
@@ skipping 10 matching lines @@
   surface->TakeLatencyInfoFromPendingFrame(&frame.metadata.latency_info);
 }
 
 void Surface::Close() {
   closed_ = true;
 }
 
 void Surface::QueueFrame(CompositorFrame frame,
                          const base::Closure& callback,
                          const WillDrawCallback& will_draw_callback) {
-  if (closed_) {
+  bool reject_frame = closed_;
+
+  gfx::Size frame_size = frame.render_pass_list.back()->output_rect.size();
+  float device_scale_factor = frame.metadata.device_scale_factor;
+
+  if (frame_size != surface_info_.size_in_pixels() ||
+      device_scale_factor != surface_info_.device_scale_factor()) {
+    DLOG(FATAL) << "Rejected CompositorFrame that violates surface invariants.";

[danakj, 2017/05/18 15:15:54]

can this be a trace event also?

[Saman Sami, 2017/05/18 20:44:37]

Done.

+    reject_frame = true;

[danakj, 2017/05/17 20:49:36]

How will this happen in practice? Why isn't this just a DCHECK?

[Saman Sami, 2017/05/17 20:52:08]

If the client misbehaves, for example.

[danakj, 2017/05/17 21:03:28]

Can't we reject that at the IPC boundary instead of here?

[Saman Sami, 2017/05/17 21:22:50]

We don't know the details of the surface at IPC boundary, unfortunately.

[danakj, 2017/05/17 21:25:28]

I guess I am asking how far out we can push it then. This is called by
CFS::SubmitCompositorFrameSink. It could query these and reject the frame
instead of calling Queue? Repeat up the call stack, how far can it go?

[Saman Sami, 2017/05/17 21:39:26]

I guess we can do the verification in CFSS but I'm not sure how much benefit
that provides. We really can't push it further because there is sometimes
nothing further up the call stack, such as in GpuCompositorFrameSink.

[Fady Samuel, 2017/05/18 11:45:25]

+1 There isn't a whole lot up the stack in some cases and moving this to
DelegatedFrameHost doesn't really help us in Mus/Mushrome/Mus+Ash...

+  }
+
+  if (reject_frame) {
     if (compositor_frame_sink_support_) {
       ReturnedResourceArray resources;
       TransferableResource::ReturnResources(frame.resource_list, &resources);
       compositor_frame_sink_support_->ReturnResources(resources);
     }
     callback.Run();
     return;
   }
 
   TakeLatencyInfoFromPendingFrame(&frame.metadata.latency_info);
@@ skipping 228 matching lines @@
     base::Closure callback = active_frame_data_->draw_callback;
     active_frame_data_->draw_callback = base::Closure();
     callback.Run();
   }
 }
 
 void Surface::RunWillDrawCallback(const gfx::Rect& damage_rect) {
   if (!active_frame_data_ || active_frame_data_->will_draw_callback.is_null())
     return;
 
-  active_frame_data_->will_draw_callback.Run(surface_id_.local_surface_id(),
+  active_frame_data_->will_draw_callback.Run(surface_id().local_surface_id(),
                                              damage_rect);
 }
 
 void Surface::AddDestructionDependency(SurfaceSequence sequence) {
   destruction_dependencies_.push_back(sequence);
 }
 
 void Surface::SatisfyDestructionDependencies(
     std::unordered_set<SurfaceSequence, SurfaceSequenceHash>* sequences,
     std::unordered_set<FrameSinkId, FrameSinkIdHash>* valid_frame_sink_ids) {
@@ skipping 45 matching lines @@
     frame->metadata.latency_info.swap(*latency_info);
     return;
   }
   std::copy(frame->metadata.latency_info.begin(),
             frame->metadata.latency_info.end(),
             std::back_inserter(*latency_info));
   frame->metadata.latency_info.clear();
 }
 
 }  // namespace cc