New issue
Advanced search Search tips

Issue 792477 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 30
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

SitePerProcessBrowserTest.CrossProcessInertSubframe is flaky

Project Member Reported by alexilin@chromium.org, Dec 6 2017

Issue description

Bot failure: https://ci.chromium.org/buildbot/chromium.android/Marshmallow%20Phone%20Tester%20(rel)/1065

See more flakes on a dashboard:
https://test-results.appspot.com/dashboards/flakiness_dashboard.html#testType=content_browsertests&tests=SitePerProcessBrowserTest.CrossProcessInertSubframe

Test output:
Note: Google Test filter = SitePerProcessBrowserTest.CrossSiteIframeDisplayNone
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from SitePerProcessBrowserTest, where TypeParam =
[ RUN      ] SitePerProcessBrowserTest.CrossProcessInertSubframe
[WARNING:dns_config_service_posix.cc(341)] Failed to read DnsConfig.
[ERROR:devtools_http_handler.cc(249)] Cannot start http server for devtools. Stop devtools.
[ERROR:instance.cc(49)] Unable to locate service manifest for metrics
[ERROR:service_manager.cc(890)] Failed to resolve service name: metrics
[ERROR:instance.cc(49)] Unable to locate service manifest for metrics
[ERROR:service_manager.cc(890)] Failed to resolve service name: metrics
[ERROR:shell_android.cc(72)] Not implemented reached in void content::Shell::PlatformSetTitle(const base::string16 &): Cross-site iframe factory
[WARNING:render_frame_host_impl.cc(2671)] OnDidStopLoading was called twice.
[WARNING:render_frame_host_impl.cc(2671)] OnDidStopLoading was called twice.
referenceTable head length=54 1
../../content/browser/site_per_process_browsertest.cc:11479: Failure
      Expected: ""
To be equal to: focused_element
      Which is: "text1"
[  FAILED  ] SitePerProcessBrowserTest.CrossProcessInertSubframe, where TypeParam =  and GetParam() =  (1199 ms)
[----------] 1 test from SitePerProcessBrowserTest (1200 ms total)
[----------] Global test environment tear-down
[==========] 1 test from 1 test case ran. (1200 ms total)
[  PASSED  ] 0 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] SitePerProcessBrowserTest.CrossProcessInertSubframe, where TypeParam =  and GetParam() =
 1 FAILED TEST
 
Cc: -kenrb@chromium.org nasko@google.com alex...@chromium.org arthurso...@chromium.org
Labels: sheriff-android OS-Android
Owner: kenrb@chromium.org
Update: Still a bit flaky on Android.

I took a look at the code, I saw this:
~~~
  // Attempt to change focus in the inert subframe. This should fail.
  // The setTimeout ensures that the inert bit can propagate before the
  // test JS code runs.
  EXPECT_TRUE(ExecuteScriptAndExtractString(
      iframe_node,
      "window.setTimeout(() => {text2.focus();"
      "domAutomationController.send(document.activeElement.id);}, 0)",
      &focused_element));
  EXPECT_EQ("", focused_element);
~~~

To me, it looks like the "setTimout" was not enough ;-)

+owner: Ken. You made the test. Do you have any other ideas how avoid using setTimeout here?
Sheriff ping
I'll have a look. The setTimeout isn't used to wait for any particular period of time, it is just supposed to defer running the JavaScript until other tasks currently in the queue have been processed. That's why the timeout value is 0. Blink tests do that often.
It looks like the flake is on the post-navigation inertness check, not the one mentioned in comment 1?

Possibly it is because the inert bit isn't propagating before the script is sent, so there is a renderer-side race. I'll put up a CL to try to address that.
Project Member

Comment 5 by bugdroid1@chromium.org, Nov 21

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

commit 5005b6138dfa5c66072769d32f9edf598224378d
Author: Ken Buchanan <kenrb@chromium.org>
Date: Wed Nov 21 23:39:22 2018

Introduce extra wait to CrossProcessInertSubframe test

This is a speculative fix to flakiness on
SitePerProcessBrowserTest.CrossProcessInertSubframe, yielding the UI
thread after navigation but before sending the script to test for
inertness. This should ensure the inertness bit propagates to the new
frame before the script is sent for execution.

Bug:  792477 
Change-Id: Iee370a863003045399730ba4a49831051d20316c
Reviewed-on: https://chromium-review.googlesource.com/c/1347617
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Commit-Queue: Ken Buchanan <kenrb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610264}
[modify] https://crrev.com/5005b6138dfa5c66072769d32f9edf598224378d/content/browser/site_per_process_browsertest.cc

Status: Fixed (was: Untriaged)
Please re-open if any flakiness persists.

Sign in to add a comment