Lacros: fix a deployment bug where ash-chrome is not restarted
Currently, when deploying lacros-chrome and the config file is
modified, ash-chrome will not be restarted automatically, which is a
bug. This CL fixes the bug and also fixes an incorrect assumption in
the unit tests.
BUG=chromium:1156369
TEST=unit tests + manual testing
Change-Id: Ie84c6c9eeab5d31752ec31bd07c6967153d4ba4d
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/chromite/+/2606845
Tested-by: Yuke Liao <liaoyuke@chromium.org>
Commit-Queue: Yuke Liao <liaoyuke@chromium.org>
Reviewed-by: Ben Pastene <bpastene@chromium.org>
diff --git a/scripts/deploy_chrome.py b/scripts/deploy_chrome.py
index 70466ba..2a94d91 100644
--- a/scripts/deploy_chrome.py
+++ b/scripts/deploy_chrome.py
@@ -481,12 +481,15 @@
self._MountRootfsAsWritable,
self._PrepareStagingDir]
- # If this is a lacros build, we only want to restart ash-chrome if
- # necessary, which is done below.
- if not self.options.lacros:
+ # If this is a lacros build, we only want to restart ash-chrome if needed.
+ restart_ui = True
+ if self.options.lacros:
+ steps.append(self._KillLacrosChrome)
+ restart_ui = self._ModifyConfigFileIfNeededForLacros()
+ if restart_ui:
+ steps.append(self._KillAshChromeIfNeeded)
self._stopped_ui = True
- steps += ([self._KillLacrosChrome] if self.options.lacros else
- [self._KillAshChromeIfNeeded])
+
ret = parallel.RunParallelSteps(steps, halt_on_error=True,
return_values=True)
self._CheckDeviceFreeSpace(ret[0])
@@ -511,27 +514,39 @@
if self.options.mount_dir is not None:
self._MountTarget()
- if self.options.lacros:
- # Update /etc/chrome_dev.conf to include appropriate flags.
- restart_ui = False
- result = self.device.run(ENABLE_LACROS_VIA_CONF_COMMAND, shell=True)
- if result.stdout.strip() == MODIFIED_CONF_FILE:
- restart_ui = True
- result = self.device.run(_SET_LACROS_PATH_VIA_CONF_COMMAND % {
- 'conf_file': _CONF_FILE, 'lacros_path': self.options.target_dir,
- 'modified_conf_file': MODIFIED_CONF_FILE}, shell=True)
- if result.stdout.strip() == MODIFIED_CONF_FILE:
- restart_ui = True
-
- if restart_ui:
- self._KillAshChromeIfNeeded()
-
# Actually deploy Chrome to the device.
self._Deploy()
if self.options.deploy_test_binaries:
self._DeployTestBinaries()
+ def _ModifyConfigFileIfNeededForLacros(self):
+ """Modifies the /etc/chrome_dev.conf file for lacros-chrome.
+
+ Returns:
+ True if the file is modified, and the return value is usually used to
+ determine whether restarting ash-chrome is needed.
+ """
+ assert self.options.lacros, (
+ 'Only deploying lacros-chrome needs to modify the config file.')
+ # Update /etc/chrome_dev.conf to include appropriate flags.
+ modified = False
+ result = self.device.run(ENABLE_LACROS_VIA_CONF_COMMAND, shell=True)
+ if result.stdout.strip() == MODIFIED_CONF_FILE:
+ modified = True
+ result = self.device.run(
+ _SET_LACROS_PATH_VIA_CONF_COMMAND % {
+ 'conf_file': _CONF_FILE,
+ 'lacros_path': self.options.target_dir,
+ 'modified_conf_file': MODIFIED_CONF_FILE
+ },
+ shell=True)
+ if result.stdout.strip() == MODIFIED_CONF_FILE:
+ modified = True
+
+ return modified
+
+
def ValidateStagingFlags(value):
"""Convert formatted string to dictionary."""
return chrome_util.ProcessShellFlags(value)
diff --git a/scripts/deploy_chrome_unittest.py b/scripts/deploy_chrome_unittest.py
index ccaa01c..273a9cc 100644
--- a/scripts/deploy_chrome_unittest.py
+++ b/scripts/deploy_chrome_unittest.py
@@ -449,6 +449,7 @@
# Raises a RunCommandError on its first invocation, but passes on subsequent
# calls.
def SideEffect(*args, **kwargs):
+ # pylint: disable=unused-argument
if not SideEffect.called:
SideEffect.called = True
raise cros_build_lib.RunCommandError('fail')
@@ -490,16 +491,10 @@
self.deploy._MountRootfsAsWritable = mock.Mock()
self.deploy._PrepareStagingDir = mock.Mock()
self.deploy._CheckDeviceFreeSpace = mock.Mock()
-
- self._ran_start_command = False
-
+ self.deploy._KillAshChromeIfNeeded = mock.Mock()
self.StartPatcher(parallel_unittest.ParallelMock())
- # Common mocking shared between tests.
- def kill_procs_side_effect():
- self.deploy._stopped_ui = True
- self.deploy._KillAshChromeIfNeeded = mock.Mock(
- side_effect=kill_procs_side_effect)
+ self._ran_start_command = False
def start_ui_side_effect(*args, **kwargs):
# pylint: disable=unused-argument
