New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 803548 link

Starred by 2 users
Status: Fixed
Owner:
riajiang@chromium.org
Closed: Aug 14
Cc:
rjkroege@chromium.org
█ gklassen@chromium.org
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows
Pri: 2
Type: Bug

Blocked on:
issue 811928
issue 840893
Blocking:
issue 787097
issue 818205



Sign in to add a comment

viz: Make OOPIF hit testing work with OOP-D

Project Member Reported by kylec...@chromium.org, Jan 18 2018 
When running with OOP-D you can't click on OOPIFs. This is due to early outs before any of the SurfaceHittest code. Running the following command line and looking at a page with ads demonstrates this.

$ ./chrome --enable-features=VizDisplayCompositor,top-document-isolation

It looks like OOP-D and site-per-process is pretty unstable but top-document-isolation works for testing this out. If we could get the slow path hit testing working, where the browser always asks the renderer, that would at least make OOPIFs usable.

Comment 1 by kylec...@chromium.org, Jan 22 2018

Labels: M-66
Summary: viz: Make OOPIF hit testing work with OOP-D (was: viz: Make OOPIF hit testing working with OOP-D)
Project Member

Comment 2 by bugdroid1@chromium.org, Jan 22 2018 

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

commit d69e3bace383645baa51bc3f91c39657de6bcfe2
Author: Ria Jiang <riajiang@chromium.org>
Date: Mon Jan 22 23:38:38 2018

Handle slow path case kHitTestAsk in HitTestQuery.

1. If the target hit-test region we find in HitTestQuery has kHitTestAsk
flag set, we need to fall back to the slow path - ask renderer to do the
targeting. Return the viz::Target we find with kHitTestAsk in that case.

2. In RenderWidgetHostInputEventRouter::FindViewAtLocation, set
RenderWidgetTargetResult::should_query_view to be true if kHitTestAsk is
set in the target we received.

Bug:  803548 
Test: viz_unittests
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: I7df66f2e09a15308ee02ee4918ab1f43077b1fbd
Reviewed-on: https://chromium-review.googlesource.com/876997
Reviewed-by: Robert Kroeger <rjkroege@chromium.org>
Commit-Queue: Ria Jiang <riajiang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531060}
[modify] https://crrev.com/d69e3bace383645baa51bc3f91c39657de6bcfe2/components/viz/host/hit_test/hit_test_query.cc
[modify] https://crrev.com/d69e3bace383645baa51bc3f91c39657de6bcfe2/components/viz/host/hit_test/hit_test_query_unittest.cc
[modify] https://crrev.com/d69e3bace383645baa51bc3f91c39657de6bcfe2/content/browser/renderer_host/render_widget_host_input_event_router.cc

Comment 3 by riajiang@chromium.org, Feb 13 2018

Blockedon: 811928

Comment 4 by riajiang@chromium.org, Mar 2 2018

Blocking: 818205
Project Member

Comment 5 by bugdroid1@chromium.org, Apr 10 2018 

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

commit e52ceae2371bec096d8a43a29adc3d151981dd95
Author: Ria Jiang <riajiang@chromium.org>
Date: Tue Apr 10 17:39:13 2018

Browser submits hit-test data in OOP-D.

This CL sets up hit-test data provider in VizProcessTransportFactory for
browser in the OOP-D (Viz DisplayCompositor) case. Browser shouldn't set
ask flag for the renderers it embeds.

Bug:  803548 
Test: viz_unittests
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: I5c00d4ef00ce40a4478f18e28585e07f450ecb15
Reviewed-on: https://chromium-review.googlesource.com/916828
Reviewed-by: kylechar <kylechar@chromium.org>
Commit-Queue: Ria Jiang <riajiang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549576}
[modify] https://crrev.com/e52ceae2371bec096d8a43a29adc3d151981dd95/components/viz/client/hit_test_data_provider_draw_quad_unittest.cc
[modify] https://crrev.com/e52ceae2371bec096d8a43a29adc3d151981dd95/content/browser/compositor/viz_process_transport_factory.cc
Project Member

Comment 6 by bugdroid1@chromium.org, May 7 2018 

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

commit 2c8b3a830be06d92784f75e831e476cd7086b6d9
Author: Ria Jiang <riajiang@chromium.org>
Date: Mon May 07 19:55:36 2018

Add mojom, struct_traits and typemap for AggregatedHitTestRegion.

Used in
https://chromium-review.googlesource.com/c/chromium/src/+/1044887.

Bug:  803548 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: If6adbd59ee035b392ea87f3df286bfea569d6950
Reviewed-on: https://chromium-review.googlesource.com/1044783
Reviewed-by: kylechar <kylechar@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Commit-Queue: Ria Jiang <riajiang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556535}
[add] https://crrev.com/2c8b3a830be06d92784f75e831e476cd7086b6d9/components/viz/common/hit_test/DEPS
[modify] https://crrev.com/2c8b3a830be06d92784f75e831e476cd7086b6d9/components/viz/common/hit_test/aggregated_hit_test_region.h
[modify] https://crrev.com/2c8b3a830be06d92784f75e831e476cd7086b6d9/mojo/public/tools/bindings/chromium_bindings_configuration.gni
[modify] https://crrev.com/2c8b3a830be06d92784f75e831e476cd7086b6d9/services/BUILD.gn
[add] https://crrev.com/2c8b3a830be06d92784f75e831e476cd7086b6d9/services/viz/public/cpp/hit_test/BUILD.gn
[add] https://crrev.com/2c8b3a830be06d92784f75e831e476cd7086b6d9/services/viz/public/cpp/hit_test/DEPS
[add] https://crrev.com/2c8b3a830be06d92784f75e831e476cd7086b6d9/services/viz/public/cpp/hit_test/OWNERS
[add] https://crrev.com/2c8b3a830be06d92784f75e831e476cd7086b6d9/services/viz/public/cpp/hit_test/aggregated_hit_test_region.typemap
[add] https://crrev.com/2c8b3a830be06d92784f75e831e476cd7086b6d9/services/viz/public/cpp/hit_test/aggregated_hit_test_region_struct_traits.h
[add] https://crrev.com/2c8b3a830be06d92784f75e831e476cd7086b6d9/services/viz/public/cpp/hit_test/struct_traits_unittest.cc
[add] https://crrev.com/2c8b3a830be06d92784f75e831e476cd7086b6d9/services/viz/public/cpp/hit_test/typemaps.gni
[modify] https://crrev.com/2c8b3a830be06d92784f75e831e476cd7086b6d9/services/viz/public/interfaces/BUILD.gn
[add] https://crrev.com/2c8b3a830be06d92784f75e831e476cd7086b6d9/services/viz/public/interfaces/hit_test/aggregated_hit_test_region.mojom

Comment 7 by riajiang@chromium.org, May 8 2018

Blockedon: 840893
Project Member

Comment 8 by bugdroid1@chromium.org, May 9 2018 

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

commit 5907148ddd47428c8d13335ed3a8dd00ac9d2f3d
Author: Ria Jiang <riajiang@chromium.org>
Date: Wed May 09 21:44:36 2018

Send hit-test data through IPC from HTA to HTQ for OOP-D.

HitTestAggregator and HitTestQuery were using shared memory for
hit-test data. For OOP-D, HitTestAggregator is in Viz and
HitTestQuery is in browser; so HTA can write to the memory HTQ
is reading from before it receives the IPC to switch. We can
modify the current shared memory scheme to fix that problem but
it's more risky. This CL changes it to send hit-test data through
IPC from HTA to HTQ for OOP-D, which is less risky and a good
starting point.

mojom related changes for AggregatedHitTestRegion is in
https://chromium-review.googlesource.com/c/chromium/src/+/1044783.

Bug:  803548 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: Iae146594da608ffc1392834ffe663c8ea9b00729
Reviewed-on: https://chromium-review.googlesource.com/1044887
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Robert Kroeger <rjkroege@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Commit-Queue: Ria Jiang <riajiang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557329}
[modify] https://crrev.com/5907148ddd47428c8d13335ed3a8dd00ac9d2f3d/components/viz/host/hit_test/hit_test_query.cc
[modify] https://crrev.com/5907148ddd47428c8d13335ed3a8dd00ac9d2f3d/components/viz/host/hit_test/hit_test_query.h
[modify] https://crrev.com/5907148ddd47428c8d13335ed3a8dd00ac9d2f3d/components/viz/host/hit_test/hit_test_query_fuzzer.cc
[modify] https://crrev.com/5907148ddd47428c8d13335ed3a8dd00ac9d2f3d/components/viz/host/hit_test/hit_test_query_unittest.cc
[modify] https://crrev.com/5907148ddd47428c8d13335ed3a8dd00ac9d2f3d/components/viz/host/host_frame_sink_manager.cc
[modify] https://crrev.com/5907148ddd47428c8d13335ed3a8dd00ac9d2f3d/components/viz/host/host_frame_sink_manager.h
[modify] https://crrev.com/5907148ddd47428c8d13335ed3a8dd00ac9d2f3d/components/viz/host/host_frame_sink_manager_unittest.cc
[modify] https://crrev.com/5907148ddd47428c8d13335ed3a8dd00ac9d2f3d/components/viz/service/frame_sinks/compositor_frame_sink_support_unittest.cc
[modify] https://crrev.com/5907148ddd47428c8d13335ed3a8dd00ac9d2f3d/components/viz/service/frame_sinks/frame_sink_manager_impl.cc
[modify] https://crrev.com/5907148ddd47428c8d13335ed3a8dd00ac9d2f3d/components/viz/service/frame_sinks/frame_sink_manager_impl.h
[modify] https://crrev.com/5907148ddd47428c8d13335ed3a8dd00ac9d2f3d/components/viz/service/hit_test/hit_test_aggregator.cc
[modify] https://crrev.com/5907148ddd47428c8d13335ed3a8dd00ac9d2f3d/components/viz/service/hit_test/hit_test_aggregator.h
[modify] https://crrev.com/5907148ddd47428c8d13335ed3a8dd00ac9d2f3d/components/viz/service/hit_test/hit_test_aggregator_delegate.h
[modify] https://crrev.com/5907148ddd47428c8d13335ed3a8dd00ac9d2f3d/components/viz/service/hit_test/hit_test_aggregator_unittest.cc
[modify] https://crrev.com/5907148ddd47428c8d13335ed3a8dd00ac9d2f3d/components/viz/service/hit_test/hit_test_manager_fuzzer.cc
[modify] https://crrev.com/5907148ddd47428c8d13335ed3a8dd00ac9d2f3d/components/viz/test/test_frame_sink_manager_client.h
[modify] https://crrev.com/5907148ddd47428c8d13335ed3a8dd00ac9d2f3d/services/ui/ws/event_processor_unittest.cc
[modify] https://crrev.com/5907148ddd47428c8d13335ed3a8dd00ac9d2f3d/services/viz/privileged/interfaces/compositing/frame_sink_manager.mojom

Comment 9 by kylec...@chromium.org, Aug 14

Status: Fixed (was: Assigned)
►

Sign in to add a comment