DBusThreadManager::GetSetterForTesting() should not alter the online/offline state of browsertests |
|
Issue description
Task:
Ensure that calls to DBusThreadManager::GetSetterForTesting in a browser test's SetUpInProcessBrowserTestFixture() do not have side effects on FakeShillManagerClient's state.
---
Background:
browser_tests tend to call DBusThreadManager::GetSetterForTesting in SetUpInProcessBrowserTestFixture() to install e.g. a fake session manager client or a fake system clock.
(If called in SetUpOnMainThread(), it's too late - CHECK(!g_dbus_thread_manager)[1] fails)
Calling GetSetterForTesting results in the call chain:
- DBusThreadManager::GetSetterForTesting
- DBusThreadManager::InitializeClients
- DBusClientsCommon::Initialize
- FakeShillManagerClient::SetupDefaultEnvironment
However, in SetUpInProcessBrowserTestFixture, there is no ThreadTaskRunnerHandle yet (base::ThreadTaskRunnerHandle::Get() returns nullptr).
This leads to FakeShillManagerClient::SetupDefaultEnvironment skipping its usual setup work[2].
The InitializeClients call will not be repeated - it is skipped in DBusThreadManager::Initialize[3] if GetSetterForTesting was called previously.
This means that FakeShillManagerClient does not have the default "fake networks" set up as a side-effect of calling DBusThreadManager::GetSetterForTesting in SetUpInProcessBrowserTestFixture.
An effect of this is that NetworkChangeNotifier::IsOffline() returns true[4], which makes HostResolverImpl::ProcTask::OnLookupComplete[5] remaps errors on host resolution to ERR_INTERNET_DISCONNECTED[6].
Note that non-localhost URL resolution defaults to returning ERR_NOT_IMPLEMENTED in browsertest due to using TestHostResolver[7] in browsertests.
An effect of this is that device_management_service retries DMServer requests, because IsConnectionError[9] now returns true. This causes some (e.g. FRE) tests to time out.
---
Workaround:
Manually call
DBusThreadManager::Get()
->GetShillManagerClient()
->GetTestInterface()
->SetupDefaultEnvironment();
e.g. in SetUpOnMainThread().
[1] https://cs.chromium.org/chromium/src/chromeos/dbus/dbus_thread_manager.cc?rcl=06b9fe48cdf3660a7d7124937fee1c7ff026553e&l=317
[2] https://cs.chromium.org/chromium/src/chromeos/dbus/fake_shill_manager_client.cc?rcl=5b0a03c08e1dd3bf9616cb1fe7024ca72f6af258&l=617
[3] https://cs.chromium.org/chromium/src/chromeos/dbus/dbus_thread_manager.cc?rcl=5b0a03c08e1dd3bf9616cb1fe7024ca72f6af258&l=296
[4] https://cs.chromium.org/chromium/src/net/base/network_change_notifier.h?rcl=5b0a03c08e1dd3bf9616cb1fe7024ca72f6af258&l=365
[5] https://cs.chromium.org/chromium/src/net/dns/host_resolver_impl.cc?rcl=5b0a03c08e1dd3bf9616cb1fe7024ca72f6af258&l=816
[6] https://cs.chromium.org/chromium/src/net/dns/host_resolver_impl.cc?rcl=5b0a03c08e1dd3bf9616cb1fe7024ca72f6af258&l=816
[7] https://cs.chromium.org/chromium/src/content/public/test/test_host_resolver.cc?type=cs&sq=package:chromium&g=0&l=64
[8] https://cs.chromium.org/chromium/src/components/policy/core/common/cloud/device_management_service.cc?rcl=5b0a03c08e1dd3bf9616cb1fe7024ca72f6af258&l=82
|
|
►
Sign in to add a comment |
|