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

Issue 787683 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Long OOO (go/where-is-mgiuca)
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug-Regression



Sign in to add a comment

ProfileShortcutManagerTest failing on chromium.win/Win 7 Tests x64 (1)

Project Member Reported by mgiuca@chromium.org, Nov 22 2017

Issue description

unit_tests failing on chromium.win/Win 7 Tests x64 (1)

Builders failed on: 
- Win 7 Tests x64 (1): 
  https://build.chromium.org/p/chromium.win/builders/Win%207%20Tests%20x64%20%281%29

Suddenly, ProfileShortcutManagerTest started failing with this:

[1899/7902] ProfileShortcutManagerTest.DesktopShortcutsCreateSecond (1061 ms)
<truncated (437296 bytes)>
posts A?).
[3736:5468:1121/174527.933:579216:WARNING:scoped_task_environment.cc(125)] ScopedTaskEnvironment::RunUntilIdle() appears to be stuck in an infinite loop (e.g. A posts B posts C posts A?).
[3736:5468:1121/174527.933:579216:WARNING:scoped_task_environment.cc(125)] ScopedTaskEnvironment::RunUntilIdle() appears to be stuck in an infinite loop (e.g. A posts B posts C posts A?).
[3736:5468:1121/174527.933:579216:WARNING:scoped_task_environment.cc(125)] ScopedTaskEnvironment::RunUntilIdle() appears to be stuck in an infinite loop (e.g. A posts B posts C posts A?).
[3736:5468:1121/174527.934:579216:WARNING:scoped_task_environment.cc(125)] ScopedTaskEnvironment::RunUntilIdle() appears to be stuck in an infinite loop (e.g. A posts B posts C posts A?).
[3736:5468:1121/174527.934:579216:WARNING:scoped_task_environment.cc(125)] ScopedTaskEnvironment::RunUntilIdle() appears to be stuck in an infinite loop (e.g. A posts B posts C posts A?).
[3736:5468:1121/174527.934:579216:WARNING:scoped_task_environment.cc(125)] ScopedTaskEnvironment::RunUntilIdle() appears to be stuck in an infinite loop (e.g. A posts B posts C posts A?).
[3736:5468:1121/174527.934:579216:WARNING:scoped_task_environment.cc(125)] ScopedTaskEnvironment::RunUntilIdle() appears to be stuck in an infinite loop (e.g. A posts B posts C posts A?).
[3736:5468:1121/174527.935:579216:WARNING:scoped_task_environment.cc(125)] ScopedTaskEnvironment::RunUntilIdle() appears to be stuck in an infinite loop (e.g. A posts B posts C posts A?).
[3736:5468:1121/174527.935:579216:WARNING:scoped_task_environment.cc(125)] ScopedTaskEnvironment::RunUntilIdle() appears to be stuck in an infinite loop (e.g. A posts B posts C posts A?).
[3736:5468:1121/174527.935:579216:WARNING:scoped_task_environment.cc(125)] ScopedTaskEnvironment::RunUntilIdle() appears to be stuck in an infinite loop (e.g. A posts B posts C posts A?).
[3736:5468:1121/174527.936:579216:WARNING:scoped_task_environment.cc(125)] ScopedTaskEnvironment::RunUntilIdle() appears to be stuck in an infinite loop (e.g. A posts B posts C posts A?).
[3736:5468:1121/174527.936:579216:WARNING:scoped_task_environment.cc(125)] ScopedTaskEnvironment::RunUntilIdle() appears to be stuck in an infinite loop (e.g. A posts B posts C posts A?).
[3736:5468:1121/174527.936:579216:WARNING:scoped_task_environment.cc(125)] ScopedTaskEnvironment::RunUntilIdle() appears to be stuck in an infinite loop (e.g. A posts B posts C posts A?).
[3736:5468:1121/174527.936:579216:WARNING:scoped_task_environment.cc(125)] ScopedTaskEnvironment::RunUntilIdle() appears to be stuck in an infinite loop (e.g. A posts B posts C posts A?).
[3736:5468:1121/174527.937:579216:WARNING:scoped_task_environment.cc(125)] ScopedTaskEnvironment::RunUntilIdle() appears to be stuck in an infinite loop (e.g. A posts B posts C posts A?).
[3736:5468:1121/174527.937:579216:WARNING:scoped_task_environment.cc(125)] ScopedTaskEnvironment::RunUntilIdle() appears to be stuck in an infinite loop (e.g. A posts B posts C posts A?).
[3736:5468:1121/174527.937:579216:WARNING:scoped_task_environment.cc(125)] ScopedTaskEnvironment::RunUntilIdle() appears to be stuck in an infinite loop (e.g. A posts B posts C posts A?).
[3736:5468:1121/174527.937:579216:WARNING:scoped_task_environment.cc(125)] ScopedTaskEnvironment::RunUntilIdle() appears to be stuck in an infinite loop (e.g. A posts B posts C posts A?).
[3736:5468:1121/174527.938:579216:WARNING:scoped_task_environment.cc(125)] ScopedTaskEnvironment::RunUntilIdle() appears to be stuck in an infinite loop (e.g. A posts B posts C posts A?).
[3736:5468:1121/174527.938:579216:WARNING:scoped_task_environment.cc(125)] ScopedTaskEnvironment::RunUntilIdle() appears to be stuck in an infinite loop (e.g. A posts B posts C posts A?).
[3736:5468:1121/174527.938:579216:WARNING:scoped_task_environment.cc(125)] ScopedTaskEnvironment::RunUntilIdle() appears to be stuck in an infinite loop (e.g. A posts B posts C posts A?).
[3736:5468:1121/174527.938:579216:WARNING:scoped_task_environment.cc(125)] ScopedTaskEnvironment::RunUntilIdle() appears to be stuck in an infinite loop (e.g. A posts B posts C posts A?).
[3736:5468:1121/174527.939:579216:WARNING:scoped_task_environment.cc(125)] ScopedTaskEnvironment::RunUntilIdle() appears to be stuck in an infinite loop (e.g. A posts B posts C posts A?).
[3736:5468:1121/174527.939:579216:WARNING:scoped_task_environment.cc(125)] ScopedTaskEnvironment::RunUntilIdle() appears to be stuck in an infinite loop (e.g. A posts B posts C posts A?).
[3736:5468:1121/174527.940:579216:WARNING:scoped_task_environment.cc(125)] ScopedTaskEnvironment::RunUntilIdle() appears to be stuck in an infinite loop (e.g. A posts B posts C posts A?).
[3736:5468:1121/174527.940:579216:WARNING:scoped_task_environment.cc(125)] ScopedTaskEnvironment::RunUntilIdle() appears to be stuck in an infinite loop (e.g. A posts B posts C posts A?).

First, two tests failed. Then all of them.

The blamelist includes just two CLs, both in Blink, so unlikely to have changed the behaviour of ProfileShortcutManagerTest. ProfileShortcutManagerTest has not changed since September and ProfileShortcutManager has not changed since October.

This regression is a mystery.
 

Comment 1 by mgiuca@chromium.org, Nov 22 2017

Summary: ProfileShortcutManagerTest failing on chromium.win/Win 7 Tests x64 (1) (was: unit_tests failing on chromium.win/Win 7 Tests x64 (1))

Comment 2 by mgiuca@chromium.org, Nov 22 2017

Now it's failed 3 of the last 78, so it's flake.

I'm going to have to disable ProfileShortcutManagerTest.

Comment 3 by mgiuca@chromium.org, Nov 22 2017

#2: Typo. I meant "3 of the last 7".

Comment 4 by mgiuca@chromium.org, Nov 22 2017

Cc: robliao@chromium.org gab@chromium.org
Status: Started (was: Assigned)
Ugh, disabling the test is way too hard: I don't know of a way to disable an entire test suite so I would have to individually disable 29 tests.

Instead I did some more digging. I think it's highly likely that r518433 is the culprit. It made significant changes to ScopedTaskEnvironment and landed two builds before the first failure (its first build was 31248). 4/13 builds since then have failed with the above error:

- https://luci-milo.appspot.com/buildbot/chromium.win/Win%207%20Tests%20x64%20%281%29/31250
- https://luci-milo.appspot.com/buildbot/chromium.win/Win%207%20Tests%20x64%20%281%29/31251
- https://luci-milo.appspot.com/buildbot/chromium.win/Win%207%20Tests%20x64%20%281%29/31255
- https://luci-milo.appspot.com/buildbot/chromium.win/Win%207%20Tests%20x64%20%281%29/31257

Therefore, reverting r518433 and its follow-up, r518521.
Project Member

Comment 5 by bugdroid1@chromium.org, Nov 22 2017

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

commit 5cf78561bec62240ed797baf804d587e749ee250
Author: Matt Giuca <mgiuca@chromium.org>
Date: Wed Nov 22 07:24:11 2017

Revert "Nits follow-up to https://chromium-review.googlesource.com/c/chromium/src/+/638550"

This reverts commit 0af3ff209e663b1df60bdd53dac0ffc2ee3d6340.

Reason for revert: Blocking revert of r518433.
Bug:  787683 

Original change's description:
> Nits follow-up to https://chromium-review.googlesource.com/c/chromium/src/+/638550
> 
> R=​robliao@chomium.org
> 
> Bug: 708584
> Change-Id: I9c362e5cc3880644392cc6afcf4754d2fc834ea8
> Reviewed-on: https://chromium-review.googlesource.com/783873
> Commit-Queue: Gabriel Charette <gab@chromium.org>
> Reviewed-by: Robert Liao <robliao@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#518521}

TBR=gab@chromium.org,robliao@chromium.org,robliao@chomium.org

Change-Id: I832c5f3aba7073614fa331dc427d9ef0f1053f52
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 708584
Reviewed-on: https://chromium-review.googlesource.com/784571
Reviewed-by: Matt Giuca <mgiuca@chromium.org>
Commit-Queue: Matt Giuca <mgiuca@chromium.org>
Cr-Commit-Position: refs/heads/master@{#518559}
[modify] https://crrev.com/5cf78561bec62240ed797baf804d587e749ee250/base/test/scoped_task_environment.cc
[modify] https://crrev.com/5cf78561bec62240ed797baf804d587e749ee250/base/test/scoped_task_environment.h

Project Member

Comment 6 by bugdroid1@chromium.org, Nov 22 2017

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

commit 43644430175628bf838aae733530e25f96bf40c0
Author: Matt Giuca <mgiuca@chromium.org>
Date: Wed Nov 22 07:27:18 2017

Revert "Add MOCK_TIME mode to ScopedTaskEnvironment :)"

This reverts commit 5e2df665dfce3a3aceb4852d90ad55d31fc36f20.

Reason for revert: Suspect causing flaky ProfileShortcutManagerTest.
Bug:  787683 

Original change's description:
> Add MOCK_TIME mode to ScopedTaskEnvironment :)
> 
> Taking advantage of the new kBoundToThread mode on
> TestMockTimeTaskRunner.
> 
> This change also required tweaking the
> ScopedTaskEnvironment::RunUntilIdle() logic as RunLoop().Run() on
> TestMockTimeTaskRunner results in advancing time when there's no
> request to quit-when-idle which is undesired here. New logic gets rid
> of need for |on_queue_empty_closure_| and I think is simpler overall.
> As of patch set 20, this new RunUntilIdle() logic also avoids using
> TaskScheduler::FlushForTesting() as that can result in hangs should a
> TaskScheduler task synchronously block on the main thread.
> 
> R=​fdoray@chromium.org
> TBR=gab@chromium.org (IWYU fixes)
> 
> Bug: 708584
> Change-Id: I76ba55ec64d398151420379d3fcdcd5186fbceb8
> Reviewed-on: https://chromium-review.googlesource.com/638550
> Commit-Queue: Gabriel Charette <gab@chromium.org>
> Reviewed-by: Gabriel Charette <gab@chromium.org>
> Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
> Reviewed-by: François Doray <fdoray@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#518433}

TBR=khorimoto@chromium.org,gab@chromium.org,fdoray@chromium.org,robliao@chromium.org

Change-Id: I8a8822b221c73159f02297a6b7bdd60ddc5c49bc
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 708584
Reviewed-on: https://chromium-review.googlesource.com/784512
Reviewed-by: Matt Giuca <mgiuca@chromium.org>
Commit-Queue: Matt Giuca <mgiuca@chromium.org>
Cr-Commit-Position: refs/heads/master@{#518560}
[modify] https://crrev.com/43644430175628bf838aae733530e25f96bf40c0/base/observer_list_unittest.cc
[modify] https://crrev.com/43644430175628bf838aae733530e25f96bf40c0/base/task_scheduler/task_tracker.cc
[modify] https://crrev.com/43644430175628bf838aae733530e25f96bf40c0/base/task_scheduler/task_tracker.h
[modify] https://crrev.com/43644430175628bf838aae733530e25f96bf40c0/base/test/scoped_task_environment.cc
[modify] https://crrev.com/43644430175628bf838aae733530e25f96bf40c0/base/test/scoped_task_environment.h
[modify] https://crrev.com/43644430175628bf838aae733530e25f96bf40c0/base/test/scoped_task_environment_unittest.cc
[modify] https://crrev.com/43644430175628bf838aae733530e25f96bf40c0/media/test/pipeline_integration_test_base.h
[modify] https://crrev.com/43644430175628bf838aae733530e25f96bf40c0/remoting/host/file_proxy_wrapper_linux_unittest.cc

Comment 7 by mgiuca@chromium.org, Nov 22 2017

Will keep open to check that the flake is resolved. (I am about to leave sheriffing for the night.)


Next sheriff: shall we say, if this error isn't seen for the next 5 builds after r518560 on:
https://luci-milo.appspot.com/buildbot/chromium.win/Win%207%20Tests%20x64%20%281%29/
close as Fixed?

Comment 8 by olka@chromium.org, Nov 22 2017

Has not happened in the last 9 builds, closing as fixed.

Comment 9 by olka@chromium.org, Nov 22 2017

Labels: -Sheriff-Chromium
Status: Fixed (was: Started)
Project Member

Comment 10 by bugdroid1@chromium.org, Nov 24 2017

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

commit d4723a3dfd0215a72507d10ce0bd0e75f5f76bb6
Author: Gabriel Charette <gab@chromium.org>
Date: Fri Nov 24 00:11:18 2017

Reland "Add MOCK_TIME mode to ScopedTaskEnvironment :)"

Reland follow-up nits CL along with it this time
(https://chromium-review.googlesource.com/c/chromium/src/+/783873)

Initial CL made ProfileShortcutManagerTest flaky.

Running them locally it turns out they mostly pass but they spew out
a whole lot of "ScopedTaskEnvironment::RunUntilIdle() appears to be
stuck in infinite loop" warning logs (which were added in this CL).
This is because the ongoing tasks that prevent DisallowRunTasks()
are slow and the main thread goes into a busy loop trying to
DisallowRunTasks(). These tests being already slow, these prints
pushed them over the edge (I think).

DisallowRunTasks() was tweaked in this reland to not return until
either one task completes or a short timeout expires (see code
comments for detailed reasoning).

This also allows removing the YieldCurrentThread() logic in the original
CL as busy-looping is avoided altogether.

The LOG statement trying to catch infinite RunUntilIdle() was also
removed as it wasn't really the nice place for it (it should a counter
in the RunOrSkipTask() override if we want it later).

This is a reland of 5e2df665dfce3a3aceb4852d90ad55d31fc36f20
Original change's description:
> Add MOCK_TIME mode to ScopedTaskEnvironment :)
>
> Taking advantage of the new kBoundToThread mode on
> TestMockTimeTaskRunner.
>
> This change also required tweaking the
> ScopedTaskEnvironment::RunUntilIdle() logic as RunLoop().Run() on
> TestMockTimeTaskRunner results in advancing time when there's no
> request to quit-when-idle which is undesired here. New logic gets rid
> of need for |on_queue_empty_closure_| and I think is simpler overall.
> As of patch set 20, this new RunUntilIdle() logic also avoids using
> TaskScheduler::FlushForTesting() as that can result in hangs should a
> TaskScheduler task synchronously block on the main thread.
>
> R=fdoray@chromium.org
> TBR=gab@chromium.org (IWYU fixes)
>
> Bug: 708584
> Change-Id: I76ba55ec64d398151420379d3fcdcd5186fbceb8
> Reviewed-on: https://chromium-review.googlesource.com/638550
> Commit-Queue: Gabriel Charette <gab@chromium.org>
> Reviewed-by: Gabriel Charette <gab@chromium.org>
> Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
> Reviewed-by: François Doray <fdoray@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#518433}

TEST=Repro'ed  https://crbug.com/787683  locally and fixed.
TBR=gab@chromium.org (IWYU fixes)

Bug: 708584,  787683 
Change-Id: I511c159ccbb608aa54394a4f35c40ad14697196b
Reviewed-on: https://chromium-review.googlesource.com/788133
Commit-Queue: Gabriel Charette <gab@chromium.org>
Reviewed-by: François Doray <fdoray@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519032}
[modify] https://crrev.com/d4723a3dfd0215a72507d10ce0bd0e75f5f76bb6/base/observer_list_unittest.cc
[modify] https://crrev.com/d4723a3dfd0215a72507d10ce0bd0e75f5f76bb6/base/task_scheduler/task_tracker.cc
[modify] https://crrev.com/d4723a3dfd0215a72507d10ce0bd0e75f5f76bb6/base/task_scheduler/task_tracker.h
[modify] https://crrev.com/d4723a3dfd0215a72507d10ce0bd0e75f5f76bb6/base/test/scoped_task_environment.cc
[modify] https://crrev.com/d4723a3dfd0215a72507d10ce0bd0e75f5f76bb6/base/test/scoped_task_environment.h
[modify] https://crrev.com/d4723a3dfd0215a72507d10ce0bd0e75f5f76bb6/base/test/scoped_task_environment_unittest.cc
[modify] https://crrev.com/d4723a3dfd0215a72507d10ce0bd0e75f5f76bb6/media/test/pipeline_integration_test_base.h
[modify] https://crrev.com/d4723a3dfd0215a72507d10ce0bd0e75f5f76bb6/remoting/host/file_proxy_wrapper_linux_unittest.cc

Sign in to add a comment