user-activation/message-event-activation-api-iframe-cross-origin.sub.tentative.html failing for Viz hit-testing |
|||||||
Issue descriptionViz hit-testing needs to wait for Viz to receive hit-test data (along with CompositorFrame) to be able to hit-test correctly. user-activation/message-event-activation-api-iframe-cross-origin.sub.tentative.html is failing with timeout error for Viz hit-testing, most likely because the click that's supposed to target the iframe didn't get there. I tried adding requestAnimationFrame before posting "child-three-loaded" (which would then initialize the click) in child_three.html so that it would wait for BeginFrame/ hit-test data arriving in Viz, but that didn't seem to help.
,
Aug 27
I see that the test is already marked as "slow". Dave, any clue why?
,
Aug 27
Not sure why it is marked slow.. But issue 874695 marks a lot of tests are slow because of oopif.
,
Aug 27
riajiang@, few questions: - What's the correct way to guarantee that the child frame has the correct hit-test data when the |requestAnimationFrame| solution you tried failed? - Is this delayed hit-test data problem specific to wpt? I seem to remember seeing (non-wpt) LayoutTests that clicks subframes for user activation, not sure if they are affected by viz hit-test data or not.
,
Aug 27
With Viz hit-testing, we have to wait for both main frame and child frame to submit hit-test data to Viz process in order to have the correct hit-test data for targeting - but I'm not sure how to guarantee that in wpt, requestAnimationFrame should work in theory because by the next BeginFrame the previous hit-test data should've arrived in Viz. In non-wpt tests, we have an observer that checks if the hit-test data has arrived in Viz or not.
,
Aug 27
As discussed offline, it would be useful to understand the reason for the test timing out: is it because the hit-test data is not being submitted by the main-page or the oopif, or is it because the hit-test data are somehow invalid (e.g. perhaps some surface-id mismatch between the embedder vs. the oopif, etc.)?
,
Aug 27
Not sure if this can explain the failure: the subframe script is executed before the load event is fired to subframe's <body>, which comes before main-frame-body's load is fired. I don't have the bandwidth to dig into this soon (as a P1). riajiang@, could you please look into this?
,
Aug 27
Sure I can look into this. But I'm not really familiar with webkit_layout_tests to fully understand the test yet - by "subframe script" do you mean the script in child_three.html? so the "postMessage("child-three-loaded")" in that script is executed before main frame is loaded?
I also can't repro this locally with
python testing/xvfb.py third_party/blink/tools/run_web_tests.py --additional-driver-flag=--site-per-process external/wpt/html/user-activation/message-event-activation-api-iframe-cross-origin.sub.tentative.html -t oxygenln
- am I missing any flags?
,
Aug 27
Can you assert that you have received your hit testing data before you process any synthetic actions? Perhaps you are getting the click event before you have frame data. If that is the case... I think the best way to solve this is to wait for the syntehtic user gesture until you have hit testing data available.
,
Aug 27
ie;... https://cs.chromium.org/chromium/src/content/browser/renderer_host/input/synthetic_gesture_controller.cc?sq=package:chromium&g=0&l=43 perhaps shouldn't be started until fire until you have received at least one frame of hit testing data?
,
Aug 27
Dave's suggestion above seems like a better solution. If it doesn't help, you can try delaying the test's click until *both* frames have received "load" events. (To clarify my #c7 above: the observed sequence is for loading a similar page on Chrome. When I ran the test locally, it passed with the following sequence: main-body-load, child-script, child-body-load.)
,
Aug 31
So I found that when running webkit_layout_tests, we don't set up the AsyncLayerTreeFrameSink https://cs.chromium.org/chromium/src/cc/mojo_embedder/async_layer_tree_frame_sink.cc?g=0&l=118 (that we use in prod and browser_tests) for renderers https://cs.chromium.org/chromium/src/content/renderer/render_thread_impl.cc?l=2125 - we early return here https://cs.chromium.org/chromium/src/content/renderer/render_thread_impl.cc?l=2085 because layout tests use the display compositor pixel dumps (not sure what it means). Instead it creates a viz::TestLayerTreeFrameSink, but even then, TestLayerTreeFrameSink::SubmitCompositorFrame was never called so we don't get any compositor-frame/ hit-test data for renderers in Viz. Is this expected for webkit_layout_tests? Viz hit-testing tests need to rely on getting the correct hit-test data from renderers to pass
,
Sep 5
Philip, are you aware of this difference between running layout tests and the prod Chrome? Is there any plan to fix this?
,
Sep 5
re comment# 12: sounds like the test should always fail with viz hit-testing turned on? Do you know why it does not repro locally? Is SubmitCompositorFrame() just never called at all? For any renderer, or just for the OOPIFs? For the short term, would it make sense to run this test as part of one of the virtual test-suites where the compositor is used (i.e. with --enable-threaded-compositing flag)?
,
Sep 5
Yea SubmitCompositorFrame() was never called for any renderers, so it should always fail with viz hit-testing turned on if we expect renderers to handle an event. It actually repros locally with an observer on the renderer hit-test data in Viz, so we are not receiving the correct hit-test data; but I don't understand why it passes locally w/o waiting for hit-test data to be ready yet...
,
Sep 5
Does it work if you force slow path always? Could it be going into the slow path locally? If so would it work to force the slow path always for layout tests?
,
Sep 17
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fda3ed7f1e8a2bf02e29b9c4c88e6a6fbae7b8ef commit fda3ed7f1e8a2bf02e29b9c4c88e6a6fbae7b8ef Author: Ria Jiang <riajiang@chromium.org> Date: Mon Sep 17 15:36:50 2018 Enable VizHitTestDrawQuad feature by default on TOT. VizHitTestDrawQuad has been in finch on Canary, Dev and Beta 50% and Stable 1% for several months. Enabling it by default on TOT before increasing the percentage on Stable channel. This CL does ask path for clients that have not submitted its own hit-test data yet for webkit_layout_tests. Also fixes WebViewAPITest.TestContextMenu since we expect root_view in RenderWidgetHostInputEventRouter::RouteMouseEvent. Bug: 804888, 877961 Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel Change-Id: I1e507afafb3cf28b424094a57d5949207929fad2 Reviewed-on: https://chromium-review.googlesource.com/1181185 Commit-Queue: Ria Jiang <riajiang@chromium.org> Reviewed-by: Nasko Oskov <nasko@chromium.org> Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org> Reviewed-by: Navid Zolghadr <nzolghadr@chromium.org> Reviewed-by: James MacLean <wjmaclean@chromium.org> Cr-Commit-Position: refs/heads/master@{#591686} [modify] https://crrev.com/fda3ed7f1e8a2bf02e29b9c4c88e6a6fbae7b8ef/components/viz/common/features.cc [modify] https://crrev.com/fda3ed7f1e8a2bf02e29b9c4c88e6a6fbae7b8ef/components/viz/common/hit_test/hit_test_region_list.h [modify] https://crrev.com/fda3ed7f1e8a2bf02e29b9c4c88e6a6fbae7b8ef/components/viz/host/hit_test/hit_test_query.cc [modify] https://crrev.com/fda3ed7f1e8a2bf02e29b9c4c88e6a6fbae7b8ef/components/viz/host/hit_test/hit_test_query.h [modify] https://crrev.com/fda3ed7f1e8a2bf02e29b9c4c88e6a6fbae7b8ef/components/viz/service/hit_test/hit_test_aggregator.cc [modify] https://crrev.com/fda3ed7f1e8a2bf02e29b9c4c88e6a6fbae7b8ef/components/viz/service/hit_test/hit_test_aggregator_unittest.cc [modify] https://crrev.com/fda3ed7f1e8a2bf02e29b9c4c88e6a6fbae7b8ef/content/public/test/hit_test_region_observer.cc [modify] https://crrev.com/fda3ed7f1e8a2bf02e29b9c4c88e6a6fbae7b8ef/extensions/browser/guest_view/web_view/web_view_apitest.cc
,
Nov 29
,
Nov 29
|
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by nzolghadr@chromium.org
, Aug 27Owner: mustaq@chromium.org