Android:Allow a read-back request to wait for a first frame

content/browser/renderer_host/render_widget_host_view_android.cc

 // Copyright (c) 2012 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/renderer_host/render_widget_host_view_android.h"
 
 #include <android/bitmap.h>
 
 #include "base/basictypes.h"
 #include "base/bind.h"
@@ skipping 194 matching lines @@
       observing_root_window_(false) {
   host_->SetView(this);
   SetContentViewCore(content_view_core);
   ImageTransportFactoryAndroid::AddObserver(this);
 }
 
 RenderWidgetHostViewAndroid::~RenderWidgetHostViewAndroid() {
   ImageTransportFactoryAndroid::RemoveObserver(this);
   SetContentViewCore(NULL);
   DCHECK(ack_callbacks_.empty());
+  DCHECK(readbacks_waiting_for_frame_.empty());
   if (resource_collection_.get())
     resource_collection_->SetClient(NULL);
 }
 
 
 bool RenderWidgetHostViewAndroid::OnMessageReceived(
     const IPC::Message& message) {
   bool handled = true;
   IPC_BEGIN_MESSAGE_MAP(RenderWidgetHostViewAndroid, message)
     IPC_MESSAGE_HANDLER(ViewHostMsg_StartContentIntent, OnStartContentIntent)
@@ skipping 63 matching lines @@
 void RenderWidgetHostViewAndroid::SetSize(const gfx::Size& size) {
   // Ignore the given size as only the Java code has the power to
   // resize the view on Android.
   default_size_ = size;
 }
 
 void RenderWidgetHostViewAndroid::SetBounds(const gfx::Rect& rect) {
   SetSize(rect.size());
 }
 
+void RenderWidgetHostViewAndroid::ProcessPendingReadbackRequests() {

[no sievers, 2014/07/28 17:07:06]

Ok, I see, so this doesn't actually do the readback. Then it should have a
better name too.
Something like Cancel/AbortPendingReadbacks or FailPendingReadbacks.

Or even better: How about getting rid of this method and letting
~ReadbackRequest handle this?
You can add something like ReadbackRequest::TakeCallback() to handle the case
where you actually do the readback and let GetScaledContentBitmap() handle
running the callback.

[sivag, 2014/08/01 10:10:20]

1. changed ProcessPendingReadbackRequests ->AbortPendingReadBackRequests
2. because the request queue is maintained in RWHVA  and we dont want to always
send the callbacks with false in destructor, some times it gets processed via
GetScaledContentBitmap(here copyFromcompositingsurface will return the
callback), except there is some contentt loss or we are hiding.

+  while (!readbacks_waiting_for_frame_.empty()) {
+    ReadbackRequest& readback_request = readbacks_waiting_for_frame_.front();
+    readback_request.GetResultCallback().Run(false, SkBitmap());
+    readbacks_waiting_for_frame_.pop();
+  }
+}
+
+void RenderWidgetHostViewAndroid::WaitForValidFrameArrival(
+    const ReadbackRequest& readback_request) {
+  readbacks_waiting_for_frame_.push(readback_request);

[no sievers, 2014/07/28 17:07:06]

nit: don't need a method for this one line of code.

[sivag, 2014/08/01 10:10:20]

Done.

+}
+
 void RenderWidgetHostViewAndroid::GetScaledContentBitmap(
     float scale,
     SkColorType color_type,
     gfx::Rect src_subrect,
     const base::Callback<void(bool, const SkBitmap&)>& result_callback) {
   if (!host_ || host_->is_hidden()) {
     result_callback.Run(false, SkBitmap());
     return;
   }
   if (!IsSurfaceAvailableForCopy()) {
-    // TODO(Sikugu): allow a read-back request to wait for a first frame if it
-    // was invoked while no frame was received yet
-    result_callback.Run(false, SkBitmap());
+    // The view is visible, probably the frame has not yet arrived.
+    // Just add the ReadbackRequest to queue and wait for frame arrival
+    // to get this request processed.
+    WaitForValidFrameArrival(
+        ReadbackRequest(scale, color_type, src_subrect, result_callback));
+    // TODO(sikugu): Do we need to have a timeout for a readback here?

[no sievers, 2014/07/28 17:07:06]

I don't *think* we need a timeout. We know we are visible and if no frame
arrives, it should be a bug.
Except see comment in Hide().

[sivag, 2014/08/01 10:10:20]

Done.

     return;
   }
 
   gfx::Size bounds = layer_->bounds();
   if (src_subrect.IsEmpty())
     src_subrect = gfx::Rect(bounds);
   DCHECK_LE(src_subrect.width() + src_subrect.x(), bounds.width());
   DCHECK_LE(src_subrect.height() + src_subrect.y(), bounds.height());
   const gfx::Display& display =
       gfx::Screen::GetNativeScreen()->GetPrimaryDisplay();
   float device_scale_factor = display.device_scale_factor();
   DCHECK_GT(device_scale_factor, 0);
   gfx::Size dst_size(
       gfx::ToCeiledSize(gfx::ScaleSize(bounds, scale / device_scale_factor)));
   CopyFromCompositingSurface(
       src_subrect, dst_size, result_callback, color_type);
 }
 
 bool RenderWidgetHostViewAndroid::HasValidFrame() const {
   if (!content_view_core_)
     return false;
   if (!layer_)
     return false;
 
   if (texture_size_in_layer_.IsEmpty())
     return false;
-
+  // This tell us whether a valid frame has arrived or not.
   if (!frame_evictor_->HasFrame())
     return false;
 
   return true;
 }
 
 gfx::NativeView RenderWidgetHostViewAndroid::GetNativeView() const {
   return content_view_core_->GetViewAndroid();
 }
 
@@ skipping 54 matching lines @@
 
 void RenderWidgetHostViewAndroid::Hide() {
   if (!is_showing_)
     return;
 
   is_showing_ = false;
   if (layer_ && locks_on_frame_count_ == 0)
     layer_->SetHideLayerAndSubtree(true);
 
   frame_evictor_->SetVisible(false);
+  // We are hiding a renderer and any readback requests pending on frame arrival
+  // need to be addressed here.
+  ProcessPendingReadbackRequests();

[no sievers, 2014/07/28 17:07:06]

It would be nice if we could allow deferring the readback if the surface was
locked.
But then I'm guessing it's not guaranteed that you will ever see a frame if you
quickly show and hide the view?

[no sievers, 2014/07/28 17:08:33]

Scrap that comment. We don't allow locking the surface if there is no frame.
So the app already has to deal with this and differentiate anyways.

[sivag, 2014/08/01 10:10:20]

Done.

[sivag, 2014/08/01 10:10:20]

Done.

   WasHidden();
 }
 
 bool RenderWidgetHostViewAndroid::IsShowing() {
   // ContentViewCoreImpl represents the native side of the Java
   // ContentViewCore.  It being NULL means that it is not attached
   // to the View system yet, so we treat this RWHVA as hidden.
   return is_showing_ && content_view_core_;
 }
 
@@ skipping 408 matching lines @@
 void RenderWidgetHostViewAndroid::UnusedResourcesAreAvailable() {
   if (ack_callbacks_.size())
     return;
   SendReturnedDelegatedResources(last_output_surface_id_);
 }
 
 void RenderWidgetHostViewAndroid::DestroyDelegatedContent() {
   RemoveLayers();
   frame_provider_ = NULL;
   layer_ = NULL;
+  ProcessPendingReadbackRequests();

[no sievers, 2014/07/28 17:07:06]

Which case is this handling?
I'm not sure it's always right, or even if we should cancel the readbacks here
at all. Currently there are 4 call sites:

1) EvictDelegatedFrame(): Doesn't seem like this should happen. If you had a
frame you did the readback rightaway. If you don't have a frame, I think
EvictDelegatedFrame() should not get called.

2) OnLostResources(): not sure; it shouldn't stop drawing (or we have other
problems) and we can complete the readback when the new context generates the
first frame

3) SwapDelegatedFrame() line 877: I think it's same as 2). Should only happen
when a context was lost.

4) SwapDelegatedFrame() line 891: That's an interesting one. I don't know when
that happens. Same issue though, if we never get a new good frame very soon,
then something is wrong. (Unless we went hidden.)

[sivag, 2014/08/01 10:10:20]

If any of above functions gets triggered, they will call
DestroyDelegatedContent  where we abortpendingreadbacks.

Relevant DCHECKS added, to counter any unprocessed readbbacks.

 }
 
 void RenderWidgetHostViewAndroid::SwapDelegatedFrame(
     uint32 output_surface_id,
     scoped_ptr<cc::DelegatedFrameData> frame_data) {
   bool has_content = !texture_size_in_layer_.IsEmpty();
 
   if (output_surface_id != last_output_surface_id_) {
     // Drop the cc::DelegatedFrameResourceCollection so that we will not return
     // any resources from the old output surface with the new output surface id.
@@ skipping 102 matching lines @@
 
   // As the metadata update may trigger view invalidation, always call it after
   // any potential compositor scheduling.
   OnFrameMetadataUpdated(frame->metadata);
 }
 
 void RenderWidgetHostViewAndroid::OnSwapCompositorFrame(
     uint32 output_surface_id,
     scoped_ptr<cc::CompositorFrame> frame) {
   InternalSwapCompositorFrame(output_surface_id, frame.Pass());
+  if (!readbacks_waiting_for_frame_.empty() && HasValidFrame()) {
+    while (!readbacks_waiting_for_frame_.empty()) {
+      ReadbackRequest& readback_request = readbacks_waiting_for_frame_.front();
+      GetScaledContentBitmap(readback_request.GetScale(),
+                             readback_request.GetColorFormat(),
+                             readback_request.GetCaptureRect(),
+                             readback_request.GetResultCallback());
+      readbacks_waiting_for_frame_.pop();
+    }

[no sievers, 2014/07/28 17:07:06]

Can you move this to the end of InternalSwapCompositorFrame()?

[sivag, 2014/08/01 10:10:20]

Done.

+  }
 }
 
 void RenderWidgetHostViewAndroid::RetainFrame(
     uint32 output_surface_id,
     scoped_ptr<cc::CompositorFrame> frame) {
   DCHECK(locks_on_frame_count_);
 
   // Store the incoming frame so that it can be swapped when all the locks have
   // been released. If there is already a stored frame, then replace and skip
   // the previous one but make sure we still eventually send the ACK. Holding
@@ skipping 669 matching lines @@
   results->orientationAngle = display.RotationAsDegree();
   results->orientationType =
       RenderWidgetHostViewBase::GetOrientationTypeForMobile(display);
   gfx::DeviceDisplayInfo info;
   results->depth = info.GetBitsPerPixel();
   results->depthPerComponent = info.GetBitsPerComponent();
   results->isMonochrome = (results->depthPerComponent == 0);
 }
 
 } // namespace content