New issue
Advanced search Search tips

Issue 896786 link

Starred by 1 user

Issue metadata

Status: Started
Owner:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocking:
issue 823888



Sign in to add a comment

Find out why there are 1% mismatches between v2 viz hit test and async hit test

Project Member Reported by sunxd@chromium.org, Oct 18

Issue description

The metrics show that there are around 1% hit test mismatches between v2 hit testing and async hit testing. This issue is to track the investigation and solution of this problem.
 
There are two known bugs but I haven't been able to reproduce them with minimal test case:
Issue 1:
1) run chrome with VizHitTestDrawQuad disabled and commandline --use-viz-hit-test-surface-layer, open http://jsfiddle.net/fy1owdjz/
2) click "Settings" on top-right corner, and choose right results layout so that the ad is below the settings dropdown menu.
3) click on other layouts

Instead of changing the layouts, the clicking event is routed to the ad below, and it will open a new google tab.

 Issue 2 :
Open slashdot.org, in very rare cases, there may be ads unclickable. I don't know which property makes it unhit testable, but by debugging I found that the unclickable ads didn't not submit hit test data because it is contained by the union of parent frame's overlapping layers. I don't know why this happens.

Still investigating.
For the second case, the reason is that there is a pointer-events none div completely covering the iframe, we skip iframes covered by hit-testable elements, so this iframe is skipped.
While we can plumb pointer-events for all cc layers, a quick workaround fix is not to skip these iframes.
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 30

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

commit 76e522cf7d35ae27240a24318f42d63a3f7be586
Author: Xianda Sun <sunxd@chromium.org>
Date: Tue Oct 30 17:21:54 2018

[VizHitTest] Mark the result as unreliable if hit test data changed

The finch result of VizHitTest v2 shows that we have 1% of possible
incorrect hit test targets. The data may be noisy because the async
verification may get answer after the page layout changes. This CL
added a new bucket for unreliable result, it counts in that bucket
when the results do not match and another synchronous hit test finds
a different target.

Bug: 896786
Change-Id: I287359251670d4d33297dced42014d03271424d3
Reviewed-on: https://chromium-review.googlesource.com/c/1289163
Reviewed-by: Jesse Doherty <jwd@chromium.org>
Reviewed-by: Robert Flack <flackr@chromium.org>
Reviewed-by: Robert Kroeger <rjkroege@chromium.org>
Commit-Queue: Xianda Sun <sunxd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#603940}
[modify] https://crrev.com/76e522cf7d35ae27240a24318f42d63a3f7be586/content/browser/renderer_host/render_widget_targeter.cc
[modify] https://crrev.com/76e522cf7d35ae27240a24318f42d63a3f7be586/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/76e522cf7d35ae27240a24318f42d63a3f7be586/tools/metrics/histograms/histograms.xml

Project Member

Comment 4 by bugdroid1@chromium.org, Oct 31

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

commit bd4ae48dae4a5c62c9513d0080301d2baffebdfa
Author: Xianda Sun <sunxd@chromium.org>
Date: Wed Oct 31 18:06:11 2018

VizHitTest: Do not skip OOPIF even if it's occluded

We previously employed an optimization that skips OOPIFs occluded by
parent frame's layers. This optimization is problematic as the layer's
bounds do not necessarily represent its hit test region, and it works
incorrectly when the occluding layers have pointer-events property.

This CL removes that optimization.

Bug:  846798 , 896786
Change-Id: Id33c7d658cfd677ab3eb496371a213de2423d349
Reviewed-on: https://chromium-review.googlesource.com/c/1296821
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Reviewed-by: Robert Flack <flackr@chromium.org>
Commit-Queue: Xianda Sun <sunxd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604329}
[modify] https://crrev.com/bd4ae48dae4a5c62c9513d0080301d2baffebdfa/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/bd4ae48dae4a5c62c9513d0080301d2baffebdfa/content/browser/site_per_process_hit_test_browsertest.cc
[add] https://crrev.com/bd4ae48dae4a5c62c9513d0080301d2baffebdfa/content/test/data/frame_tree/page_with_occluded_iframes.html

Project Member

Comment 5 by bugdroid1@chromium.org, Nov 7

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

commit f009bbf97de20dcedb9aba9a7a1b72b3c8e9219f
Author: Xianda Sun <sunxd@chromium.org>
Date: Wed Nov 07 20:22:09 2018

Do async event targeting when mid-level iframes have ask flag

We used to only look at the eventual target iframe's hit test flag to
determine whether we need async event targeting. This is fine with v1
set up because all non-trivial iframes have kHitTestAsk flag. But this
resulted in a bug in v2 in this situation:

When we have nested main_frame->parent_frame->child_frame and only the
mid-level parent_frame has kHitTestAsk flag, we will directly dispatch
event to child_frame. This is not correct because parent_frame may have
overlapping regions with main_frame divs. This patch makes RWHIER always
ask when it encouters a mid-level kHitTestAsk flag.

Bug: 896786
Change-Id: Ib9c81ad44d562b251515d8f4dcde572db8d1fa1d
Reviewed-on: https://chromium-review.googlesource.com/c/1310114
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Reviewed-by: Ria Jiang <riajiang@chromium.org>
Commit-Queue: Xianda Sun <sunxd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606145}
[modify] https://crrev.com/f009bbf97de20dcedb9aba9a7a1b72b3c8e9219f/components/viz/host/hit_test/hit_test_query.cc
[modify] https://crrev.com/f009bbf97de20dcedb9aba9a7a1b72b3c8e9219f/content/browser/site_per_process_hit_test_browsertest.cc
[add] https://crrev.com/f009bbf97de20dcedb9aba9a7a1b72b3c8e9219f/content/test/data/frame_tree/page_with_nested_frames_and_occluding_div.html

Sign in to add a comment