New issue
Advanced search Search tips

Issue 753482 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug-Regression



Sign in to add a comment

"Examine" tab of perf alerts occasionally reloads, losing scroll location and number of builds

Project Member Reported by charliea@chromium.org, Aug 8 2017

Issue description

Steps to reproduce: 

1) Go to the chromium.perf Sheriff-o-matic page (https://sheriff-o-matic.appspot.com/chromium.perf)
2) Click on "Examine" for a specific alert to load up all builds for a specific builder
3) Scroll down to "Show: " after the bottom of all of the builds. Click on "200" to load the 200 most recent builds.
4) Wait. (I had to wait about a minute.)

Expected behavior: nothing. The page continues to sit idle.

Actual behavior: the page reloads, showing only the 25 most recent builds instead of the 200 most recent builds. The user also loses their scroll location.

This was pretty annoying while trying to dig through to find when an actual error started happening.

 
Cc: seanmccullough@chromium.org
Labels: Milestone-UX
Owner: zhangtiff@chromium.org
Status: Assigned (was: Untriaged)
Thanks for filing a bug on this! I think we should be able to make data updates for the examine page less intrusive. 
This repro's somewhat consistently. It looks like there's a page data refresh that triggers a reload of the luci-milo iframe to e.g. https://luci-milo.appspot.com/buildbot/chromium.perf/Mac%20Air%2010.11%20Perf (no ?limit=200 or whatever).

So if you click on Show: 200, sometimes the iframe will load the next 200 results as you would expect. Then a few seconds later, it goes back to the default 25.

Sometimes the iframe'd request gets cancelled while it's still processing (probably due to the refresh) and you never see the next 200 results.

I'm guessing there's some weird interaction between the page location history, the timer for refreshing alerts data, and databinding to the iframe src url to get re-evaluated without the extra params added by the "200" click.

See https://cs.chromium.org/chromium/infra/go/src/infra/appengine/sheriff-o-matic/frontend/elements/som-examine/som-examine.html?l=78



For what it's worth, I think I'd prefer no updates happening on the "Examine" page. I often click "200", ctrl+click a bunch of specific runs to open them up, and spend minutes on those runs before returning to Sheriff-O-Matic. When doing this, it really helps to be in the same scroll location in Sheriff-O-Matic as when I left it minutes ago because I use that as a signal for the next thing to look at (usually, I continue to view even older runs).
The examine page shouldn't refresh any of the iframes, so that's almost certainly a bug.

[I'd like to follow up over email regarding your "ctrl-click + several minutes looking at runs" actions - in particular paging back and forth over consecutive runs. I think you're probably not the only user who does that, and we might be able to speed that up or make it less annoying.]
Sure! Happy to answer any questions. I'm a big believer in Sheriff-O-Matic and would love to help make the tool even better.
Project Member

Comment 6 by bugdroid1@chromium.org, Aug 30 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/infra/+/431635274b3dbaf7cdd2904a5a10f7fbeee3160f

commit 431635274b3dbaf7cdd2904a5a10f7fbeee3160f
Author: Tiff Zhang <zhangtiff@google.com>
Date: Wed Aug 30 23:23:17 2017

SoM: Disable examine page refresh for everything except annotations.

Bug:753482
Change-Id: I241eda5810a4102105afbf22f4b806bd2ff2d96f
Reviewed-on: https://chromium-review.googlesource.com/644606
Reviewed-by: Joanna Wang <jojwang@google.com>
Commit-Queue: Tiffany Zhang <zhangtiff@chromium.org>

[modify] https://crrev.com/431635274b3dbaf7cdd2904a5a10f7fbeee3160f/go/src/infra/appengine/sheriff-o-matic/frontend/elements/som-alert-view/som-alert-view.js

Status: Fixed (was: Assigned)
I forgot to mark this fixed, but I did disable refresh on the examine page a bit back. 

Sign in to add a comment