New issue
Advanced search Search tips

Issue 884306 link

Starred by 3 users

Issue metadata

Status: Untriaged
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

"WebViewTest.SpatialNavigationJavascriptAPI" is flaky

Project Member Reported by chromium...@appspot.gserviceaccount.com, Sep 14

Issue description

"WebViewTest.SpatialNavigationJavascriptAPI" is flaky.

This issue was created automatically by the chromium-try-flakes app. Please find the right owner to fix the respective test/step and assign this issue to them. If the step/test is infrastructure-related, please add Infra-Troopers label and change issue status to Untriaged. When done, please remove the issue from Sheriff Bug Queue by removing the Sheriff-Chromium label.

We have detected 4 recent flakes. List of all flakes can be found at https://chromium-try-flakes.appspot.com/all_flake_occurrences?key=ahVzfmNocm9taXVtLXRyeS1mbGFrZXNyNQsSBUZsYWtlIipXZWJWaWV3VGVzdC5TcGF0aWFsTmF2aWdhdGlvbkphdmFzY3JpcHRBUEkM.

Flaky tests should be disabled within 30 minutes unless culprit CL is found and reverted. Please see more details here: https://sites.google.com/a/chromium.org/dev/developers/tree-sheriffs/sheriffing-bug-queues#triaging-auto-filed-flakiness-bugs
 
Detected 10 new flakes for test/step "WebViewTest.SpatialNavigationJavascriptAPI". To see the actual flakes, please visit https://chromium-try-flakes.appspot.com/all_flake_occurrences?key=ahVzfmNocm9taXVtLXRyeS1mbGFrZXNyNQsSBUZsYWtlIipXZWJWaWV3VGVzdC5TcGF0aWFsTmF2aWdhdGlvbkphdmFzY3JpcHRBUEkM. This message was posted automatically by the chromium-try-flakes app.
Cc: shend@chromium.org jaragu...@igalia.com
 Issue 884570  has been merged into this issue.
Labels: -Sheriff-Chromium
Labels: Sheriff-Chromium
Detected 3 new flakes for test/step "WebViewTest.SpatialNavigationJavascriptAPI". To see the actual flakes, please visit https://chromium-try-flakes.appspot.com/all_flake_occurrences?key=ahVzfmNocm9taXVtLXRyeS1mbGFrZXNyNQsSBUZsYWtlIipXZWJWaWV3VGVzdC5TcGF0aWFsTmF2aWdhdGlvbkphdmFzY3JpcHRBUEkM. This message was posted automatically by the chromium-try-flakes app. Since flakiness is ongoing, the issue was moved back into Sheriff Bug Queue (unless already there).
From https://ci.chromium.org/p/chromium/builders/luci.chromium.try/linux-chromeos-rel/94855

[ RUN      ] WebViewTest.SpatialNavigationJavascriptAPI
[3183:3183:0917/040759.285636:WARNING:user_policy_manager_factory_chromeos.cc(208)] No policy loaded for known non-enterprise user
[3183:3183:0917/040759.554852:ERROR:gpu_interface_provider.cc(85)] Not implemented reached in virtual void content::GpuInterfaceProvider::RegisterOzoneGpuInterfaces(service_manager::BinderRegistry *)
GMOCK WARNING:
Uninteresting mock function call - returning directly.
    Function call: RegisterMediaRoutesObserver(0x3544a11a1840)
NOTE: You can safely ignore the above warning unless this call should not happen.  Do not suppress it by blindly adding an EXPECT_CALL() if you don't mean to enforce the call.  See https://github.com/google/googletest/blob/master/googlemock/docs/CookBook.md#knowing-when-to-expect for details.
GMOCK WARNING:
Uninteresting mock function call - taking default action specified at:
../../chrome/browser/apps/platform_apps/app_browsertest_util.cc:68:
    Function call: RegisterMediaSinksObserver(0x3544a11a1868)
          Returns: true
NOTE: You can safely ignore the above warning unless this call should not happen.  Do not suppress it by blindly adding an EXPECT_CALL() if you don't mean to enforce the call.  See https://github.com/google/googletest/blob/master/googlemock/docs/CookBook.md#knowing-when-to-expect for details.
[3183:3183:0917/040800.231802:INFO:CONSOLE(14)] "focused:1", source: data:text/html,%0A    <body>%0A      <a id='1' href='%23'>b</a>%0A      <a id='2' href='%23'>c</a>%0A      <a id='3' href='%23'>d</a>%0A      <a id='4' href='%23'>e</a>%0A      <a id='5' href='%23'>f</a>%0A      <a id='6' href='%23'>g</a>%0A      <a id='7' href='%23'>h</a>%0A      <a id='8' href='%23'>i</a>%0A      <script>%0A        document.querySelectorAll('a').forEach(function(each) {%0A          each.onfocus = function(ev) {%0A            console.log('focused:'+ev.target.id);%0A          };%0A        });%0A        </script>%0A    </body> (14)
[3183:3183:0917/040800.235924:INFO:CONSOLE(14)] "focused:2", source: data:text/html,%0A    <body>%0A      <a id='1' href='%23'>b</a>%0A      <a id='2' href='%23'>c</a>%0A      <a id='3' href='%23'>d</a>%0A      <a id='4' href='%23'>e</a>%0A      <a id='5' href='%23'>f</a>%0A      <a id='6' href='%23'>g</a>%0A      <a id='7' href='%23'>h</a>%0A      <a id='8' href='%23'>i</a>%0A      <script>%0A        document.querySelectorAll('a').forEach(function(each) {%0A          each.onfocus = function(ev) {%0A            console.log('focused:'+ev.target.id);%0A          };%0A        });%0A        </script>%0A    </body> (14)
[3183:3183:0917/040800.246131:INFO:CONSOLE(14)] "focused:3", source: data:text/html,%0A    <body>%0A      <a id='1' href='%23'>b</a>%0A      <a id='2' href='%23'>c</a>%0A      <a id='3' href='%23'>d</a>%0A      <a id='4' href='%23'>e</a>%0A      <a id='5' href='%23'>f</a>%0A      <a id='6' href='%23'>g</a>%0A      <a id='7' href='%23'>h</a>%0A      <a id='8' href='%23'>i</a>%0A      <script>%0A        document.querySelectorAll('a').forEach(function(each) {%0A          each.onfocus = function(ev) {%0A            console.log('focused:'+ev.target.id);%0A          };%0A        });%0A        </script>%0A    </body> (14)
[3183:3183:0917/040800.246655:INFO:CONSOLE(14)] "focused:2", source: data:text/html,%0A    <body>%0A      <a id='1' href='%23'>b</a>%0A      <a id='2' href='%23'>c</a>%0A      <a id='3' href='%23'>d</a>%0A      <a id='4' href='%23'>e</a>%0A      <a id='5' href='%23'>f</a>%0A      <a id='6' href='%23'>g</a>%0A      <a id='7' href='%23'>h</a>%0A      <a id='8' href='%23'>i</a>%0A      <script>%0A        document.querySelectorAll('a').forEach(function(each) {%0A          each.onfocus = function(ev) {%0A            console.log('focused:'+ev.target.id);%0A          };%0A        });%0A        </script>%0A    </body> (14)
../../chrome/browser/apps/guest_view/web_view_browsertest.cc:1139: Failure
Value of: next_step_listener.WaitUntilSatisfied()
  Actual: false
Expected: true
Stack trace:
#0 0x000002656bcc testing::internal::UnitTestImpl::CurrentOsStackTraceExceptTop()
#1 0x0000026565a9 testing::internal::AssertHelper::operator=()
#2 0x000000ade5c5 WebViewTest_SpatialNavigationJavascriptAPI_Test::RunTestOnMainThread()
#3 0x0000055387b2 content::BrowserTestBase::ProxyRunTestOnMainThreadLoop()
#4 0x000005006806 ChromeBrowserMainParts::PreMainMessageLoopRunImpl()
#5 0x0000050055fa ChromeBrowserMainParts::PreMainMessageLoopRun()
#6 0x000002169f78 chromeos::ChromeBrowserMainPartsChromeos::PreMainMessageLoopRun()
#7 0x0000034c05a1 content::BrowserMainLoop::PreMainMessageLoopRun()
#8 0x0000039653c5 content::StartupTaskRunner::RunAllTasksNow()
#9 0x0000034bece7 content::BrowserMainLoop::CreateStartupTasks()
#10 0x0000034c2e03 content::BrowserMainRunnerImpl::Initialize()
#11 0x0000034bc852 content::BrowserMain()
#12 0x000004de7d1f content::ContentMainRunnerImpl::Run()
#13 0x0000071cbfe5 service_manager::Main()
#14 0x000004de61b4 content::ContentMain()
#15 0x000005538379 content::BrowserTestBase::SetUp()
#16 0x000004fa74a5 InProcessBrowserTest::SetUp()
[3257:3257:0917/040800.397238:WARNING:ipc_message_attachment_set.cc(49)] MessageAttachmentSet destroyed with unconsumed attachments: 0/1
GMOCK WARNING:
Uninteresting mock function call - returning directly.
    Function call: UnregisterMediaSinksObserver(0x3544a11a1868)
NOTE: You can safely ignore the above warning unless this call should not happen.  Do not suppress it by blindly adding an EXPECT_CALL() if you don't mean to enforce the call.  See https://github.com/google/googletest/blob/master/googlemock/docs/CookBook.md#knowing-when-to-expect for details.
GMOCK WARNING:
Uninteresting mock function call - returning directly.
    Function call: UnregisterMediaRoutesObserver(0x3544a11a1840)
NOTE: You can safely ignore the above warning unless this call should not happen.  Do not suppress it by blindly adding an EXPECT_CALL() if you don't mean to enforce the call.  See https://github.com/google/googletest/blob/master/googlemock/docs/CookBook.md#knowing-when-to-expect for details.
[3183:3216:0917/040800.475680:WARNING:discardable_shared_memory_manager.cc(438)] Some MojoDiscardableSharedMemoryManagerImpls are still alive. They will be leaked.
[3183:3599:0917/040800.478707:WARNING:internal_linux.cc(64)] Failed to read /proc/3257/stat
[3183:3183:0917/040800.478984:WARNING:pref_notifier_impl.cc(23)] Pref observer found at shutdown.
[3183:3183:0917/040800.479000:WARNING:pref_notifier_impl.cc(23)] Pref observer found at shutdown.
[  FAILED  ] WebViewTest.SpatialNavigationJavascriptAPI, where TypeParam =  and GetParam() =  (1564 ms)

Cc: hu...@vewd.com
Thanks for the heads-up, I'm taking this bug.
It might not be a bad idea to disable the test first if the fixing takes too long.
Based on https://findit-for-me.appspot.com/ranked-flakes, this test has impacted 20+ CLs in CQ.
Sent CL to disable the test while I look carefully into this problem. https://chromium-review.googlesource.com/c/chromium/src/+/1230019
Project Member

Comment 10 by bugdroid1@chromium.org, Sep 18

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

commit ba5d0d3d5fedb96af985c70d7f0da647fbe9d36b
Author: Jacobo Aragunde Pérez <jaragunde@igalia.com>
Date: Tue Sep 18 15:09:23 2018

Disable WebViewTest.SpatialNavigationJavascriptAPI.

Bug: 884306
Change-Id: Ibeff0b4c01007d9cfa0fbed3115a4acd5c3c8586
Reviewed-on: https://chromium-review.googlesource.com/1230019
Reviewed-by: Kevin McNee <mcnee@chromium.org>
Commit-Queue: Kevin McNee <mcnee@chromium.org>
Cr-Commit-Position: refs/heads/master@{#592040}
[modify] https://crrev.com/ba5d0d3d5fedb96af985c70d7f0da647fbe9d36b/chrome/browser/apps/guest_view/web_view_browsertest.cc

Labels: -Sheriff-Chromium
Taking off sheriff queue since affected test is now disabled.
Labels: Sheriff-Chromium
Detected 15 new flakes for test/step "WebViewTest.SpatialNavigationJavascriptAPI". To see the actual flakes, please visit https://chromium-try-flakes.appspot.com/all_flake_occurrences?key=ahVzfmNocm9taXVtLXRyeS1mbGFrZXNyNQsSBUZsYWtlIipXZWJWaWV3VGVzdC5TcGF0aWFsTmF2aWdhdGlvbkphdmFzY3JpcHRBUEkM. This message was posted automatically by the chromium-try-flakes app. Since flakiness is ongoing, the issue was moved back into Sheriff Bug Queue (unless already there).
Labels: -Sheriff-Chromium
All detected flakes predate #10
I was unable to reproduce this until I increased the load of my system while running the tests; then, the test started to fail one out of 100 times, approx. I think that the bigger the system load, more chances to fail.

The cause of the failure is a timeout. My hypothesis is that the C++ and JS sides of the test sometimes are not in sync, and one side moves faster than expected by the other side. 
Found another reason for flakiness: under heavy system load, it could happen that `isSpatialNavigationEnabled` reports the new state before `setSpatialNavigationEnabled` actually sets it. The reason is that the setter code must reach the render process via IPC, while the getter returns a cached value: see https://chromium-review.googlesource.com/c/chromium/src/+/1133011/17/extensions/browser/guest_view/web_view/web_view_guest.cc#1187
Project Member

Comment 16 by bugdroid1@chromium.org, Nov 19

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

commit cfe37a2b26a025c52e6cd399d7a58e9860ef57a5
Author: Jacobo Aragunde Pérez <jaragunde@igalia.com>
Date: Mon Nov 19 11:36:15 2018

Fix flaky WebViewTest.SpatialNavigationJavascriptAPI.

Two reasons for flakiness were identified:

* Occasional lack of sync between the C++ and JS sides of the test.
Sometimes, the JS side emitted signals before the C++ code was ready to
listen to them, or the C++ side emitted keystrokes before listeners were
ready in JS code. Fixed with slight changes in the order of the test
code.

* When checking the spatial navigation status immediately after changing
it, the wrong state could be reported. When the system is under heavy
load, it could happen that the changed state is reported before the
change has been made effective in the render process. Fixed sending a
message to the guest render process with postMessage, which uses the
same IPC channel; when the message is answered we can be sure that the
SetSpatialNavigationEnabled operation is complete.

Bug: 884306
Change-Id: Iae227098cd36980357d8b747ddcb527df2317fa1
Reviewed-on: https://chromium-review.googlesource.com/c/1327204
Reviewed-by: Lucas Gadani <lfg@chromium.org>
Commit-Queue: Jacobo Aragunde Pérez <jaragunde@igalia.com>
Cr-Commit-Position: refs/heads/master@{#609241}
[modify] https://crrev.com/cfe37a2b26a025c52e6cd399d7a58e9860ef57a5/chrome/browser/apps/guest_view/web_view_browsertest.cc
[modify] https://crrev.com/cfe37a2b26a025c52e6cd399d7a58e9860ef57a5/chrome/test/data/extensions/platform_apps/web_view/spatial_navigation_state_api/main.js

Sign in to add a comment