New issue
Advanced search Search tips

Issue 695073 link

Starred by 1 user
Status: Untriaged
Owner: ----
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Investigate why is RunAllPendingInMessageLoop() needed for some extension tests to work

Project Member Reported by ah...@yandex-team.ru, Feb 22 2017 
Some tests (see the list below) that use ExtensionTestNotificationObserver
started failing after the removal of implicit deferred quit in the
WindowedNotificationObserver in https://codereview.chromium.org/2701473007/.
This means that waiting for the notification itself is not sufficient. It is
needed to find out what is missing, and (probably) add code to wait for that
event(s). Or just leave RunAllPending if it turns out that these events do not
involve other threads/processes and happen without any delay. Also it is
possible that these failures surfaced because of a race in production code.

linux_chromium_rel_ng
    browser_tests
        DeclarativeContentApiTest.UninstallWhileActivePageAction
        ExtensionLoadingTest.KeepAliveWithDevToolsOpenOnReload
        ExtensionLoadingTest.RuntimeValidWhileDevToolsOpen

linux_chromium_chromeos_rel_ng
    browser_tests
        AppListControllerDelegateAshTest.IsPlatformAppOpen
        DeclarativeContentApiTest.UninstallWhileActivePageAction
        DriveAppProviderTest.MatchingChromeAppInstalled
        DriveAppProviderTest.UserInstallResetsUninstallTracking
        ExtensionLoadingTest.KeepAliveWithDevToolsOpenOnReload
        ExtensionLoadingTest.RuntimeValidWhileDevToolsOpen

linux_chromium_chromeos_ozone_rel_ng
    unit_tests
        DeviceLocalAccountExternalPolicyLoaderTest.ForceInstallListSet

windows
    browser_tests
        ExtensionLoadingTest.KeepAliveWithDevToolsOpenOnReload
        ExtensionLoadingTest.RuntimeValidWhileDevToolsOpen

mac
    browser_tests
        DeclarativeContentApiTest.UninstallWhileActivePageAction
        ExtensionLoadingTest.KeepAliveWithDevToolsOpenOnReload
        ExtensionLoadingTest.RuntimeValidWhileDevToolsOpen
        ExtensionManagementTest.InstallSameVersion
        NativeAppWindowCocoaBrowserTestInstance_NativeAppWindowCocoaBrowserTest.HideShowWithAppWithShim_0
        NativeAppWindowCocoaBrowserTestInstance_NativeAppWindowCocoaBrowserTest.HideShowWithAppWithShim_1
        NativeAppWindowCocoaBrowserTestInstance_NativeAppWindowCocoaBrowserTest.Maximize_0
        NativeAppWindowCocoaBrowserTestInstance_NativeAppWindowCocoaBrowserTest.Maximize_1
        NativeAppWindowCocoaBrowserTestInstance_NativeAppWindowCocoaBrowserTest.MaximizeFullscreen_0
        NativeAppWindowCocoaBrowserTestInstance_NativeAppWindowCocoaBrowserTest.MaximizeFullscreen_1
        NativeAppWindowCocoaBrowserTestInstance_NativeAppWindowCocoaBrowserTest.Minimize_0
        NativeAppWindowCocoaBrowserTestInstance_NativeAppWindowCocoaBrowserTest.Minimize_1
        NativeAppWindowCocoaBrowserTestInstance_NativeAppWindowCocoaBrowserTest.MinimizeMaximize_0
        NativeAppWindowCocoaBrowserTestInstance_NativeAppWindowCocoaBrowserTest.MinimizeMaximize_1

Builds and logs can be found in https://codereview.chromium.org/2709183002.

Comment 1 by ah...@yandex-team.ru, Feb 27 2017 

It turned out that DeviceLocalAccountExternalPolicyLoaderTest.ForceInstallListSet failures were not fixed by patching ExtensionTestNotificationObserver, probably it is a different issue (though I don't think it deserves a separate bug).

This test has the following code at its end:

// Spin the loop until the cache shutdown callback is invoked. Verify that at
// that point, no further file I/O tasks are pending.
shutdown_run_loop.Run();
EXPECT_TRUE(base::MessageLoop::current()->IsIdleForTesting());

And this check is failing sometimes, because there are a few pending tasks all posted from CallOnHandleReady@../../mojo/public/cpp/system/watcher.cc:112. I guess that these tasks might be completely unrelated to the condition that the author intended to check.

Comment 2 by ah...@yandex-team.ru, Mar 6 2017

Components: Platform>Extensions

Comment 3 by ah...@yandex-team.ru, Mar 9 2017

Summary: Investigate why is RunAllPendingInMessageLoop() needed for some extension tests to work (was: Investigate why is RunAllPendingInMessageLoop() needed in ExtensionTestNotificationObserver)
One more instance of this was found: see jam's comment https://codereview.chromium.org/2720513003/#msg20 and corresponding patch https://codereview.chromium.org/2740783003/diff2/1:20001/chrome/browser/extensions/api/extension_action/browser_action_apitest.cc
►

Sign in to add a comment