Add resize gutter layers to DelegatedFrameHost

content/browser/compositor/delegated_frame_host.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 "content/browser/compositor/delegated_frame_host.h"
 
 #include <algorithm>
 #include <string>
 #include <utility>
 #include <vector>
@@ skipping 51 matching lines @@
 ////////////////////////////////////////////////////////////////////////////////
 // DelegatedFrameHost
 
 DelegatedFrameHost::DelegatedFrameHost(DelegatedFrameHostClient* client)
     : client_(client),
       compositor_(nullptr),
       tick_clock_(new base::DefaultTickClock()),
       last_output_surface_id_(0),
       pending_delegated_ack_count_(0),
       skipped_frames_(false),
+      background_color_(SK_ColorWHITE),

[danakj, 2016/03/08 00:09:43]

There's some work to be done to not show the wrong color during navigation:
https://bugs.chromium.org/p/chromium/issues/detail?id=470669

And this adds yet another WHITE constant somewhere, as well as another place
that decides on the right color to use, which I'm worried will cause some weird
stuff like black background white gutters sometimes or something. Can you
alleviate my fears or find a way to get a shared value for this?

[jbauman, 2016/03/08 01:45:29]

Actually this initialization isn't really necessary (I just did it because I
wanted to be sure we weren't reading uninitialized memory in crazy cases - I
could remove it).

We only create the gutters after we've received a frame from the renderer which
informs us of the metadata.root_background_color. That color was originally
intended to be used to create resize gutters, and we already use it for that
purpose on android. It's not guaranteed to always be correct, but in general it
seems pretty good.

[danakj, 2016/03/10 22:53:18]

Can you make it some more crazy noticeable color then? Red or Yellow or
something.

       current_scale_factor_(1.f),
       can_lock_compositor_(YES_CAN_LOCK),
       delegated_frame_evictor_(new DelegatedFrameEvictor(this)) {
   ImageTransportFactory* factory = ImageTransportFactory::GetInstance();
   factory->AddObserver(this);
   id_allocator_ = factory->GetContextFactory()->CreateSurfaceIdAllocator();
 }
 
 void DelegatedFrameHost::WasShown(const ui::LatencyInfo& latency_info) {
   delegated_frame_evictor_->SetVisible(true);
@@ skipping 164 matching lines @@
 
   return size_in_dip != resize_lock_->expected_size();
 }
 
 void DelegatedFrameHost::WasResized() {
   if (client_->DelegatedFrameHostDesiredSizeInDIP() !=
           current_frame_size_in_dip_ &&
       !client_->DelegatedFrameHostIsVisible())
     EvictDelegatedFrame();
   MaybeCreateResizeLock();
+  UpdateGutters();
+}
+
+void DelegatedFrameHost::UpdateGutters() {
+  if (surface_id_.is_null()) {
+    right_gutter_.reset();
+    bottom_gutter_.reset();
+    return;
+  }
+  if (current_frame_size_in_dip_.width() <
+      client_->DelegatedFrameHostDesiredSizeInDIP().width()) {
+    // The right gutter also include the bottom-right corner, if necessary.

[danakj, 2016/03/08 00:09:44]

includes

[jbauman, 2016/03/08 01:45:29]

Done.

+    right_gutter_.reset(new ui::Layer(ui::LAYER_SOLID_COLOR));
+    right_gutter_->SetColor(background_color_);
+    int width = client_->DelegatedFrameHostDesiredSizeInDIP().width() -
+                current_frame_size_in_dip_.width();
+    right_gutter_->SetBounds(
+        gfx::Rect(current_frame_size_in_dip_.width(), 0, width,
+                  client_->DelegatedFrameHostDesiredSizeInDIP().height()));

[danakj, 2016/03/08 00:09:44]

nit: put this value in a temp |height| and move the comment above down onto that
variable?

[jbauman, 2016/03/08 01:45:29]

Done.

+
+    client_->DelegatedFrameHostGetLayer()->Add(right_gutter_.get());

[danakj, 2016/03/08 00:09:43]

One design question.. could cc::SurfaceLayer just show the background color as
solid color quads gutters from its AppendQuads? Does it have the info it would
need to do so? Would that be simpler or worse?

[jbauman, 2016/03/08 01:45:29]

I've thought about it, but currently the SurfaceLayer doesn't know the right
size or the correct background color to use (because it doesn't get the frame
data itself). So it's possible, but I don't know that it would actually make
more sense.

+  } else {
+    right_gutter_.reset();
+  }
+
+  if (current_frame_size_in_dip_.height() <
+      client_->DelegatedFrameHostDesiredSizeInDIP().height()) {
+    bottom_gutter_.reset(new ui::Layer(ui::LAYER_SOLID_COLOR));
+    bottom_gutter_->SetColor(background_color_);
+    int height = client_->DelegatedFrameHostDesiredSizeInDIP().height() -
+                 current_frame_size_in_dip_.height();
+    bottom_gutter_->SetBounds(gfx::Rect(0, current_frame_size_in_dip_.height(),
+                                        current_frame_size_in_dip_.width(),

[danakj, 2016/03/08 00:09:43]

nit: put this into a |width| var too, it's nice to compute them both beside each
other?

[jbauman, 2016/03/08 01:45:29]

Done.

+                                        height));
+    client_->DelegatedFrameHostGetLayer()->Add(bottom_gutter_.get());
+
+  } else {
+    bottom_gutter_.reset();
+  }
 }
 
 gfx::Size DelegatedFrameHost::GetRequestedRendererSize() const {
   if (resize_lock_)
     return resize_lock_->expected_size();
   else
     return client_->DelegatedFrameHostDesiredSizeInDIP();
 }
 
 void DelegatedFrameHost::CheckResizeLock() {
@@ skipping 127 matching lines @@
 
     surface_factory_.reset();
     if (!surface_returned_resources_.empty())
       SendReturnedDelegatedResources(last_output_surface_id_);
 
     last_output_surface_id_ = output_surface_id;
   }
   bool skip_frame = false;
   pending_delegated_ack_count_++;
 
+  background_color_ = frame->metadata.root_background_color;
+
   if (frame_size.IsEmpty()) {
     DCHECK(frame_data->resource_list.empty());
     EvictDelegatedFrame();
   } else {
     ImageTransportFactory* factory = ImageTransportFactory::GetInstance();
     cc::SurfaceManager* manager = factory->GetSurfaceManager();
     if (!surface_factory_) {
       surface_factory_ =
           make_scoped_ptr(new cc::SurfaceFactory(manager, this));
     }
@@ skipping 27 matching lines @@
       ack_callback = base::Bind(&DelegatedFrameHost::SurfaceDrawn,
                                 AsWeakPtr(), output_surface_id);
     }
     surface_factory_->SubmitCompositorFrame(surface_id_, std::move(frame),
                                             ack_callback);
   }
   released_front_lock_ = NULL;
   current_frame_size_in_dip_ = frame_size_in_dip;
   CheckResizeLock();
 
+  UpdateGutters();
+
   if (!damage_rect_in_dip.IsEmpty())
     client_->DelegatedFrameHostGetLayer()->OnDelegatedFrameDamage(
         damage_rect_in_dip);

[danakj, 2016/03/08 00:09:43]

When if ever should damage include the gutters?

[jbauman, 2016/03/08 01:45:29]

Never, AFAIK. This is only supposed to use damage from the delegated frame, and
in practice we use this only to detect whether a video is playing or not, so
including the gutters doesn't make sense.

 
   // Note that |compositor_| may be reset by SetShowSurface or
   // SetShowDelegatedContent above.
   if (!compositor_ || skip_frame) {
     SendDelegatedFrameAck(output_surface_id);
   } else {
     can_lock_compositor_ = NO_PENDING_COMMIT;
   }
   if (!surface_id_.is_null())
     delegated_frame_evictor_->SwappedFrame(
@@ skipping 54 matching lines @@
   // TODO(tansell): Hook this up.
 }
 
 void DelegatedFrameHost::EvictDelegatedFrame() {
   client_->DelegatedFrameHostGetLayer()->SetShowSolidColorContent();
   if (!surface_id_.is_null()) {
     surface_factory_->Destroy(surface_id_);
     surface_id_ = cc::SurfaceId();
   }
   delegated_frame_evictor_->DiscardedFrame();
+  UpdateGutters();
 }
 
 // static
 void DelegatedFrameHost::ReturnSubscriberTexture(
     base::WeakPtr<DelegatedFrameHost> dfh,
     scoped_refptr<OwnedMailbox> subscriber_texture,
     const gpu::SyncToken& sync_token) {
   if (!subscriber_texture.get())
     return;
   if (!dfh)
@@ skipping 299 matching lines @@
     cc::SurfaceManager* manager = factory->GetSurfaceManager();
     new_layer->SetShowSurface(
         surface_id_, base::Bind(&SatisfyCallback, base::Unretained(manager)),
         base::Bind(&RequireCallback, base::Unretained(manager)),
         current_surface_size_, current_scale_factor_,
         current_frame_size_in_dip_);
   }
 }
 
 }  // namespace content