Move content offset to ViewAndroid

ui/snapshot/snapshot_android.cc

Index: ui/snapshot/snapshot_android.cc
diff --git a/ui/snapshot/snapshot_android.cc b/ui/snapshot/snapshot_android.cc
index e56ab7399f61f9981393bbdf91306a7ee7bfda1d..cd0af62f45ac131690facc80166dd10d128d6d4f 100644
--- a/ui/snapshot/snapshot_android.cc
+++ b/ui/snapshot/snapshot_android.cc
@@ -37,6 +37,7 @@ bool GrabWindowSnapshot(gfx::NativeWindow window,
 
 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 =
@@ -49,7 +50,6 @@ static void MakeAsyncCopyRequest(
       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.

@@ -66,6 +66,7 @@ void GrabWindowSnapshotAndScaleAsync(
     scoped_refptr<base::TaskRunner> background_task_runner,
     const GrabWindowSnapshotAsyncCallback& callback) {
   MakeAsyncCopyRequest(window,
+                       window->content_offset(),
                        source_rect,
                        base::Bind(&SnapshotAsync::ScaleCopyOutputResult,
                                   callback,
@@ -79,6 +80,7 @@ void GrabWindowSnapshotAsync(
     scoped_refptr<base::TaskRunner> background_task_runner,
     const GrabWindowSnapshotAsyncPNGCallback& callback) {
   MakeAsyncCopyRequest(window,
+                       window->content_offset(),
                        source_rect,
                        base::Bind(&SnapshotAsync::EncodeCopyOutputResult,
                                   callback,
@@ -90,8 +92,12 @@ void GrabViewSnapshotAsync(
     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