From aee4c9f50df8c0ab073c4313b292c3f5276d0b00 Mon Sep 17 00:00:00 2001 From: jam Date: Tue, 26 Aug 2014 10:22:50 -0700 Subject: [PATCH] Revert of Add support for codepen to form_based_credentials_background (patchset #9 of https://codereview.chromium.org/485743002/) Reason for revert: The test change is causing flakiness on trybots even with retries, see telemetry/core/backends/google_credentials_backend_unittest/TestGoogleCredentialsBackend/testLoginUsingMock: http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_swarming/builds/5713 http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_swarming/builds/5703 http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_swarming/builds/6959 http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_swarming/builds/9980 http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_swarming/builds/9973 http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_rel_swarming/builds/2974 http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_rel_swarming/builds/2966 http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_swarming/builds/6950 Original issue's description: > Add support for codepen to form_based_credentials_background > > Fix google credentials support. > > Migrate to action_runner when possible. > > BUG= > > Committed: https://chromium.googlesource.com/chromium/src/+/a64e885edb85f66035f35a848c0add3726a663af TBR=tonyg@chromium.org,sullivan@chromium.org NOTREECHECKS=true NOTRY=true BUG= Review URL: https://codereview.chromium.org/502533003 Cr-Commit-Position: refs/heads/master@{#291925} --- .../perf/page_sets/data/credentials.json.sha1 | 2 +- .../backends/codepen_credentials_backend.py | 41 ----------- .../codepen_credentials_backend_unittest.py | 18 ----- .../backends/facebook_credentials_backend.py | 10 +-- .../facebook_credentials_backend_unittest.py | 4 +- .../form_based_credentials_backend.py | 73 +++++++++---------- ...based_credentials_backend_unittest_base.py | 17 ++--- .../backends/google_credentials_backend.py | 8 +- .../google_credentials_backend_unittest.py | 5 +- .../telemetry/core/browser_credentials.py | 6 +- .../core/browser_credentials_unittest.py | 2 +- 11 files changed, 53 insertions(+), 133 deletions(-) delete mode 100644 tools/telemetry/telemetry/core/backends/codepen_credentials_backend.py delete mode 100644 tools/telemetry/telemetry/core/backends/codepen_credentials_backend_unittest.py diff --git a/tools/perf/page_sets/data/credentials.json.sha1 b/tools/perf/page_sets/data/credentials.json.sha1 index dd0deb54ecfc8..76f00e0f76dbb 100644 --- a/tools/perf/page_sets/data/credentials.json.sha1 +++ b/tools/perf/page_sets/data/credentials.json.sha1 @@ -1 +1 @@ -3a0f9c995a41e058857af7d9c2e265b046ba96e6 \ No newline at end of file +11752daf3b27c9ced2d530b5241cab7e5b109dd9 \ No newline at end of file diff --git a/tools/telemetry/telemetry/core/backends/codepen_credentials_backend.py b/tools/telemetry/telemetry/core/backends/codepen_credentials_backend.py deleted file mode 100644 index a12ae4b0c179c..0000000000000 --- a/tools/telemetry/telemetry/core/backends/codepen_credentials_backend.py +++ /dev/null @@ -1,41 +0,0 @@ -# Copyright 2014 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. - -from telemetry.core.backends import form_based_credentials_backend - - -class CodePenCredentialsBackend( - form_based_credentials_backend.FormBasedCredentialsBackend): - - @property - def logged_in_javascript(self): - """Evaluates to true iff already logged in.""" - return 'document.querySelector(".login-area") === null' - - @property - def credentials_type(self): - return 'codepen' - - @property - def url(self): - return 'https://codepen.io/login' - - @property - def login_form_id(self): - return 'login-login-form' - - @property - def login_button_javascript(self): - return """ - LoginSettings.timeOnPageStartTime = 0; - document.getElementById("log-in-button").click(); - """ - - @property - def login_input_id(self): - return 'login-email-field' - - @property - def password_input_id(self): - return 'login-password-field_' diff --git a/tools/telemetry/telemetry/core/backends/codepen_credentials_backend_unittest.py b/tools/telemetry/telemetry/core/backends/codepen_credentials_backend_unittest.py deleted file mode 100644 index 2142429f220e4..0000000000000 --- a/tools/telemetry/telemetry/core/backends/codepen_credentials_backend_unittest.py +++ /dev/null @@ -1,18 +0,0 @@ -# Copyright 2014 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. -from telemetry.core.backends import form_based_credentials_backend_unittest_base -from telemetry.core.backends import codepen_credentials_backend - - -class TestCodePenCredentialsBackend( - form_based_credentials_backend_unittest_base. - FormBasedCredentialsBackendUnitTestBase): - def setUp(self): - self._credentials_type = 'codepen' - - def testLoginUsingMock(self): - self._LoginUsingMock( - codepen_credentials_backend.CodePenCredentialsBackend(), - 'https://codepen.io/login', 'login-email-field', 'login-password', - 'login-login-form', 'document.querySelector(".login-area") === null') diff --git a/tools/telemetry/telemetry/core/backends/facebook_credentials_backend.py b/tools/telemetry/telemetry/core/backends/facebook_credentials_backend.py index b391e9f91992b..2b66f9dc1063f 100644 --- a/tools/telemetry/telemetry/core/backends/facebook_credentials_backend.py +++ b/tools/telemetry/telemetry/core/backends/facebook_credentials_backend.py @@ -7,12 +7,10 @@ class FacebookCredentialsBackend( form_based_credentials_backend.FormBasedCredentialsBackend): - - @property - def logged_in_javascript(self): - """Evaluates to true iff already logged in.""" - return ('document.getElementById("fbNotificationsList")!== null || ' - 'document.getElementById("m_home_notice")!== null') + def IsAlreadyLoggedIn(self, tab): + return tab.EvaluateJavaScript( + 'document.getElementById("fbNotificationsList")!== null || ' + 'document.getElementById("m_home_notice")!== null') @property def credentials_type(self): diff --git a/tools/telemetry/telemetry/core/backends/facebook_credentials_backend_unittest.py b/tools/telemetry/telemetry/core/backends/facebook_credentials_backend_unittest.py index a14185d72bdd1..2f6b22f9a94a9 100644 --- a/tools/telemetry/telemetry/core/backends/facebook_credentials_backend_unittest.py +++ b/tools/telemetry/telemetry/core/backends/facebook_credentials_backend_unittest.py @@ -15,6 +15,4 @@ def setUp(self): def testLoginUsingMock(self): self._LoginUsingMock( facebook_credentials_backend.FacebookCredentialsBackend(), - 'http://www.facebook.com/', 'email', 'pass', 'login_form', - ('document.getElementById("fbNotificationsList")!== null || ' - 'document.getElementById("m_home_notice")!== null')) + 'http://www.facebook.com/', 'email', 'pass') diff --git a/tools/telemetry/telemetry/core/backends/form_based_credentials_backend.py b/tools/telemetry/telemetry/core/backends/form_based_credentials_backend.py index 93c2cc26e5872..bb32f32251a34 100644 --- a/tools/telemetry/telemetry/core/backends/form_based_credentials_backend.py +++ b/tools/telemetry/telemetry/core/backends/form_based_credentials_backend.py @@ -6,12 +6,32 @@ from telemetry.core import util +def _WaitForLoginFormToLoad(backend, login_form_id, tab): + def IsFormLoadedOrAlreadyLoggedIn(): + return (tab.EvaluateJavaScript( + 'document.querySelector("#%s")!== null' % login_form_id) or + backend.IsAlreadyLoggedIn(tab)) + + # Wait until the form is submitted and the page completes loading. + util.WaitFor(IsFormLoadedOrAlreadyLoggedIn, 60) + +def _SubmitFormAndWait(form_id, tab): + tab.ExecuteJavaScript( + 'document.getElementById("%s").submit();' % form_id) + + def FinishedLoading(): + return not tab.EvaluateJavaScript( + 'document.querySelector("#%s") !== null' % form_id) + + # Wait until the form is submitted and the page completes loading. + util.WaitFor(FinishedLoading, 60) + class FormBasedCredentialsBackend(object): def __init__(self): self._logged_in = False def IsAlreadyLoggedIn(self, tab): - return tab.EvaluateJavaScript(self.logged_in_javascript) + raise NotImplementedError() @property def credentials_type(self): @@ -25,11 +45,6 @@ def url(self): def login_form_id(self): raise NotImplementedError() - @property - def login_button_javascript(self): - """Some sites have custom JS to log in.""" - return None - @property def login_input_id(self): raise NotImplementedError() @@ -38,11 +53,6 @@ def login_input_id(self): def password_input_id(self): raise NotImplementedError() - @property - def logged_in_javascript(self): - """Evaluates to true iff already logged in.""" - raise NotImplementedError() - def IsLoggedIn(self): return self._logged_in @@ -52,31 +62,7 @@ def _ResetLoggedInState(self): """ self._logged_in = False - def _WaitForLoginState(self, action_runner): - """Waits until it can detect either the login form, or already logged in.""" - condition = '(document.querySelector("#%s") !== null) || (%s)' % ( - self.login_form_id, self.logged_in_javascript) - action_runner.WaitForJavaScriptCondition(condition, 60) - - def _SubmitLoginFormAndWait(self, action_runner, tab, username, password): - """Submits the login form and waits for the navigation.""" - tab.WaitForDocumentReadyStateToBeInteractiveOrBetter() - email_id = 'document.querySelector("#%s #%s").value = "%s"; ' % ( - self.login_form_id, self.login_input_id, username) - password = 'document.querySelector("#%s #%s").value = "%s"; ' % ( - self.login_form_id, self.password_input_id, password) - tab.ExecuteJavaScript(email_id) - tab.ExecuteJavaScript(password) - if self.login_button_javascript: - tab.ExecuteJavaScript(self.login_button_javascript) - else: - tab.ExecuteJavaScript( - 'document.getElementById("%s").submit();' % self.login_form_id) - # Wait for the form element to disappear as confirmation of the navigation. - action_runner.WaitForNavigate() - - - def LoginNeeded(self, tab, action_runner, config): + def LoginNeeded(self, tab, config): """Logs in to a test account. Raises: @@ -100,14 +86,23 @@ def LoginNeeded(self, tab, action_runner, config): try: logging.info('Loading %s...', url) tab.Navigate(url) - self._WaitForLoginState(action_runner) + _WaitForLoginFormToLoad(self, self.login_form_id, tab) if self.IsAlreadyLoggedIn(tab): self._logged_in = True return True - self._SubmitLoginFormAndWait( - action_runner, tab, config['username'], config['password']) + tab.WaitForDocumentReadyStateToBeInteractiveOrBetter() + logging.info('Loaded page: %s', url) + + email_id = 'document.querySelector("#%s").%s.value = "%s"; ' % ( + self.login_form_id, self.login_input_id, config['username']) + password = 'document.querySelector("#%s").%s.value = "%s"; ' % ( + self.login_form_id, self.password_input_id, config['password']) + tab.ExecuteJavaScript(email_id) + tab.ExecuteJavaScript(password) + + _SubmitFormAndWait(self.login_form_id, tab) self._logged_in = True return True diff --git a/tools/telemetry/telemetry/core/backends/form_based_credentials_backend_unittest_base.py b/tools/telemetry/telemetry/core/backends/form_based_credentials_backend_unittest_base.py index ef292d6853bf1..3ce0c864aa0cb 100644 --- a/tools/telemetry/telemetry/core/backends/form_based_credentials_backend_unittest_base.py +++ b/tools/telemetry/telemetry/core/backends/form_based_credentials_backend_unittest_base.py @@ -95,23 +95,18 @@ def testLoginUsingMock(self): raise NotImplementedError() def _LoginUsingMock(self, backend, login_page_url, email_element_id, - password_element_id, form_element_id, - already_logged_in_js): # pylint: disable=R0201 + password_element_id): # pylint: disable=R0201 tab = simple_mock.MockObject() - ar = simple_mock.MockObject() config = {'username': 'blah', 'password': 'blargh'} tab.ExpectCall('Navigate', login_page_url) - tab.ExpectCall('EvaluateJavaScript', already_logged_in_js).WillReturn(False) + tab.ExpectCall('EvaluateJavaScript', _).WillReturn(False) + tab.ExpectCall('EvaluateJavaScript', _).WillReturn(True) + tab.ExpectCall('EvaluateJavaScript', _).WillReturn(False) tab.ExpectCall('WaitForDocumentReadyStateToBeInteractiveOrBetter') - ar.ExpectCall('WaitForJavaScriptCondition', - '(document.querySelector("#%s") !== null) || (%s)' % ( - form_element_id, already_logged_in_js), 60) - ar.ExpectCall('WaitForNavigate') - def VerifyEmail(js): assert email_element_id in js assert 'blah' in js @@ -123,10 +118,10 @@ def VerifyPw(js): tab.ExpectCall('ExecuteJavaScript', _).WhenCalled(VerifyPw) def VerifySubmit(js): - assert '.submit' in js or '.click' in js + assert '.submit' in js tab.ExpectCall('ExecuteJavaScript', _).WhenCalled(VerifySubmit) # Checking for form still up. tab.ExpectCall('EvaluateJavaScript', _).WillReturn(False) - backend.LoginNeeded(tab, ar, config) + backend.LoginNeeded(tab, config) diff --git a/tools/telemetry/telemetry/core/backends/google_credentials_backend.py b/tools/telemetry/telemetry/core/backends/google_credentials_backend.py index 1d7cea174b771..780a5ea1f2f11 100644 --- a/tools/telemetry/telemetry/core/backends/google_credentials_backend.py +++ b/tools/telemetry/telemetry/core/backends/google_credentials_backend.py @@ -7,11 +7,9 @@ class GoogleCredentialsBackend( form_based_credentials_backend.FormBasedCredentialsBackend): - - @property - def logged_in_javascript(self): - """Evaluates to true iff already logged in.""" - return 'document.getElementById("gb")!== null' + def IsAlreadyLoggedIn(self, tab): + return tab.EvaluateJavaScript( + 'document.getElementById("gb")!== null') @property def credentials_type(self): diff --git a/tools/telemetry/telemetry/core/backends/google_credentials_backend_unittest.py b/tools/telemetry/telemetry/core/backends/google_credentials_backend_unittest.py index 45f6c7680d802..f6dc050230145 100644 --- a/tools/telemetry/telemetry/core/backends/google_credentials_backend_unittest.py +++ b/tools/telemetry/telemetry/core/backends/google_credentials_backend_unittest.py @@ -13,6 +13,5 @@ def setUp(self): def testLoginUsingMock(self): self._LoginUsingMock(google_credentials_backend.GoogleCredentialsBackend(), - 'https://accounts.google.com/', 'Email', 'Passwd', - 'gaia_loginform', - 'document.getElementById("gb")!== null') + 'https://accounts.google.com/', 'Email', + 'Passwd') diff --git a/tools/telemetry/telemetry/core/browser_credentials.py b/tools/telemetry/telemetry/core/browser_credentials.py index b44e47b07a115..d78ced98568ed 100644 --- a/tools/telemetry/telemetry/core/browser_credentials.py +++ b/tools/telemetry/telemetry/core/browser_credentials.py @@ -7,10 +7,8 @@ import os from telemetry.core import util -from telemetry.core.backends import codepen_credentials_backend from telemetry.core.backends import facebook_credentials_backend from telemetry.core.backends import google_credentials_backend -from telemetry.page.actions import action_runner from telemetry.unittest import options_for_unittests @@ -26,7 +24,6 @@ def __init__(self, backends = None): if backends is None: backends = [ - codepen_credentials_backend.CodePenCredentialsBackend(), facebook_credentials_backend.FacebookCredentialsBackend(), google_credentials_backend.GoogleCredentialsBackend()] @@ -58,9 +55,8 @@ def LoginNeeded(self, tab, credentials_type): 'Unrecognized credentials type: %s', credentials_type) if credentials_type not in self._credentials: return False - runner = action_runner.ActionRunner(tab) return self._backends[credentials_type].LoginNeeded( - tab, runner, self._credentials[credentials_type]) + tab, self._credentials[credentials_type]) def LoginNoLongerNeeded(self, tab, credentials_type): assert credentials_type in self._backends diff --git a/tools/telemetry/telemetry/core/browser_credentials_unittest.py b/tools/telemetry/telemetry/core/browser_credentials_unittest.py index fd79ac87972b6..7093dcba47964 100644 --- a/tools/telemetry/telemetry/core/browser_credentials_unittest.py +++ b/tools/telemetry/telemetry/core/browser_credentials_unittest.py @@ -23,7 +23,7 @@ def __init__(self, credentials_type): self.login_no_longer_needed_called = None self.credentials_type = credentials_type - def LoginNeeded(self, config, _, tab): + def LoginNeeded(self, config, tab): self.login_needed_called = (config, tab) return True