Move content offset to ViewAndroid

ui/snapshot/snapshot_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 "ui/snapshot/snapshot.h"
 
 #include <memory>
 #include <utility>
 
 #include "base/bind.h"
 #include "cc/output/copy_output_request.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/android/view_android.h"
 #include "ui/android/window_android.h"
 #include "ui/android/window_android_compositor.h"
 #include "ui/display/display.h"
 #include "ui/display/screen.h"
 #include "ui/gfx/geometry/point_conversions.h"
 #include "ui/gfx/geometry/rect_conversions.h"
 #include "ui/snapshot/snapshot_async.h"
 
 namespace ui {
 
 bool GrabViewSnapshot(gfx::NativeView view,
                       std::vector<unsigned char>* png_representation,
                       const gfx::Rect& snapshot_bounds) {
   return GrabWindowSnapshot(

[no sievers, 2016/08/05 18:15:14]

You can actually fix this too here and call GrabViewSnapshotAsync() passing
|view| which will then use the correct offset for that view.

[Jinsuk Kim, 2016/12/22 12:35:48]

This is for synchronous version which is not preferred on Android as per the
comment below. Moved it above these two methods to make it more visible.

       view->GetWindowAndroid(), png_representation, snapshot_bounds);
 }
 
 bool GrabWindowSnapshot(gfx::NativeWindow window,
                         std::vector<unsigned char>* png_representation,
                         const gfx::Rect& snapshot_bounds) {
   // Not supported in Android.  Callers should fall back to the async version.
   return false;
 }
 
 static void MakeAsyncCopyRequest(
     gfx::NativeWindow window,
+    const gfx::Vector2dF& offset,
     const gfx::Rect& source_rect,
     const cc::CopyOutputRequest::CopyOutputRequestCallback& callback) {
   std::unique_ptr<cc::CopyOutputRequest> request =
       cc::CopyOutputRequest::CreateBitmapRequest(callback);
 
   const display::Display& display =
       display::Screen::GetScreen()->GetPrimaryDisplay();
   float device_scale_factor = display.device_scale_factor();
   gfx::Rect source_rect_in_pixel =
       gfx::ScaleToEnclosingRect(source_rect, device_scale_factor);
 
   // Account for the toolbar offset.
-  gfx::Vector2dF offset = window->content_offset();
   gfx::Rect adjusted_source_rect(gfx::ToRoundedPoint(
       gfx::PointF(source_rect_in_pixel.x() + offset.x(),
                   source_rect_in_pixel.y() + offset.y())),

[no sievers, 2016/08/05 19:32:12]

Argh, this code makes no sense.

The caller passes in a rect. Then we somehow adjust it for an offset by
*translating* it (and keeping the size)?

Ok so let's look at the call sites which we support:

1)  For devtools/telemetry/tracing from RenderWidgetHostImpl, see
GrabViewSnapshot*() call sites.

This queries the RWHV for the bounds and passes that in.
Looking at the code this already ends up accounting for the the top control
heights in ContentViewCoreImpl::GetViewSize(). So looks like this is currently
broken.

2) chrome/browser/android/feedback/screenshot_task.cc

This grabs a whole window screenshots ('createCompositorScreenShot') and queries
the Android root view/window for the total size that it passes in. It would then
shift the rect down out of bounds which doesn't make sense. So also broken.

That being said, we should probably remove View/WindowAndroid::content_offset()
instead. Looks like the call sites should (and would already) end up passing in
the correct rect. (And only call site 1) really has a notion of a content
offset.)

I think you can test both call sites. 1) through tracing with frame
tracing/grabbing enabled, and 2) through the feedback reporter.

[no sievers, 2016/08/05 19:38:26]

Ah sorry, I was slightly off:

gfx::Rect RenderWidgetHostViewAndroid::GetViewBounds() const {
  if (!content_view_core_)
    return gfx::Rect(default_size_);

  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
      switches::kEnableOSKOverscroll))
    return gfx::Rect(content_view_core_->GetViewSizeWithOSKHidden());

  return gfx::Rect(content_view_core_->GetViewSize());
}

So content_view_core_->GetViewSize() shrinks for the size but then converts this
to a rect with offset (0,0).

I think this option makes the least sense (vs. either including the  omnibox and
saying the origin is 0, or not including it and saying the content is
translated).

We might want to attempt to change it but have to be a bit careful and look at
all the call sites to see what it might break.

       source_rect_in_pixel.size());
 
   request->set_area(adjusted_source_rect);
   window->GetCompositor()->RequestCopyOfOutputOnRootLayer(std::move(request));
 }
 
 void GrabWindowSnapshotAndScaleAsync(
     gfx::NativeWindow window,
     const gfx::Rect& source_rect,
     const gfx::Size& target_size,
     scoped_refptr<base::TaskRunner> background_task_runner,
     const GrabWindowSnapshotAsyncCallback& callback) {
   MakeAsyncCopyRequest(window,
+                       window->content_offset(),
                        source_rect,
                        base::Bind(&SnapshotAsync::ScaleCopyOutputResult,
                                   callback,
                                   target_size,
                                   background_task_runner));
 }
 
 void GrabWindowSnapshotAsync(
     gfx::NativeWindow window,
     const gfx::Rect& source_rect,
     scoped_refptr<base::TaskRunner> background_task_runner,
     const GrabWindowSnapshotAsyncPNGCallback& callback) {
   MakeAsyncCopyRequest(window,
+                       window->content_offset(),
                        source_rect,
                        base::Bind(&SnapshotAsync::EncodeCopyOutputResult,
                                   callback,
                                   background_task_runner));
 }
 
 void GrabViewSnapshotAsync(
     gfx::NativeView view,
     const gfx::Rect& source_rect,
     scoped_refptr<base::TaskRunner> background_task_runner,
     const GrabWindowSnapshotAsyncPNGCallback& callback) {
-  GrabWindowSnapshotAsync(
-      view->GetWindowAndroid(), source_rect, background_task_runner, callback);
+  MakeAsyncCopyRequest(view->GetWindowAndroid(),
+                       view->content_offset(),
+                       source_rect,
+                       base::Bind(&SnapshotAsync::EncodeCopyOutputResult,
+                                  callback,
+                                  background_task_runner));
 }
 
 }  // namespace ui