LockCompositingSurface to make sure frame is not evicted.

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 <utility>
 
@@ skipping 852 matching lines @@
   host_->SetBackgroundOpaque(GetBackgroundOpaque());
   UpdateBackgroundColor(color);
 }
 
 void RenderWidgetHostViewAndroid::CopyFromCompositingSurface(
     const gfx::Rect& src_subrect,
     const gfx::Size& dst_size,
     const ReadbackRequestCallback& callback,
     const SkColorType preferred_color_type) {
   TRACE_EVENT0("cc", "RenderWidgetHostViewAndroid::CopyFromCompositingSurface");
-  if (!host_ || host_->is_hidden()) {
+  if (!host_ || host_->is_hidden() || !IsSurfaceAvailableForCopy()) {
     callback.Run(SkBitmap(), READBACK_SURFACE_UNAVAILABLE);
     return;
   }
-  base::TimeTicks start_time = base::TimeTicks::Now();
-  if (!IsSurfaceAvailableForCopy()) {
-    callback.Run(SkBitmap(), READBACK_SURFACE_UNAVAILABLE);
+  if (!content_view_core_ || !(content_view_core_->GetWindowAndroid())) {
+    callback.Run(SkBitmap(), READBACK_FAILED);
     return;
   }
+
+  base::TimeTicks start_time = base::TimeTicks::Now();
   const display::Display& display =
       display::Screen::GetScreen()->GetPrimaryDisplay();
   float device_scale_factor = display.device_scale_factor();
   gfx::Size dst_size_in_pixel =
       gfx::ConvertRectToPixel(device_scale_factor, gfx::Rect(dst_size)).size();
   gfx::Rect src_subrect_in_pixel =
       gfx::ConvertRectToPixel(device_scale_factor, src_subrect);
 
   if (!using_browser_compositor_) {
     SynchronousCopyContents(src_subrect_in_pixel, dst_size_in_pixel, callback,
                             preferred_color_type);
     UMA_HISTOGRAM_TIMES("Compositing.CopyFromSurfaceTimeSynchronous",
                         base::TimeTicks::Now() - start_time);
     return;
   }
 
-  if (!content_view_core_ || !(content_view_core_->GetWindowAndroid())) {
-    callback.Run(SkBitmap(), READBACK_FAILED);
-    return;
-  }
   ui::WindowAndroidCompositor* compositor =
       content_view_core_->GetWindowAndroid()->GetCompositor();
   DCHECK(compositor);
   DCHECK(!surface_id_.is_null());
   std::unique_ptr<cc::CopyOutputRequest> request =
-      cc::CopyOutputRequest::CreateRequest(
-          base::Bind(&PrepareTextureCopyOutputResult, dst_size_in_pixel,
-                     preferred_color_type, start_time, callback));
+      cc::CopyOutputRequest::CreateRequest(base::Bind(
+          &PrepareTextureCopyOutputResult, AsWeakPtr(), dst_size_in_pixel,
+          preferred_color_type, start_time, callback));
   if (!src_subrect_in_pixel.IsEmpty())
     request->set_area(src_subrect_in_pixel);
+  // Lock the surface before placing the CopyOutputRequest.

[no sievers, 2016/06/01 22:32:06]

nit: I'd say 'Make sure the layer doesn't get deleted until we fulfill the
request' or something like that

[sivag, 2016/06/02 14:34:50]

Done.

+  LockCompositingSurface();
   layer_->RequestCopyOfOutput(std::move(request));
 }
 
 void RenderWidgetHostViewAndroid::CopyFromCompositingSurfaceToVideoFrame(
     const gfx::Rect& src_subrect,
     const scoped_refptr<media::VideoFrame>& target,
     const base::Callback<void(const gfx::Rect&, bool)>& callback) {
   NOTIMPLEMENTED();
   callback.Run(gfx::Rect(), false);
 }
@@ skipping 999 matching lines @@
   DCHECK(ack_callbacks_.empty());
 }
 
 // TODO(wjmaclean): There is significant overlap between
 // PrepareTextureCopyOutputResult and CopyFromCompositingSurfaceFinished in
 // this file, and the versions in surface_utils.cc. They should
 // be merged. See https://crbug.com/582955
 
 // static
 void RenderWidgetHostViewAndroid::PrepareTextureCopyOutputResult(
+    base::WeakPtr<RenderWidgetHostViewAndroid> rwhva,
     const gfx::Size& dst_size_in_pixel,
     SkColorType color_type,
     const base::TimeTicks& start_time,
     const ReadbackRequestCallback& callback,
     std::unique_ptr<cc::CopyOutputResult> result) {
   base::ScopedClosureRunner scoped_callback_runner(
       base::Bind(callback, SkBitmap(), READBACK_FAILED));
   TRACE_EVENT0("cc",
                "RenderWidgetHostViewAndroid::PrepareTextureCopyOutputResult");
-
+  if (!rwhva)

[no sievers, 2016/06/01 22:32:06]

When RWHVA goes away the layer goes away, which means the CopyOutputRequest goes
away. Then again we have to be careful with what we're doing when inside the
RWHVA destructor. RWHVA already unlocks the compositing surface during teardown.

Maybe it should just be?
if (rwhva)
  rwhva->UnlockCompositingSurface();

Because we can still return the result at this point if it's valid.

[sivag, 2016/06/02 14:34:50]

Done.

+    return;
+  // Unlock surface after recieving texture as response.
+  rwhva->UnlockCompositingSurface();
   if (!result->HasTexture() || result->IsEmpty() || result->size().IsEmpty())
     return;
+  cc::TextureMailbox texture_mailbox;
+  std::unique_ptr<cc::SingleReleaseCallback> release_callback;
+  result->TakeTexture(&texture_mailbox, &release_callback);
+  DCHECK(texture_mailbox.IsTexture());
+  if (!texture_mailbox.IsTexture())
+    return;
+  display_compositor::GLHelper* gl_helper = GetPostReadbackGLHelper();
+  if (!gl_helper)
+    return;
+  if (!gl_helper->IsReadbackConfigSupported(color_type))
+    color_type = kRGBA_8888_SkColorType;
 
   gfx::Size output_size_in_pixel;
   if (dst_size_in_pixel.IsEmpty())
     output_size_in_pixel = result->size();
   else
     output_size_in_pixel = dst_size_in_pixel;
 
-  display_compositor::GLHelper* gl_helper = GetPostReadbackGLHelper();
-  if (!gl_helper)
-    return;
-  if (!gl_helper->IsReadbackConfigSupported(color_type))
-    color_type = kRGBA_8888_SkColorType;
   std::unique_ptr<SkBitmap> bitmap(new SkBitmap);
   if (!bitmap->tryAllocPixels(SkImageInfo::Make(output_size_in_pixel.width(),
                                                 output_size_in_pixel.height(),
                                                 color_type,
                                                 kOpaque_SkAlphaType))) {
     scoped_callback_runner.Reset(
         base::Bind(callback, SkBitmap(), READBACK_BITMAP_ALLOCATION_FAILURE));
     return;
   }
 
   std::unique_ptr<SkAutoLockPixels> bitmap_pixels_lock(
       new SkAutoLockPixels(*bitmap));
   uint8_t* pixels = static_cast<uint8_t*>(bitmap->getPixels());
 
-  cc::TextureMailbox texture_mailbox;
-  std::unique_ptr<cc::SingleReleaseCallback> release_callback;
-  result->TakeTexture(&texture_mailbox, &release_callback);
-  DCHECK(texture_mailbox.IsTexture());
-  if (!texture_mailbox.IsTexture())
-    return;
-
   ignore_result(scoped_callback_runner.Release());
 
   gl_helper->CropScaleReadbackAndCleanMailbox(
       texture_mailbox.mailbox(), texture_mailbox.sync_token(), result->size(),
       gfx::Rect(result->size()), output_size_in_pixel, pixels, color_type,
       base::Bind(&CopyFromCompositingSurfaceFinished, callback,
                  base::Passed(&release_callback), base::Passed(&bitmap),
                  start_time, base::Passed(&bitmap_pixels_lock)),
       display_compositor::GLHelper::SCALER_QUALITY_GOOD);
 }
@@ skipping 60 matching lines @@
     case ui::MotionEvent::ACTION_UP:
     case ui::MotionEvent::ACTION_POINTER_UP:
       UMA_HISTOGRAM_CUSTOM_COUNTS("Event.Latency.OS.TOUCH_RELEASED",
                                   delta.InMicroseconds(), 1, 1000000, 50);
     default:
       return;
   }
 }
 
 }  // namespace content