New issue
Advanced search Search tips

Issue 590766 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Support both new and legacy google login flow allowing updating a single google recording without updating all of them

Project Member Reported by flackr@chromium.org, Feb 29 2016

Issue description

In https://chromium.googlesource.com/chromium/src/+/65af911fdf45a594f73125aec870f68d987eb7f0# I updated the gmail recording, which required updating the login method. Unfortunately, there are 15 separate recordings of gmail (https://code.google.com/p/chromium/codesearch#search/&q=mail%5C.google%5C.com%20file:%5Esrc/tools/perf/page_sets/data/.*%5C.json&sq=package:chromium&type=cs) which would also have to be updated if top_pages.GmailPage only supports the new flow. Updating only one of them caused these other suites to start failing in  issue 590735 . I think that we should support either login flow to avoid having to update so many recordings at once.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Feb 29 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/184919658089ea1a9e1f66a4799abc2905717e6a

commit 184919658089ea1a9e1f66a4799abc2905717e6a
Author: flackr <flackr@chromium.org>
Date: Mon Feb 29 19:17:43 2016

Only use new login flow for GmailSmoothPage

Updating the https://mail.google.com/mail/ recording for only top_25_smooth.json
required changing the login mechanism. This unfortunately caused tests using
the old login flow to begin failing. This is a short term patch to only use the
new flow for top_25_smooth.json until a more general solution is developed.

BUG= 590735 , 590766 

Review URL: https://codereview.chromium.org/1742363002

Cr-Commit-Position: refs/heads/master@{#378245}

[modify] https://crrev.com/184919658089ea1a9e1f66a4799abc2905717e6a/tools/perf/page_sets/top_25_smooth.py
[modify] https://crrev.com/184919658089ea1a9e1f66a4799abc2905717e6a/tools/perf/page_sets/top_pages.py

Project Member

Comment 2 by bugdroid1@chromium.org, Jun 30 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8d097cf30a2dee37de4586de97f06269ed03da09

commit 8d097cf30a2dee37de4586de97f06269ed03da09
Author: Robert Flack <flackr@chromium.org>
Date: Fri Jun 30 16:42:01 2017

Update google_login helper and gmail recordings.

Support old and new google login fields. Update the gmail recordings used for
smoothness metrics. Removes the old custom login behavior in GmailSmoothPage.

Bug:  590766 , 663155 
Change-Id: I7c6d22b48e7cb16501dc393564d7e4df097490db
Reviewed-on: https://chromium-review.googlesource.com/546615
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Commit-Queue: Robert Flack <flackr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#483732}
[modify] https://crrev.com/8d097cf30a2dee37de4586de97f06269ed03da09/tools/perf/page_sets/data/key_desktop_move_cases.json
[add] https://crrev.com/8d097cf30a2dee37de4586de97f06269ed03da09/tools/perf/page_sets/data/key_desktop_move_cases_002.wpr.sha1
[add] https://crrev.com/8d097cf30a2dee37de4586de97f06269ed03da09/tools/perf/page_sets/data/top_25_006.wpr.sha1
[modify] https://crrev.com/8d097cf30a2dee37de4586de97f06269ed03da09/tools/perf/page_sets/data/top_25_smooth.json
[modify] https://crrev.com/8d097cf30a2dee37de4586de97f06269ed03da09/tools/perf/page_sets/key_desktop_move_cases.py
[modify] https://crrev.com/8d097cf30a2dee37de4586de97f06269ed03da09/tools/perf/page_sets/login_helpers/google_login.py
[modify] https://crrev.com/8d097cf30a2dee37de4586de97f06269ed03da09/tools/perf/page_sets/top_25_smooth.py
[modify] https://crrev.com/8d097cf30a2dee37de4586de97f06269ed03da09/tools/perf/page_sets/top_pages.py

Comment 3 by flackr@chromium.org, Jun 30 2017

Status: Fixed (was: Assigned)
Supporting the legacy google login flow can be done using a different authentication mechanism and now every recording using TopPages uses the new login flow. google_login has been updated to support various iterations of the new flow and with TopPages.GmailPage re-recorded we no longer have to special case top_25_smooth.

Sign in to add a comment