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

Issue 899384 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 899073



Sign in to add a comment

WakeLockTest.OutOfProcessFrame is flaky

Project Member Reported by danakj@chromium.org, Oct 26

Issue description

This test races with FrameHostMsg_SwapOut_ACK. Delaying the ACK causes the test to fail.

[ RUN      ] WakeLockTest.OutOfProcessFrame

DevTools listening on ws://127.0.0.1:41231/devtools/browser/51ea839d-caa8-42ea-a7dd-e9b35dd7292d
../../content/browser/wake_lock/wake_lock_browsertest.cc:352: Failure
Value of: HasWakeLock()
  Actual: true
Expected: false
Stack trace:
#0 0x000000cbc16c testing::internal::UnitTestImpl::CurrentOsStackTraceExceptTop()
#1 0x000000cbbb39 testing::internal::AssertHelper::operator=()
#2 0x000000b1daf2 content::WakeLockTest_OutOfProcessFrame_Test::RunTestOnMainThread()
#3 0x000000df27c2 content::BrowserTestBase::ProxyRunTestOnMainThreadLoop()
#4 0x000000e908d6 content::ShellBrowserMainParts::PreMainMessageLoopRun()
#5 0x7fad5a88c56a content::BrowserMainLoop::PreMainMessageLoopRun()
#6 0x7fad5add3035 content::StartupTaskRunner::RunAllTasksNow()
#7 0x7fad5a88b0ae content::BrowserMainLoop::CreateStartupTasks()
#8 0x7fad5a88f1ed content::BrowserMainRunnerImpl::Initialize()
#9 0x000000e9031a ShellBrowserMain()
#10 0x000000e48310 content::ShellMainDelegate::RunProcess()
#11 0x7fad5b3d0653 content::ContentMainRunnerImpl::RunServiceManager()
#12 0x7fad5b3d0519 content::ContentMainRunnerImpl::Run()
#13 0x7fad4fbe33e9 service_manager::Main()
#14 0x7fad5b3ce881 content::ContentMain()
#15 0x000000df231d content::BrowserTestBase::SetUp()

[177715:177745:1026/174146.462216:WARNING:discardable_shared_memory_manager.cc(409)] Some MojoDiscardableSharedMemoryManagerImpls are still alive. They will be leaked.
[177715:177857:1026/174146.472933:WARNING:internal_linux.cc(64)] Failed to read /proc/177756/stat
[  FAILED  ] WakeLockTest.OutOfProcessFrame, where TypeParam =  and GetParam() =  (2226 ms)

 
Description: Show this description
Cc: alogvi...@yandex-team.ru
Cc: jochen@chromium.org mlamouri@chromium.org nasko@chromium.org tsepez@chromium.org
This race may imply that if a new renderer process asks for a WakeLock before the old renderer is torn down, that the old renderer will end up clearing the lock on teardown?
Owner: alogvi...@yandex-team.ru
Status: Assigned (was: Untriaged)
https://chromium-review.googlesource.com/c/chromium/src/+/1303176 fixes the flake by making the test deterministic.

However the lock itself may still be flaky. alogvinov@ can you please look into this?

This change would cause the race to resolve more deterministically, which caused the test to fail before my above CL.

--- a/content/renderer/render_frame_impl.cc
+++ b/content/renderer/render_frame_impl.cc
@@ -2218,6 +2218,7 @@ void RenderFrameImpl::OnSwapOut(
   // Swap out and stop sending any IPC messages that are not ACKs.
   if (is_main_frame_)
     render_view_->SetSwappedOut(true);
+  base::PlatformThread::Sleep(base::TimeDelta::FromMilliseconds(333));
 
   RenderViewImpl* render_view = render_view_;
   bool is_main_frame = is_main_frame_;

Project Member

Comment 5 by bugdroid1@chromium.org, Oct 26

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

commit 3b11a3797aeeb9b7707c6e4171d4a76d7cfd122c
Author: danakj <danakj@chromium.org>
Date: Fri Oct 26 23:41:42 2018

Deflake the WakeLockTest.OutOfProcessFrame test.

The test takes a screen wake lock in an iframe, then navigates the
iframe to another origin, and verifies the lock is released.

The problem is the lock is released when the original iframe's renderer
tears down and sends FrameHostMsg_SwapOut_ACK to the browser. But
navigation in the iframe can complete before that (it's a race). So
the test now waits for the old frame's RenderFrameHost to be cleaned
up before verifying the lock is released.

R=avi@chromium.org

Change-Id: I4aed27d1a78b8fdf6451228e3ee125bd465f6f4f
Bug: 899384
Reviewed-on: https://chromium-review.googlesource.com/c/1303176
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#603254}
[modify] https://crrev.com/3b11a3797aeeb9b7707c6e4171d4a76d7cfd122c/content/browser/wake_lock/wake_lock_browsertest.cc

Sign in to add a comment