[Telemetry] Move startup_url computation to possible browsers
This should not be an implementation detail of the browser backends.
Bug: chromium:787834
Change-Id: I54e4b3a92ee67ccec20d4dc6114bc1a6225252ca
Reviewed-on: https://chromium-review.googlesource.com/c/1380131
Commit-Queue: Juan Antonio Navarro Pérez <perezju@chromium.org>
Auto-Submit: Juan Antonio Navarro Pérez <perezju@chromium.org>
Reviewed-by: Caleb Rouleau <crouleau@chromium.org>
diff --git a/telemetry/telemetry/internal/backends/chrome/android_browser_backend.py b/telemetry/telemetry/internal/backends/chrome/android_browser_backend.py
index 9be8e90..c866578 100644
--- a/telemetry/telemetry/internal/backends/chrome/android_browser_backend.py
+++ b/telemetry/telemetry/internal/backends/chrome/android_browser_backend.py
@@ -124,17 +124,6 @@
flags=[intent.FLAG_ACTIVITY_RESET_TASK_IF_NEEDED]),
blocking=True)
- def GetBrowserStartupUrl(self):
- # TODO(crbug.com/787834): Move to the corresponding possible-browser class.
- if self.browser_options.startup_url:
- return self.browser_options.startup_url
- elif self.browser_options.profile_dir:
- return None
- else:
- # If we have no existing tabs start with a blank page since default
- # startup with the NTP can lead to race conditions with Telemetry
- return 'about:blank'
-
def ForceJavaHeapGarbageCollection(self):
# Send USR1 signal to force GC on Chrome processes forked from Zygote.
# (c.f. crbug.com/724032)
diff --git a/telemetry/telemetry/internal/backends/chrome/android_browser_finder.py b/telemetry/telemetry/internal/backends/chrome/android_browser_finder.py
index ff2557d..94433f7 100644
--- a/telemetry/telemetry/internal/backends/chrome/android_browser_finder.py
+++ b/telemetry/telemetry/internal/backends/chrome/android_browser_finder.py
@@ -2,7 +2,7 @@
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
-"""Finds android browsers that can be controlled by telemetry."""
+"""Finds android browsers that can be started and controlled by telemetry."""
import contextlib
import logging
@@ -208,9 +208,21 @@
finally:
self._flag_changer = None
+ def _GetBrowserStartupUrl(self):
+ if self._browser_options.startup_url:
+ return self._browser_options.startup_url
+ elif self._browser_options.profile_dir:
+ return None
+ else:
+ # If we have no existing tabs start with a blank page since default
+ # startup with the NTP can lead to race conditions with Telemetry
+ return 'about:blank'
+
def Create(self, clear_caches=True):
"""Launch the browser on the device and return a Browser object."""
- return self._GetBrowserInstance(existing=False, clear_caches=clear_caches)
+ return self._GetBrowserInstance(
+ existing=False, clear_caches=clear_caches,
+ startup_url=self._GetBrowserStartupUrl())
def FindExistingBrowser(self):
"""Find a browser running on the device and bind a Browser object to it.
@@ -223,7 +235,7 @@
"""
return self._GetBrowserInstance(existing=True, clear_caches=False)
- def _GetBrowserInstance(self, existing, clear_caches):
+ def _GetBrowserInstance(self, existing, clear_caches, startup_url=None):
browser_backend = android_browser_backend.AndroidBrowserBackend(
self._platform_backend, self._browser_options,
self.browser_directory, self.profile_directory,
@@ -234,7 +246,8 @@
try:
returned_browser = browser.Browser(
browser_backend, self._platform_backend, startup_args=(),
- find_existing=existing)
+ startup_url=startup_url, find_existing=existing)
+ # TODO(crbug.com/916086): Move this assertion to callers.
if self._browser_options.assert_gpu_compositing:
gpu_compositing_checker.AssertGpuCompositingEnabled(
returned_browser.GetSystemInfo())
diff --git a/telemetry/telemetry/internal/backends/chrome/chrome_browser_backend.py b/telemetry/telemetry/internal/backends/chrome/chrome_browser_backend.py
index ade267a..be7ea2d 100644
--- a/telemetry/telemetry/internal/backends/chrome/chrome_browser_backend.py
+++ b/telemetry/telemetry/internal/backends/chrome/chrome_browser_backend.py
@@ -69,10 +69,6 @@
return True
return [arg for arg in args if arg.startswith('--proxy-server=')]
- def GetBrowserStartupUrl(self):
- # TODO(crbug.com/787834): Move to the corresponding possible-browser class.
- return None
-
def HasDevToolsConnection(self):
return self._devtools_client and self._devtools_client.IsAlive()
diff --git a/telemetry/telemetry/internal/backends/chrome/cros_browser_finder.py b/telemetry/telemetry/internal/backends/chrome/cros_browser_finder.py
index 8770782..2e83600 100644
--- a/telemetry/telemetry/internal/backends/chrome/cros_browser_finder.py
+++ b/telemetry/telemetry/internal/backends/chrome/cros_browser_finder.py
@@ -1,7 +1,7 @@
# Copyright 2013 The Chromium Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
-"""Finds CrOS browsers that can be controlled by telemetry."""
+"""Finds CrOS browsers that can be started and controlled by telemetry."""
import logging
import os
@@ -103,6 +103,7 @@
browser_backend, self._platform_backend, startup_args)
returned_browser = browser.Browser(
browser_backend, self._platform_backend, startup_args)
+ # TODO(crbug.com/916086): Move this assertion to callers.
if self._browser_options.assert_gpu_compositing:
gpu_compositing_checker.AssertGpuCompositingEnabled(
returned_browser.GetSystemInfo())
diff --git a/telemetry/telemetry/internal/backends/chrome/desktop_browser_backend.py b/telemetry/telemetry/internal/backends/chrome/desktop_browser_backend.py
index 54af7c1..67e2f5d 100644
--- a/telemetry/telemetry/internal/backends/chrome/desktop_browser_backend.py
+++ b/telemetry/telemetry/internal/backends/chrome/desktop_browser_backend.py
@@ -203,10 +203,6 @@
browser_target = lines[1] if len(lines) >= 2 else None
return devtools_port, browser_target
- def GetBrowserStartupUrl(self):
- # TODO(crbug.com/787834): Move to the corresponding possible-browser class.
- return self.browser_options.startup_url
-
def Start(self, startup_args, startup_url=None):
assert not self._proc, 'Must call Close() before Start()'
diff --git a/telemetry/telemetry/internal/backends/chrome/desktop_browser_finder.py b/telemetry/telemetry/internal/backends/chrome/desktop_browser_finder.py
index 2dddec4..f5fc1e5 100644
--- a/telemetry/telemetry/internal/backends/chrome/desktop_browser_finder.py
+++ b/telemetry/telemetry/internal/backends/chrome/desktop_browser_finder.py
@@ -1,7 +1,7 @@
# Copyright 2013 The Chromium Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
-"""Finds desktop browsers that can be controlled by telemetry."""
+"""Finds desktop browsers that can be started and controlled by telemetry."""
import logging
import os
@@ -140,6 +140,7 @@
# may not be guaranteed the same each time
# For example, see: crbug.com/865895#c17
startup_args = self.GetBrowserStartupArgs(self._browser_options)
+ startup_url = self._browser_options.startup_url
returned_browser = None
browser_backend = desktop_browser_backend.DesktopBrowserBackend(
@@ -152,7 +153,9 @@
self._ClearCachesOnStart()
returned_browser = browser.Browser(
- browser_backend, self._platform_backend, startup_args)
+ browser_backend, self._platform_backend, startup_args,
+ startup_url=startup_url)
+ # TODO(crbug.com/916086): Move this assertion to callers.
if self._browser_options.assert_gpu_compositing:
gpu_compositing_checker.AssertGpuCompositingEnabled(
returned_browser.GetSystemInfo())
diff --git a/telemetry/telemetry/internal/browser/browser.py b/telemetry/telemetry/internal/browser/browser.py
index 4e82e48..db51ddb 100644
--- a/telemetry/telemetry/internal/browser/browser.py
+++ b/telemetry/telemetry/internal/browser/browser.py
@@ -30,7 +30,7 @@
See telemetry.internal.browser.possible_browser for more details and use
cases.
"""
- def __init__(self, backend, platform_backend, startup_args,
+ def __init__(self, backend, platform_backend, startup_args, startup_url=None,
find_existing=False):
super(Browser, self).__init__(app_backend=backend,
platform_backend=platform_backend)
@@ -40,11 +40,10 @@
self._tabs = tab_list.TabList(backend.tab_list_backend)
self._browser_backend.SetBrowser(self)
if find_existing:
+ if startup_url is not None:
+ raise ValueError("Can't use startup_url and find_existing together.")
self._browser_backend.BindDevToolsClient()
else:
- # TODO(crbug.com/787834): Move url computation out of the browser
- # backend and into the callers of this constructor.
- startup_url = self._browser_backend.GetBrowserStartupUrl()
self._browser_backend.Start(startup_args, startup_url=startup_url)
self._LogBrowserInfo()
except Exception:
