New issue
Advanced search Search tips

Issue 760348 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Feature


Sign in to add a comment

Add scale ratio option to CopyOutputRequest

Project Member Reported by m...@chromium.org, Aug 29 2017

Issue description

This is required for moving tab capture into VIZ. The idea is to add an option to request the result of a copy request be uniformly scaled.

Proposed API (addition to viz::CopyOutputRequest):
	
  // Optionally specify that the result should be uniformly scaled. |numerator|
  // and |denominator| describe the scale ratio: Downscale for num < denom,
  // upscale for num > denom, or no scaling for num == denom. The setter method
  // will reduce these values by dividing both by their common muliples (e.g.,
  // 1600:400 will become 4:1).
  void SetScaleRatio(int numerator, int denominator);
  const std::tuple<int, int>& scale_ratio() const { return scale_ratio_; }
  bool is_scaled() const {
    return std::get<0>(scale_ratio_) != std::get<1>(scale_ratio_);
  }

This is follow-up to track an action item from this code review: https://chromium-review.googlesource.com/c/chromium/src/+/637003/6

 
Project Member

Comment 1 by bugdroid1@chromium.org, Sep 9 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0b62c1d3ddf30c98930e19464c22803092868acc

commit 0b62c1d3ddf30c98930e19464c22803092868acc
Author: Yuri Wiitala <miu@chromium.org>
Date: Sat Sep 09 02:08:14 2017

Add skia::ImageOperations::Resize(SkPixmap) alternative.

Changes the base Resize() function to source pixel data from a SkPixmap
rather than a SkBitmap. Provides backwards-compatibility for callers of
the original variant as well. The purpose of this is to avoid having to
copy data into a new SkBitmap before calling Resize() (to be used in an
upcoming CL). Instead, Resize() can now directly source any pixel data
wrapped by a SkPixmap.

Bug:  760348 
Change-Id: Iac0ace63917d06ed16a11c530c8a48033e6de123
Reviewed-on: https://chromium-review.googlesource.com/654418
Commit-Queue: Yuri Wiitala <miu@chromium.org>
Reviewed-by: Mike Klein <mtklein@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500782}
[modify] https://crrev.com/0b62c1d3ddf30c98930e19464c22803092868acc/skia/ext/image_operations.cc
[modify] https://crrev.com/0b62c1d3ddf30c98930e19464c22803092868acc/skia/ext/image_operations.h

Comment 2 by m...@chromium.org, Sep 21 2017

Status: Started (was: Assigned)
Project Member

Comment 3 by bugdroid1@chromium.org, Sep 21 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/edb2ce24a97f7624c4783977982a292efe33bb2b

commit edb2ce24a97f7624c4783977982a292efe33bb2b
Author: Yuri Wiitala <miu@chromium.org>
Date: Thu Sep 21 19:51:17 2017

Lazy-init GLHelperReadbackSupport to eliminate sync calls at startup.

When a GLHelper instance was created, it also constructed a
GLHelperReadbackSupport instance. However, in doing so, there were
certain calls being made on the GL context that were synchronous, which
is: 1) bad to do during init time; and 2) really bad to do if the client
code will never use the results anyway. Therefore, this change simply
lazy-creates the GLHelperReadbackSupport instance upon first use.

Bug:  760348 ,  760351 
Change-Id: I44e63a71331f31b6d88305bb00f29af9928a722e
Reviewed-on: https://chromium-review.googlesource.com/676945
Reviewed-by: Xiangjun Zhang <xjz@chromium.org>
Commit-Queue: Yuri Wiitala <miu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503540}
[modify] https://crrev.com/edb2ce24a97f7624c4783977982a292efe33bb2b/components/viz/common/gl_helper.cc
[modify] https://crrev.com/edb2ce24a97f7624c4783977982a292efe33bb2b/components/viz/common/gl_helper.h

Project Member

Comment 4 by bugdroid1@chromium.org, Sep 21 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a757f6ec69794ab39a3d7f3e18d718bc861252f9

commit a757f6ec69794ab39a3d7f3e18d718bc861252f9
Author: Yuri Wiitala <miu@chromium.org>
Date: Thu Sep 21 23:22:44 2017

GLHelper: Reusable fixed-ratio scaling and YUV-readback pipelines.

Re-wires the API and internals of GLHelperScaling (and
GLHelper::ScalerInterface and GLHelper::YUVReadbackInterface) to allow
client code to create fixed-ratio scaling pipelines. Callers can then
request scaling of any subset of the output space. This is just like the
skia::ImageOperations::Resize() API.

This is a prerequisite need for the referenced crbugs (below).

Rationale: In the old API, callers had to specify exact source and
destination sizes upfront; and fully reconstruct pipelines whenever
either changed. In addition, the old API required whole-number source
rect coordinates; but this prevented using many scaling ratios (e.g.,
3:1) that require sampling from non-whole-numbered source rect
coordinates.

Much of this change is simply plumbing-throught the API changes
everywhere. However, the following were mostly rewritten:

1. [gl_helper_scaling.cc] ScalerImpl now back-computes the source
sampling rect based on the requested result rect. It also works
backwards through the "ScalerImpl chain" to ensure minimaly-sized
intermediate textures are produced.

2. [gl_helper.cc ] YUVReadbackImpl and YUVReadback_MRT were simplified
and able to be merged into one class.

Bug:  760348 ,  754872 
Change-Id: Iba3ed5cf504eeed69eb115b4c38d031db47bebcc
Reviewed-on: https://chromium-review.googlesource.com/669918
Commit-Queue: Yuri Wiitala <miu@chromium.org>
Reviewed-by: Xiangjun Zhang <xjz@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503608}
[modify] https://crrev.com/a757f6ec69794ab39a3d7f3e18d718bc861252f9/components/viz/common/gl_helper.cc
[modify] https://crrev.com/a757f6ec69794ab39a3d7f3e18d718bc861252f9/components/viz/common/gl_helper.h
[modify] https://crrev.com/a757f6ec69794ab39a3d7f3e18d718bc861252f9/components/viz/common/gl_helper_benchmark.cc
[modify] https://crrev.com/a757f6ec69794ab39a3d7f3e18d718bc861252f9/components/viz/common/gl_helper_scaling.cc
[modify] https://crrev.com/a757f6ec69794ab39a3d7f3e18d718bc861252f9/components/viz/common/gl_helper_scaling.h
[modify] https://crrev.com/a757f6ec69794ab39a3d7f3e18d718bc861252f9/components/viz/common/gl_helper_unittest.cc
[modify] https://crrev.com/a757f6ec69794ab39a3d7f3e18d718bc861252f9/components/viz/common/yuv_readback_unittest.cc
[modify] https://crrev.com/a757f6ec69794ab39a3d7f3e18d718bc861252f9/content/browser/compositor/surface_utils.cc
[modify] https://crrev.com/a757f6ec69794ab39a3d7f3e18d718bc861252f9/content/browser/media/capture/aura_window_capture_machine.cc
[modify] https://crrev.com/a757f6ec69794ab39a3d7f3e18d718bc861252f9/content/browser/renderer_host/delegated_frame_host.cc
[modify] https://crrev.com/a757f6ec69794ab39a3d7f3e18d718bc861252f9/content/browser/renderer_host/render_widget_host_view_android.cc

Comment 5 by m...@chromium.org, Sep 23 2017

Blocking: 759310
Project Member

Comment 6 by bugdroid1@chromium.org, Sep 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/33c015fe245e81ae4186db1c05a636d16cf38a8f

commit 33c015fe245e81ae4186db1c05a636d16cf38a8f
Author: Yuri Wiitala <miu@chromium.org>
Date: Thu Sep 28 06:46:12 2017

Add scaling capability to CopyOutputRequests. (Part 1 of 2)

Introduce a new scaling ratio property to CopyOutputRequest that asks
for result bitmaps/textures to be scaled. This is the first of a
two-part change: The CopyOutputRequest API is changed, along with its
corresponding mojo struct traits, and the SoftwareRenderer impl to
support this feature is put in place. (Part 2 will add the GLRenderer
implementation.)

This is a prerequisite change needed for moving tab capture into VIZ, as
well as several clean-up efforts to simplify the snapshot-taking code in
libcontent.

TBR=fsamuel@chromium.org

Bug:  760348 ,  759310 
Change-Id: I98d2f5fdb4ea2b7dfe234f35873a9a0cc56d4ccb
Reviewed-on: https://chromium-review.googlesource.com/679975
Commit-Queue: Yuri Wiitala <miu@chromium.org>
Reviewed-by: enne <enne@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#504927}
[modify] https://crrev.com/33c015fe245e81ae4186db1c05a636d16cf38a8f/components/viz/common/BUILD.gn
[modify] https://crrev.com/33c015fe245e81ae4186db1c05a636d16cf38a8f/components/viz/common/frame_sinks/copy_output_request.cc
[modify] https://crrev.com/33c015fe245e81ae4186db1c05a636d16cf38a8f/components/viz/common/frame_sinks/copy_output_request.h
[modify] https://crrev.com/33c015fe245e81ae4186db1c05a636d16cf38a8f/components/viz/common/frame_sinks/copy_output_result.h
[add] https://crrev.com/33c015fe245e81ae4186db1c05a636d16cf38a8f/components/viz/common/frame_sinks/copy_output_util.cc
[add] https://crrev.com/33c015fe245e81ae4186db1c05a636d16cf38a8f/components/viz/common/frame_sinks/copy_output_util.h
[add] https://crrev.com/33c015fe245e81ae4186db1c05a636d16cf38a8f/components/viz/common/frame_sinks/copy_output_util_unittest.cc
[modify] https://crrev.com/33c015fe245e81ae4186db1c05a636d16cf38a8f/components/viz/service/display/gl_renderer.cc
[modify] https://crrev.com/33c015fe245e81ae4186db1c05a636d16cf38a8f/components/viz/service/display/software_renderer.cc
[modify] https://crrev.com/33c015fe245e81ae4186db1c05a636d16cf38a8f/components/viz/service/display/software_renderer_unittest.cc
[modify] https://crrev.com/33c015fe245e81ae4186db1c05a636d16cf38a8f/services/viz/public/cpp/compositing/copy_output_request_struct_traits.cc
[modify] https://crrev.com/33c015fe245e81ae4186db1c05a636d16cf38a8f/services/viz/public/cpp/compositing/copy_output_request_struct_traits.h
[modify] https://crrev.com/33c015fe245e81ae4186db1c05a636d16cf38a8f/services/viz/public/cpp/compositing/struct_traits_unittest.cc
[modify] https://crrev.com/33c015fe245e81ae4186db1c05a636d16cf38a8f/services/viz/public/interfaces/compositing/copy_output_request.mojom

Comment 7 by m...@chromium.org, Sep 30 2017

Blocking: 770422
Project Member

Comment 8 by bugdroid1@chromium.org, Oct 2 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d3a0b09d1642a912fe3d1d95aada9388b9cfc831

commit d3a0b09d1642a912fe3d1d95aada9388b9cfc831
Author: Yuri Wiitala <miu@chromium.org>
Date: Mon Oct 02 11:00:30 2017

CopyOutputRequest scaling pixel tests for all DirectRenderer impls

Moves the testing recently added to software_renderer_unittest.cc into
its own copy_output_scaling_pixeltest.cc, and then adds a little
boilerplate wrapping to allow all DirectRenderer impls to be tested with
the same code.

In a soon-upcoming CL, the GLRenderer variant of these tests will be
enabled. For now, only SoftwareRenderer is tested; so this change should
not introduce any behavior change (other than gtest naming).

Bug:  760348 
Change-Id: I4442e59d318980cb14809f633ff3e81043b8f198
Reviewed-on: https://chromium-review.googlesource.com/693296
Reviewed-by: enne <enne@chromium.org>
Commit-Queue: Yuri Wiitala <miu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#505572}
[modify] https://crrev.com/d3a0b09d1642a912fe3d1d95aada9388b9cfc831/components/viz/service/BUILD.gn
[add] https://crrev.com/d3a0b09d1642a912fe3d1d95aada9388b9cfc831/components/viz/service/display/copy_output_scaling_pixeltest.cc
[modify] https://crrev.com/d3a0b09d1642a912fe3d1d95aada9388b9cfc831/components/viz/service/display/software_renderer_unittest.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Oct 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/081f9dcc223a535202a52c351392514ad5635650

commit 081f9dcc223a535202a52c351392514ad5635650
Author: Yuri Wiitala <miu@chromium.org>
Date: Fri Oct 06 23:50:16 2017

Add scaling capability to CopyOutputRequests. (Part 2 of 2)

This adds the GLRenderer implementation needed to support "scaling"
CopyOutputRequests. Because this and additional upcoming implementation
are rather "meaty," the decision was made to break out all the code
related to copy request execution into a separate helper class,
GLRendererCopier.

GLRendererCopier not only executes CopyOutputRequests using GL, it also
manages the caching of resources needed to ensure efficient video
performance (i.e., for screen capture use cases). This change should
perform as well, or better, than the previous implementation in the perf
bot waterfall.

This change also adds a plethora of testing to ensure all possible code
paths are exercised, rect calculations and Y-flipping are being done
correctly, caching works as expected, and enables the GLRenderer-variant
of the recently-added scaling tests in copy_output_scaling_pixeltest.cc.

This is a prerequisite change needed for moving tab capture into VIZ, as
well as several clean-up efforts to simplify the snapshot-taking code in
libcontent.

Bug:  760348 ,  759310 
Change-Id: Idff091ec34c8f76bffce859615308084be310abe
Reviewed-on: https://chromium-review.googlesource.com/696489
Commit-Queue: Yuri Wiitala <miu@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#507232}
[modify] https://crrev.com/081f9dcc223a535202a52c351392514ad5635650/components/viz/BUILD.gn
[modify] https://crrev.com/081f9dcc223a535202a52c351392514ad5635650/components/viz/common/frame_sinks/copy_output_request.h
[modify] https://crrev.com/081f9dcc223a535202a52c351392514ad5635650/components/viz/common/frame_sinks/copy_output_result.cc
[modify] https://crrev.com/081f9dcc223a535202a52c351392514ad5635650/components/viz/service/BUILD.gn
[modify] https://crrev.com/081f9dcc223a535202a52c351392514ad5635650/components/viz/service/display/copy_output_scaling_pixeltest.cc
[modify] https://crrev.com/081f9dcc223a535202a52c351392514ad5635650/components/viz/service/display/direct_renderer.cc
[modify] https://crrev.com/081f9dcc223a535202a52c351392514ad5635650/components/viz/service/display/gl_renderer.cc
[modify] https://crrev.com/081f9dcc223a535202a52c351392514ad5635650/components/viz/service/display/gl_renderer.h
[add] https://crrev.com/081f9dcc223a535202a52c351392514ad5635650/components/viz/service/display/gl_renderer_copier.cc
[add] https://crrev.com/081f9dcc223a535202a52c351392514ad5635650/components/viz/service/display/gl_renderer_copier.h
[add] https://crrev.com/081f9dcc223a535202a52c351392514ad5635650/components/viz/service/display/gl_renderer_copier_pixeltest.cc
[add] https://crrev.com/081f9dcc223a535202a52c351392514ad5635650/components/viz/service/display/gl_renderer_copier_unittest.cc
[add] https://crrev.com/081f9dcc223a535202a52c351392514ad5635650/components/viz/test/data/16_color_rects.png
[add] https://crrev.com/081f9dcc223a535202a52c351392514ad5635650/components/viz/test/data/half_of_one_of_16_color_rects.png
[add] https://crrev.com/081f9dcc223a535202a52c351392514ad5635650/components/viz/test/data/one_of_16_color_rects.png

Project Member

Comment 10 by bugdroid1@chromium.org, Oct 12 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d44d9f8090374684ba2d8aa996d3b47f66037fcc

commit d44d9f8090374684ba2d8aa996d3b47f66037fcc
Author: Yuri Wiitala <miu@chromium.org>
Date: Thu Oct 12 22:58:53 2017

Fix copy_output::ComputeResultRect()

The original implementation erroneously operated on the width/height
instead of the right/bottom coordinates. This caused occasional off-by-
one errors in the results when downscaling.

Note that the bug should not have broken anything since nothing is using
the "scaling" CopyOutputRequests yet; and calls to the function are
only made from code paths related to that feature.

Bug:  760348 
Change-Id: I086b97b2f494fa5e74e056bb5eb411d694f79052
Reviewed-on: https://chromium-review.googlesource.com/715156
Reviewed-by: Xiangjun Zhang <xjz@chromium.org>
Commit-Queue: Yuri Wiitala <miu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508518}
[modify] https://crrev.com/d44d9f8090374684ba2d8aa996d3b47f66037fcc/components/viz/common/frame_sinks/copy_output_util.cc
[modify] https://crrev.com/d44d9f8090374684ba2d8aa996d3b47f66037fcc/components/viz/common/frame_sinks/copy_output_util_unittest.cc

Comment 11 by m...@chromium.org, Oct 19 2017

At this point, two other issues are left before we can call this finished:

1. The GLHelper scaler currently interprets its "vertical flipping flag" in two different ways: a) For computing offset calculations in the source texture; and b) As a request to flip the rows of the image in the GPU during rendering. These are actually two different things, and the code in GLRendererCopier needs (a) but does NOT want (b). So, we'll need to modify the GLHelper scaler to account for this.

2. It turns out that for several reasons, the CopyOutputRequest::set_area() API should be changed so that the client supplies scaled (output not source) coordinates. This will simplify both the client-side code making the scaling CopyOuputRequests and GLRendererCopier+SoftwareRenderer code that needs to execute those requests.

Comment 12 by m...@chromium.org, Oct 19 2017

Blocking: 760351
Project Member

Comment 13 by bugdroid1@chromium.org, Oct 23 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/43ebdaa507565a9abae4ce04b51e8fc7cd53c9f3

commit 43ebdaa507565a9abae4ce04b51e8fc7cd53c9f3
Author: Yuri Wiitala <miu@chromium.org>
Date: Mon Oct 23 22:18:02 2017

GLHelper: Split vertical-flip into the two separate things it means.

This changes splits the GLHelper scaler's "vertical flip" mode into two
separate things: 1) Whether the source texture content is vertically
flipped (for the source rect math), and 2) Whether the scaler's output
should be vertically flipped (to prevent the need to reverse the row
order in a later readback operation).

Bug:  760348 ,  775740 
Change-Id: Ieabfa0a6ec151ba47470677b45518604c7a80a26
Reviewed-on: https://chromium-review.googlesource.com/731766
Reviewed-by: Xiangjun Zhang <xjz@chromium.org>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Commit-Queue: Yuri Wiitala <miu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#510938}
[modify] https://crrev.com/43ebdaa507565a9abae4ce04b51e8fc7cd53c9f3/components/viz/common/gl_helper.cc
[modify] https://crrev.com/43ebdaa507565a9abae4ce04b51e8fc7cd53c9f3/components/viz/common/gl_helper.h
[modify] https://crrev.com/43ebdaa507565a9abae4ce04b51e8fc7cd53c9f3/components/viz/common/gl_helper_benchmark.cc
[modify] https://crrev.com/43ebdaa507565a9abae4ce04b51e8fc7cd53c9f3/components/viz/common/gl_helper_scaling.cc
[modify] https://crrev.com/43ebdaa507565a9abae4ce04b51e8fc7cd53c9f3/components/viz/common/gl_helper_scaling.h
[modify] https://crrev.com/43ebdaa507565a9abae4ce04b51e8fc7cd53c9f3/components/viz/common/gl_helper_unittest.cc
[modify] https://crrev.com/43ebdaa507565a9abae4ce04b51e8fc7cd53c9f3/components/viz/service/display/gl_renderer_copier.cc
[modify] https://crrev.com/43ebdaa507565a9abae4ce04b51e8fc7cd53c9f3/content/browser/media/capture/aura_window_capture_machine.cc
[modify] https://crrev.com/43ebdaa507565a9abae4ce04b51e8fc7cd53c9f3/content/browser/renderer_host/delegated_frame_host.cc

Comment 14 Deleted

Project Member

Comment 15 by bugdroid1@chromium.org, Oct 26 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/21865dc7acb7dfb02e7bd182e438cc620b7084bc

commit 21865dc7acb7dfb02e7bd182e438cc620b7084bc
Author: Yuri Wiitala <miu@chromium.org>
Date: Thu Oct 26 23:37:41 2017

Add CopyOutputRequest::set_result_selection().

Adds a new property to CopyOutputRequests that allows for selecting a
portion of the result from the result (post-scaled) coordinate space.

Motivations:

1. While working on adding I420 format support to copy requests, it
became apparent that resolving certain coordinate calculations required
extra back-and-forth rect transformations with some weird edge cases.
The extra math goes away when GL/SoftwareRenderer can resolve alignment
issues entirely in the scaled coordinate space.

2. Simplifies the usage of copy requests from the perspective of client-
side code when using scaled output to patch changes to whole images (see
 crbug.com/775740  for discussion).

3. Allows for simpler integration with the GLHelperScaling impl. A
future optimization will be able to use the implementation's "minimal
sampling rect" utility to improve performance.

Bug:  760348 ,  760351 
Change-Id: I1df527514a7b11119d2a68b21f05aa74c3bdd765
Reviewed-on: https://chromium-review.googlesource.com/736429
Commit-Queue: Yuri Wiitala <miu@chromium.org>
Reviewed-by: enne <enne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512015}
[modify] https://crrev.com/21865dc7acb7dfb02e7bd182e438cc620b7084bc/components/viz/common/frame_sinks/copy_output_request.h
[modify] https://crrev.com/21865dc7acb7dfb02e7bd182e438cc620b7084bc/components/viz/service/display/copy_output_scaling_pixeltest.cc
[modify] https://crrev.com/21865dc7acb7dfb02e7bd182e438cc620b7084bc/components/viz/service/display/gl_renderer.cc
[modify] https://crrev.com/21865dc7acb7dfb02e7bd182e438cc620b7084bc/components/viz/service/display/gl_renderer_copier.cc
[modify] https://crrev.com/21865dc7acb7dfb02e7bd182e438cc620b7084bc/components/viz/service/display/gl_renderer_copier.h
[modify] https://crrev.com/21865dc7acb7dfb02e7bd182e438cc620b7084bc/components/viz/service/display/gl_renderer_copier_pixeltest.cc
[modify] https://crrev.com/21865dc7acb7dfb02e7bd182e438cc620b7084bc/components/viz/service/display/gl_renderer_copier_unittest.cc
[modify] https://crrev.com/21865dc7acb7dfb02e7bd182e438cc620b7084bc/components/viz/service/display/software_renderer.cc

Comment 16 by m...@chromium.org, Nov 8 2017

Status: Fixed (was: Started)
Components: -Internals>MUS Internals>Services>WindowService
Project Member

Comment 18 by bugdroid1@chromium.org, Feb 27 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6a4443f04150a3024c623205f4f276ec53c6a929

commit 6a4443f04150a3024c623205f4f276ec53c6a929
Author: Yuri Wiitala <miu@chromium.org>
Date: Tue Feb 27 22:29:27 2018

Remove unused elements from RWHV::CopyFromSurface() API and dead code.

Removes both: 1) the SkColorType argument; and 2) the ReadbackResponse
enum from the callback. Neither of these is being used meaningfully
anywhere. In addition, migrated Bind→BindOnce for the callback argument.

As a result, this interface change also exposes many obvious now-dead
code paths, and they are also removed: 1) the "decompress bitmap" code
in the Android TabContentManager (java and c++); 2) the extra post-copy
scaling GLHelper readback infrastructure in content/.../surface_utils.*.

Furthermore, due to touched lines of code, there are misc changes to
placate presubmit warnings (e.g., Bind→BindOnce, and test code that was
using content::RunMessageLoopUntilIdle()).

Finally, an order-of-operations bug in the use of CopyFromSurface() was
discovered in headless/... code and was fixed.

Bug:  759310 ,  582955 ,  415682 ,  760348 ,  807843 ,  787941 
Change-Id: I3398761661b7472ef24f40119278ec969a4929d5
Reviewed-on: https://chromium-review.googlesource.com/929874
Reviewed-by: Marc Treib <treib@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Eric Seckler <eseckler@chromium.org>
Commit-Queue: Yuri Wiitala <miu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#539564}
[modify] https://crrev.com/6a4443f04150a3024c623205f4f276ec53c6a929/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabObserver.java
[modify] https://crrev.com/6a4443f04150a3024c623205f4f276ec53c6a929/chrome/android/java/src/org/chromium/chrome/browser/share/ShareMenuActionHandler.java
[modify] https://crrev.com/6a4443f04150a3024c623205f4f276ec53c6a929/chrome/browser/android/compositor/tab_content_manager.cc
[modify] https://crrev.com/6a4443f04150a3024c623205f4f276ec53c6a929/chrome/browser/devtools/devtools_eye_dropper.cc
[modify] https://crrev.com/6a4443f04150a3024c623205f4f276ec53c6a929/chrome/browser/devtools/devtools_eye_dropper.h
[modify] https://crrev.com/6a4443f04150a3024c623205f4f276ec53c6a929/chrome/browser/extensions/api/tabs/tabs_api.cc
[modify] https://crrev.com/6a4443f04150a3024c623205f4f276ec53c6a929/chrome/browser/plugins/plugin_power_saver_browsertest.cc
[modify] https://crrev.com/6a4443f04150a3024c623205f4f276ec53c6a929/chrome/browser/thumbnails/thumbnail_tab_helper.cc
[modify] https://crrev.com/6a4443f04150a3024c623205f4f276ec53c6a929/chrome/browser/thumbnails/thumbnail_tab_helper.h
[modify] https://crrev.com/6a4443f04150a3024c623205f4f276ec53c6a929/chrome/browser/ui/exclusive_access/flash_fullscreen_interactive_browsertest.cc
[modify] https://crrev.com/6a4443f04150a3024c623205f4f276ec53c6a929/content/browser/browser_plugin/browser_plugin_guest.h
[modify] https://crrev.com/6a4443f04150a3024c623205f4f276ec53c6a929/content/browser/compositor/surface_utils.cc
[modify] https://crrev.com/6a4443f04150a3024c623205f4f276ec53c6a929/content/browser/compositor/surface_utils.h
[modify] https://crrev.com/6a4443f04150a3024c623205f4f276ec53c6a929/content/browser/devtools/devtools_frame_trace_recorder.cc
[modify] https://crrev.com/6a4443f04150a3024c623205f4f276ec53c6a929/content/browser/devtools/protocol/page_handler.cc
[modify] https://crrev.com/6a4443f04150a3024c623205f4f276ec53c6a929/content/browser/devtools/protocol/page_handler.h
[modify] https://crrev.com/6a4443f04150a3024c623205f4f276ec53c6a929/content/browser/frame_host/navigation_controller_impl_unittest.cc
[modify] https://crrev.com/6a4443f04150a3024c623205f4f276ec53c6a929/content/browser/frame_host/navigation_entry_screenshot_manager.cc
[modify] https://crrev.com/6a4443f04150a3024c623205f4f276ec53c6a929/content/browser/frame_host/navigation_entry_screenshot_manager.h
[modify] https://crrev.com/6a4443f04150a3024c623205f4f276ec53c6a929/content/browser/oop_browsertest.cc
[modify] https://crrev.com/6a4443f04150a3024c623205f4f276ec53c6a929/content/browser/renderer_host/delegated_frame_host.cc
[modify] https://crrev.com/6a4443f04150a3024c623205f4f276ec53c6a929/content/browser/renderer_host/delegated_frame_host.h
[modify] https://crrev.com/6a4443f04150a3024c623205f4f276ec53c6a929/content/browser/renderer_host/render_widget_host_browsertest.cc
[modify] https://crrev.com/6a4443f04150a3024c623205f4f276ec53c6a929/content/browser/renderer_host/render_widget_host_impl.cc
[modify] https://crrev.com/6a4443f04150a3024c623205f4f276ec53c6a929/content/browser/renderer_host/render_widget_host_impl.h
[modify] https://crrev.com/6a4443f04150a3024c623205f4f276ec53c6a929/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/6a4443f04150a3024c623205f4f276ec53c6a929/content/browser/renderer_host/render_widget_host_view_android.h
[modify] https://crrev.com/6a4443f04150a3024c623205f4f276ec53c6a929/content/browser/renderer_host/render_widget_host_view_aura.cc
[modify] https://crrev.com/6a4443f04150a3024c623205f4f276ec53c6a929/content/browser/renderer_host/render_widget_host_view_aura.h
[modify] https://crrev.com/6a4443f04150a3024c623205f4f276ec53c6a929/content/browser/renderer_host/render_widget_host_view_base.cc
[modify] https://crrev.com/6a4443f04150a3024c623205f4f276ec53c6a929/content/browser/renderer_host/render_widget_host_view_base.h
[modify] https://crrev.com/6a4443f04150a3024c623205f4f276ec53c6a929/content/browser/renderer_host/render_widget_host_view_browsertest.cc
[modify] https://crrev.com/6a4443f04150a3024c623205f4f276ec53c6a929/content/browser/renderer_host/render_widget_host_view_child_frame.cc
[modify] https://crrev.com/6a4443f04150a3024c623205f4f276ec53c6a929/content/browser/renderer_host/render_widget_host_view_child_frame.h
[modify] https://crrev.com/6a4443f04150a3024c623205f4f276ec53c6a929/content/browser/renderer_host/render_widget_host_view_mac.h
[modify] https://crrev.com/6a4443f04150a3024c623205f4f276ec53c6a929/content/browser/renderer_host/render_widget_host_view_mac.mm
[modify] https://crrev.com/6a4443f04150a3024c623205f4f276ec53c6a929/content/browser/site_per_process_browsertest.cc
[modify] https://crrev.com/6a4443f04150a3024c623205f4f276ec53c6a929/content/browser/web_contents/web_contents_android.cc
[modify] https://crrev.com/6a4443f04150a3024c623205f4f276ec53c6a929/content/browser/web_contents/web_contents_android.h
[modify] https://crrev.com/6a4443f04150a3024c623205f4f276ec53c6a929/content/public/android/BUILD.gn
[modify] https://crrev.com/6a4443f04150a3024c623205f4f276ec53c6a929/content/public/android/java/src/org/chromium/content/browser/webcontents/WebContentsImpl.java
[modify] https://crrev.com/6a4443f04150a3024c623205f4f276ec53c6a929/content/public/android/java/src/org/chromium/content_public/browser/ContentBitmapCallback.java
[modify] https://crrev.com/6a4443f04150a3024c623205f4f276ec53c6a929/content/public/browser/BUILD.gn
[delete] https://crrev.com/e1d1a13cdd8e14067786c5c81e3aa135e83b85b4/content/public/browser/readback_types.h
[modify] https://crrev.com/6a4443f04150a3024c623205f4f276ec53c6a929/content/public/browser/render_widget_host_view.h
[modify] https://crrev.com/6a4443f04150a3024c623205f4f276ec53c6a929/extensions/browser/api/guest_view/web_view/web_view_internal_api.cc
[modify] https://crrev.com/6a4443f04150a3024c623205f4f276ec53c6a929/extensions/browser/api/web_contents_capture_client.cc
[modify] https://crrev.com/6a4443f04150a3024c623205f4f276ec53c6a929/extensions/browser/api/web_contents_capture_client.h
[modify] https://crrev.com/6a4443f04150a3024c623205f4f276ec53c6a929/headless/lib/browser/headless_web_contents_impl.cc
[modify] https://crrev.com/6a4443f04150a3024c623205f4f276ec53c6a929/headless/lib/browser/headless_web_contents_impl.h

Sign in to add a comment