1575610 - Determine cause for increase in number of Nightly users seeing short tab switch spinners starting August 9th, 2019

Status: RESOLVED FIXED

(Firefox :: Tabbed Browser, defect, P2)

Description

[Mike Conley (:mconley) (:⚙️)]

Attachment: image.png

I just noticed this when browsing to my tab switch spinner dashboard to make sure it was still running.

See screenshot. It looks like the number of unique clients on Nightly seeing short (0ms-49ms) tab switch spinners has increased starting with the August 9th build.

Here are the changesets between those builds: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=8dd0375cfaf81d140c19c98f8a9dda28b9a201c1&tochange=3afb892abb74c6d281f3e66431408cbb2e16b8c4

Comment 1

[Mike Conley (:mconley) (:⚙️)]

I wonder if this is caused by bug 1523638...

Hey nika, I'm not really up to speed on what bug 1523638 does, but might it make the remoteTab getter on the frameLoader more likely to be null early in the lifetime of a new tab?

Comment 2

[Mike Conley (:mconley) (:⚙️)]

19:58:00.319 Loading tab 38(about:blank)
19:58:00.319 Tab should be blank: true
19:58:00.319 Requested tab is remote?: true
19:58:00.319 Switch to tab 15 - 38(about:blank)
19:58:00.322 DEBUG: tab switch time = 3
19:58:00.322 done 0:(-) 1:(-) 2:(-) 3:(-) 4:(-) 5:(-) 6:(-) 7:(-) 8:(-) 9:(-) 10:(-) 11:(-) 12:(-) 13:(-) 14:(-) 15:(-) 16:(-) 17:(-) 18:(-) 19:(-) 20:(-) 21:(-) 22:(-) 23:(-) 24:(-) 25:(-) 26:(-) 27:(-) 28:(-) 29:(-) 30:(-) 31:(-) 32:(-) 33:(-) 34:(-) 35:(-) 36:(-) 37:(-) 38:VLRB(AR)(+?) 39:(-) 40:(-) 41:(AR)(+) 42:(-) 43:(-) 44:(-) 45:(-) 46:(-) 47:(-) 48:(-) 49:(-) 50:(-) 51:(-) 52:(-) 53:(-) 54:(-) 55:(-) 56:(-) 57:(-) 58:(-) 59:(-) 60:(-) 61:(-) 62:(-) 63:(-) 64:(-) 65:(-) 66:(-) 67:(-) 68:(-) 69:(-) 70:(-) 71:(-) 72:(-) 73:(-) 74:(-) 75:(-) 76:(-) 77:(-) 78:(-) 79:(-) 80:(-) 81:(-) 82:(-) 83:(-) 84:(-) 85:(-) 86:(-) cached: 0
19:58:00.337 onRemotenessChange(38, true) 0:(-) 1:(-) 2:(-) 3:(-) 4:(-) 5:(-) 6:(-) 7:(-) 8:(-) 9:(-) 10:(-) 11:(-) 12:(-) 13:(-) 14:(-) 15:(-) 16:(-) 17:(-) 18:(-) 19:(-) 20:(-) 21:(-) 22:(-) 23:(-) 24:(-) 25:(-) 26:(-) 27:(-) 28:(-) 29:(-) 30:(-) 31:(-) 32:(-) 33:(-) 34:(-) 35:(-) 36:(-) 37:(-) 38:VLRB(AR)(+?) 39:(-) 40:(-) 41:(AR)(+) 42:(-) 43:(-) 44:(-) 45:(-) 46:(-) 47:(-) 48:(-) 49:(-) 50:(-) 51:(-) 52:(-) 53:(-) 54:(-) 55:(-) 56:(-) 57:(-) 58:(-) 59:(-) 60:(-) 61:(-) 62:(-) 63:(-) 64:(-) 65:(-) 66:(-) 67:(-) 68:(-) 69:(-) 70:(-) 71:(-) 72:(-) 73:(-) 74:(-) 75:(-) 76:(-) 77:(-) 78:(-) 79:(-) 80:(-) 81:(-) 82:(-) 83:(-) 84:(-) 85:(-) 86:(-) cached: 0
19:58:00.337 Tab should be blank: true
19:58:00.337 Requested tab is remote?: true
19:58:00.337 done 0:(-) 1:(-) 2:(-) 3:(-) 4:(-) 5:(-) 6:(-) 7:(-) 8:(-) 9:(-) 10:(-) 11:(-) 12:(-) 13:(-) 14:(-) 15:(-) 16:(-) 17:(-) 18:(-) 19:(-) 20:(-) 21:(-) 22:(-) 23:(-) 24:(-) 25:(-) 26:(-) 27:(-) 28:(-) 29:(-) 30:(-) 31:(-) 32:(-) 33:(-) 34:(-) 35:(-) 36:(-) 37:(-) 38:VLRB(AR)(+?) 39:(-) 40:(-) 41:(AR)(+) 42:(-) 43:(-) 44:(-) 45:(-) 46:(-) 47:(-) 48:(-) 49:(-) 50:(-) 51:(-) 52:(-) 53:(-) 54:(-) 55:(-) 56:(-) 57:(-) 58:(-) 59:(-) 60:(-) 61:(-) 62:(-) 63:(-) 64:(-) 65:(-) 66:(-) 67:(-) 68:(-) 69:(-) 70:(-) 71:(-) 72:(-) 73:(-) 74:(-) 75:(-) 76:(-) 77:(-) 78:(-) 79:(-) 80:(-) 81:(-) 82:(-) 83:(-) 84:(-) 85:(-) 86:(-) cached: 0
19:58:00.394 Tab should be blank: false
19:58:00.394 Requested tab is remote?: true
19:58:00.395 done 0:(-) 1:(-) 2:(-) 3:(-) 4:(-) 5:(-) 6:(-) 7:(-) 8:(-) 9:(-) 10:(-) 11:(-) 12:(-) 13:(-) 14:(-) 15:(-) 16:(-) 17:(-) 18:(-) 19:(-) 20:(-) 21:(-) 22:(-) 23:(-) 24:(-) 25:(-) 26:(-) 27:(-) 28:(-) 29:(-) 30:(-) 31:(-) 32:(-) 33:(-) 34:(-) 35:(-) 36:(-) 37:(-) 38:VLR(AR)(+?) 39:(-) 40:(-) 41:(AR)(+) 42:(-) 43:(-) 44:(-) 45:(-) 46:(-) 47:(-) 48:(-) 49:(-) 50:(-) 51:(-) 52:(-) 53:(-) 54:(-) 55:(-) 56:(-) 57:(-) 58:(-) 59:(-) 60:(-) 61:(-) 62:(-) 63:(-) 64:(-) 65:(-) 66:(-) 67:(-) 68:(-) 69:(-) 70:(-) 71:(-) 72:(-) 73:(-) 74:(-) 75:(-) 76:(-) 77:(-) 78:(-) 79:(-) 80:(-) 81:(-) 82:(-) 83:(-) 84:(-) 85:(-) 86:(-) cached: 0
19:58:00.457 Content Security Policy: Ignoring “'unsafe-inline'” within script-src or style-src: nonce-source or hash-source specified 2
19:58:00.460 onLayersReady(38, true) 0:(-) 1:(-) 2:(-) 3:(-) 4:(-) 5:(-) 6:(-) 7:(-) 8:(-) 9:(-) 10:(-) 11:(-) 12:(-) 13:(-) 14:(-) 15:(-) 16:(-) 17:(-) 18:(-) 19:(-) 20:(-) 21:(-) 22:(-) 23:(-) 24:(-) 25:(-) 26:(-) 27:(-) 28:(-) 29:(-) 30:(-) 31:(-) 32:(-) 33:(-) 34:(-) 35:(-) 36:(-) 37:(-) 38:VLR(AR)(+?) 39:(-) 40:(-) 41:(AR)(+) 42:(-) 43:(-) 44:(-) 45:(-) 46:(-) 47:(-) 48:(-) 49:(-) 50:(-) 51:(-) 52:(-) 53:(-) 54:(-) 55:(-) 56:(-) 57:(-) 58:(-) 59:(-) 60:(-) 61:(-) 62:(-) 63:(-) 64:(-) 65:(-) 66:(-) 67:(-) 68:(-) 69:(-) 70:(-) 71:(-) 72:(-) 73:(-) 74:(-) 75:(-) 76:(-) 77:(-) 78:(-) 79:(-) 80:(-) 81:(-) 82:(-) 83:(-) 84:(-) 85:(-) 86:(-) cached: 0
19:58:00.460 Tab should be blank: false
19:58:00.460 Requested tab is remote?: true
19:58:00.460 DEBUG: spinner time = 66

Comment 3

[Nika Layzell [:nika] (ni? for response)]

Hmm. This is interesting. I'm not an expert at the logic for the tab switcher, so I'm going to be hypothesizing a bit here.

The changes in that patch theoretically shouldn't be changing anything about the speed at which the new browser is stood up. That being said, I could totally believe something related to the timing here is changing.

The big timing change which can occur during that patch is when the initial about:blank document is created. This has previously happened lazily, as a loaded framescript would trigger creation by accessing content. Now, the initial about:blank document is created during initialization, before frame scripts are loaded and run, in order to immediately hook it up to the correct PWindowGlobal actor. (https://searchfox.org/mozilla-central/rev/34cb8d0a2a324043bcfc2c56f37b31abe7fb23a8/toolkit/components/browser/nsWebBrowser.cpp#188-190)

Based on the return value of Services.mm.getDelayedFrameScripts() on my nightly, the first 4 scripts loaded are:

chrome://global/content/browser-content.js - https://searchfox.org/mozilla-central/rev/34cb8d0a2a324043bcfc2c56f37b31abe7fb23a8/toolkit/content/browser-content.js

• Assuming loaded JSMs don't access the current mm through ambient state, wouldn't trigger creation AFAICT

resource://gre/modules/addons/Content.js - https://searchfox.org/mozilla-central/rev/34cb8d0a2a324043bcfc2c56f37b31abe7fb23a8/toolkit/mozapps/extensions/internal/Content.js

• Wouldn't trigger creation - only adds a couple of event listeners.

chrome://formautofill/content/FormAutofillFrameScript.js - https://searchfox.org/mozilla-central/rev/34cb8d0a2a324043bcfc2c56f37b31abe7fb23a8/browser/extensions/formautofill/content/FormAutofillFrameScript.js

• Adds message & event listeners - wouldn't trigger creation

resource://gre/modules/WebNavigationContent.js - https://searchfox.org/mozilla-central/rev/34cb8d0a2a324043bcfc2c56f37b31abe7fb23a8/toolkit/components/extensions/WebNavigationContent.js

• Triggers creation when initializing the WebProgressListener object
  • https://searchfox.org/mozilla-central/rev/34cb8d0a2a324043bcfc2c56f37b31abe7fb23a8/toolkit/components/extensions/WebNavigationContent.js#146
  • specifically when creating the JS wrapper for the returned outer window object: https://searchfox.org/mozilla-central/rev/34cb8d0a2a324043bcfc2c56f37b31abe7fb23a8/dom/base/nsGlobalWindowOuter.h#230

I've been digging around to see if I can figure out what in particular used to be run before and now is run after which could cause the event to not fire as early as it used to in content, but don't have a great answer yet. I think the event which is being dropped or delayed is "MozLayerTreeReady", as dispatched from here: https://searchfox.org/mozilla-central/rev/325c1a707819602feff736f129cb36055ba6d94f/dom/ipc/BrowserParent.cpp#3398.

Comment 4

[Nika Layzell [:nika] (ni? for response)]

It looks like another change would be when rendering is initialized. It was previously initialized before the initial about:blank is created, and it is now created before it. This could potentially cause an event requesting a paint of that about:blank document to be dropped.

Initializing Rendering: https://searchfox.org/mozilla-central/rev/34cb8d0a2a324043bcfc2c56f37b31abe7fb23a8/dom/ipc/BrowserChild.cpp#1222

Comment 5

[Alex Thayer [:alexical] (she/her)]

Hmm - I think this does qualify as an [fxperf:p1]. Mike or Nika, if you feel like you have enough cycles and are already deep enough in this feel free to take this over, but I'm assuming not, so I'm assigning to myself just to ensure we get to the bottom of it.

Comment 6

[Alex Thayer [:alexical] (she/her)]

Mike, can you clarify a few things for me?

(In reply to Mike Conley (:mconley) (:⚙️) from comment #1)

but might it make the remoteTab getter on the frameLoader more likely to be null early in the lifetime of a new tab?

Could you clarify what your theory is here on the remoteTab getter being null? Should I discard this idea - seems not present in your log you posted afterwards.

(In reply to Mike Conley (:mconley) (:⚙️) from comment #2)

19:58:00.319 Loading tab 38(about:blank)
19:58:00.319 Tab should be blank: true
19:58:00.319 Requested tab is remote?: true
19:58:00.319 Switch to tab 15 - 38(about:blank)
19:58:00.322 DEBUG: tab switch time = 3
19:58:00.322 done 0:(-) 1:(-) 2:(-) 3:(-) 4:(-) 5:(-) 6:(-) 7:(-) 8:(-) 9:(-) 10:(-) 11:(-) 12:(-) 13:(-) 14:(-) 15:(-) 16:(-) 17:(-) 18:(-) 19:(-) 20:(-) 21:(-) 22:(-) 23:(-) 24:(-) 25:(-) 26:(-) 27:(-) 28:(-) 29:(-) 30:(-) 31:(-) 32:(-) 33:(-) 34:(-) 35:(-) 36:(-) 37:(-) 38:VLRB(AR)(+?) 39:(-) 40:(-) 41:(AR)(+) 42:(-) 43:(-) 44:(-) 45:(-) 46:(-) 47:(-) 48:(-) 49:(-) 50:(-) 51:(-) 52:(-) 53:(-) 54:(-) 55:(-) 56:(-) 57:(-) 58:(-) 59:(-) 60:(-) 61:(-) 62:(-) 63:(-) 64:(-) 65:(-) 66:(-) 67:(-) 68:(-) 69:(-) 70:(-) 71:(-) 72:(-) 73:(-) 74:(-) 75:(-) 76:(-) 77:(-) 78:(-) 79:(-) 80:(-) 81:(-) 82:(-) 83:(-) 84:(-) 85:(-) 86:(-) cached: 0
19:58:00.337 onRemotenessChange(38, true) 0:(-) 1:(-) 2:(-) 3:(-) 4:(-) 5:(-) 6:(-) 7:(-) 8:(-) 9:(-) 10:(-) 11:(-) 12:(-) 13:(-) 14:(-) 15:(-) 16:(-) 17:(-) 18:(-) 19:(-) 20:(-) 21:(-) 22:(-) 23:(-) 24:(-) 25:(-) 26:(-) 27:(-) 28:(-) 29:(-) 30:(-) 31:(-) 32:(-) 33:(-) 34:(-) 35:(-) 36:(-) 37:(-) 38:VLRB(AR)(+?) 39:(-) 40:(-) 41:(AR)(+) 42:(-) 43:(-) 44:(-) 45:(-) 46:(-) 47:(-) 48:(-) 49:(-) 50:(-) 51:(-) 52:(-) 53:(-) 54:(-) 55:(-) 56:(-) 57:(-) 58:(-) 59:(-) 60:(-) 61:(-) 62:(-) 63:(-) 64:(-) 65:(-) 66:(-) 67:(-) 68:(-) 69:(-) 70:(-) 71:(-) 72:(-) 73:(-) 74:(-) 75:(-) 76:(-) 77:(-) 78:(-) 79:(-) 80:(-) 81:(-) 82:(-) 83:(-) 84:(-) 85:(-) 86:(-) cached: 0
19:58:00.337 Tab should be blank: true
19:58:00.337 Requested tab is remote?: true
19:58:00.337 done 0:(-) 1:(-) 2:(-) 3:(-) 4:(-) 5:(-) 6:(-) 7:(-) 8:(-) 9:(-) 10:(-) 11:(-) 12:(-) 13:(-) 14:(-) 15:(-) 16:(-) 17:(-) 18:(-) 19:(-) 20:(-) 21:(-) 22:(-) 23:(-) 24:(-) 25:(-) 26:(-) 27:(-) 28:(-) 29:(-) 30:(-) 31:(-) 32:(-) 33:(-) 34:(-) 35:(-) 36:(-) 37:(-) 38:VLRB(AR)(+?) 39:(-) 40:(-) 41:(AR)(+) 42:(-) 43:(-) 44:(-) 45:(-) 46:(-) 47:(-) 48:(-) 49:(-) 50:(-) 51:(-) 52:(-) 53:(-) 54:(-) 55:(-) 56:(-) 57:(-) 58:(-) 59:(-) 60:(-) 61:(-) 62:(-) 63:(-) 64:(-) 65:(-) 66:(-) 67:(-) 68:(-) 69:(-) 70:(-) 71:(-) 72:(-) 73:(-) 74:(-) 75:(-) 76:(-) 77:(-) 78:(-) 79:(-) 80:(-) 81:(-) 82:(-) 83:(-) 84:(-) 85:(-) 86:(-) cached: 0
19:58:00.394 Tab should be blank: false
19:58:00.394 Requested tab is remote?: true
19:58:00.395 done 0:(-) 1:(-) 2:(-) 3:(-) 4:(-) 5:(-) 6:(-) 7:(-) 8:(-) 9:(-) 10:(-) 11:(-) 12:(-) 13:(-) 14:(-) 15:(-) 16:(-) 17:(-) 18:(-) 19:(-) 20:(-) 21:(-) 22:(-) 23:(-) 24:(-) 25:(-) 26:(-) 27:(-) 28:(-) 29:(-) 30:(-) 31:(-) 32:(-) 33:(-) 34:(-) 35:(-) 36:(-) 37:(-) 38:VLR(AR)(+?) 39:(-) 40:(-) 41:(AR)(+) 42:(-) 43:(-) 44:(-) 45:(-) 46:(-) 47:(-) 48:(-) 49:(-) 50:(-) 51:(-) 52:(-) 53:(-) 54:(-) 55:(-) 56:(-) 57:(-) 58:(-) 59:(-) 60:(-) 61:(-) 62:(-) 63:(-) 64:(-) 65:(-) 66:(-) 67:(-) 68:(-) 69:(-) 70:(-) 71:(-) 72:(-) 73:(-) 74:(-) 75:(-) 76:(-) 77:(-) 78:(-) 79:(-) 80:(-) 81:(-) 82:(-) 83:(-) 84:(-) 85:(-) 86:(-) cached: 0
19:58:00.457 Content Security Policy: Ignoring “'unsafe-inline'” within script-src or style-src: nonce-source or hash-source specified 2
19:58:00.460 onLayersReady(38, true) 0:(-) 1:(-) 2:(-) 3:(-) 4:(-) 5:(-) 6:(-) 7:(-) 8:(-) 9:(-) 10:(-) 11:(-) 12:(-) 13:(-) 14:(-) 15:(-) 16:(-) 17:(-) 18:(-) 19:(-) 20:(-) 21:(-) 22:(-) 23:(-) 24:(-) 25:(-) 26:(-) 27:(-) 28:(-) 29:(-) 30:(-) 31:(-) 32:(-) 33:(-) 34:(-) 35:(-) 36:(-) 37:(-) 38:VLR(AR)(+?) 39:(-) 40:(-) 41:(AR)(+) 42:(-) 43:(-) 44:(-) 45:(-) 46:(-) 47:(-) 48:(-) 49:(-) 50:(-) 51:(-) 52:(-) 53:(-) 54:(-) 55:(-) 56:(-) 57:(-) 58:(-) 59:(-) 60:(-) 61:(-) 62:(-) 63:(-) 64:(-) 65:(-) 66:(-) 67:(-) 68:(-) 69:(-) 70:(-) 71:(-) 72:(-) 73:(-) 74:(-) 75:(-) 76:(-) 77:(-) 78:(-) 79:(-) 80:(-) 81:(-) 82:(-) 83:(-) 84:(-) 85:(-) 86:(-) cached: 0
19:58:00.460 Tab should be blank: false
19:58:00.460 Requested tab is remote?: true
19:58:00.460 DEBUG: spinner time = 66

How reliably are you able to reproduce this? This does seem to be a buggy flow. I'm particularly interested in explaining the "done" log at 19:58:00.395. This means we're calling postActions, and I'm trying to figure out what event could be getting us here, as there's only a few which don't log themselves and I'm not sure any fit the bill. This event is what switches us from shouldBeBlank to !shouldBeBlank, which means we show a spinner.

In general, I think anything which causes us to show a spinner before the 400ms TAB_SWITCH_TIMEOUT strikes me as a bug with the AsyncTabSwitcher logic, and maybe we should consider making that mechanism more robust. (Although its fragility does allow us to catch changes like this, maybe we should just be tracking more detailed timings here in our telemetry.

Comment 7

[Alex Thayer [:alexical] (she/her)]

Attachment: Bug 1575610 - Revamp AsyncTabSwitcher.jsm logging r?mconley

This just adds a bit of information to the AsyncTabSwitcher's
logging and cleans up the display to make it quicker to find
what changed, especially with large numbers of tabs. The bit of
new information that I'm particularly interested in is what
event triggered a particular update - so now every time we call
postActions, we include the name of the event.

Comment 8

[Mike Conley (:mconley) (:⚙️)]

(In reply to Doug Thayer [:dthayer] from comment #6)

Could you clarify what your theory is here on the remoteTab getter being null? Should I discard this idea - seems not present in your log you posted afterwards.

Yeah, I think we can discard it as a theory. It was mainly speculation based on the commits that we suspected caused the issue, but you're right, the logs don't support it.

What I observed when I do observe this, is that in updateDisplay, we evaluate hasSufficientlyLoaded here: https://searchfox.org/mozilla-central/rev/e5327b05c822cdac24e233afa37d72c0552dbbaf/browser/modules/AsyncTabSwitcher.jsm#375-377

and evaluate it to be true sometimes during the restoration of a discarded tab. That check itself is pretty suspect - checking the "busy" state of a xul:tab probably isn't the greatest indicator for whether we've loaded enough of the page yet.

I wonder if somehow we've increased the likelihood that the tab won't have started the network request yet and entered the "busy" state, and then decide that the tab shouldn't be blank...

How reliably are you able to reproduce this? This does seem to be a buggy flow. I'm particularly interested in explaining the "done" log at 19:58:00.395. This means we're calling postActions, and I'm trying to figure out what event could be getting us here, as there's only a few which don't log themselves and I'm not sure any fit the bill. This event is what switches us from shouldBeBlank to !shouldBeBlank, which means we show a spinner.

So my STR may or may not explain the spike in the graph, but effectively, what I do is:

0. Install Spindoctor in the profile to make it easier to detect short spinners
1. Load a bunch of tabs at a bunch of different sites
2. Open about:preferences in a new tab, and change the tracking protection from Standard to Strict (or vise-versa). Doing so will reveal a button, "Reload All Tabs", which discards all of the pre-existing tabs.
3. Then I start selecting tabs somewhat rapidly with the mouse. I don't know if the "rapidly" part is necessary. If Spindoctor doesn't detect a spinner, I repeat from step 2 again.

In general, I think anything which causes us to show a spinner before the 400ms TAB_SWITCH_TIMEOUT strikes me as a bug with the AsyncTabSwitcher logic, and maybe we should consider making that mechanism more robust. (Although its fragility does allow us to catch changes like this, maybe we should just be tracking more detailed timings here in our telemetry.

More robust: AsyncTabSwitcher desperately needs that, yes. I agree that spinners that occur anytime before TAB_SWITCH_TIMEOUT are a bug.

Comment 9

[Alex Thayer [:alexical] (she/her)]

A mozregression run seems to confirm that mconley's STR do indeed reproduce the problem introduced on August 9th. What's killing us seems to be a MozAfterPaint notification, which we don't do anything with except sometimes notice that our about:blank tab has become a <some URL> tab, which causes shouldBeBlank to turn to false, and the async tab switcher to decide that we need to show a spinner.

I think Nika's theories for this seem reasonable enough to me, and I don't see any reason why we shouldn't here just go forward with making our "should we show a spinner?" logic less flaky. Because we really shouldn't be affected by whether we've received a MozLayerTreeReady event for a blank document or not.

Comment 10

[Alex Thayer [:alexical] (she/her)]

Attachment: Bug 1575610 - Ensure we only show spinner after 400ms r?mconley

I haven't been able to work out a reason why we should show the
spinner before this.loadTimer is cleared. All this does is allow
for random reordering of events to sometimes show a spinner early.
This should ideally just make the spinner logic more rubust to
event ordering changes, without sacrificing visibility into tab
switch timings.

Depends on D44710

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f15a0a9b8abd
Revamp AsyncTabSwitcher.jsm logging r=mconley
https://hg.mozilla.org/integration/autoland/rev/f1a82e55bc80
Ensure we only show spinner after 400ms r=mconley

Comment 12

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/f15a0a9b8abd
https://hg.mozilla.org/mozilla-central/rev/f1a82e55bc80

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Was this a Nightly-only issue or is Beta70 still affected?

Comment 14

[Mike Conley (:mconley) (:⚙️)]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #13)

Was this a Nightly-only issue or is Beta70 still affected?

I suspect Beta is affected. I think this is worth uplifting (at least attachment 9090795 [details]).

dthayer is on PTO this week, but I'll make the uplift request.

Comment 15

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 9090795 [details]
Bug 1575610 - Ensure we only show spinner after 400ms r?mconley

Beta/Release Uplift Approval Request

• User impact if declined: Users might experience more short (<50ms) tab switch spinners in various circumstances - particularly when restoring discarded tabs.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): While the AsyncTabSwitcher is normally pretty finicky, the patch here in question is really simple and straight-forward.
• String changes made/needed: None.

Comment 16

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 9090795 [details]
Bug 1575610 - Ensure we only show spinner after 400ms r?mconley

Improvement for a perf regression in 70, we're still in early beta, let's take this and see how it does.

Comment 17

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Liz, is the other patch in this bug also supposed to get uplifted?

Comment 18

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/releases/mozilla-beta/rev/18d2a997708a

Comment 19

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/releases/mozilla-beta/rev/6e1216ffc35a

Comment 20

[Florin Strugariu [:Bebe]]

== Change summary for alert #23088 (as of Tue, 17 Sep 2019 04:50:12 GMT) ==

Improvements:

7% Images linux64-stylo-sequential opt 4,648,714.53 -> 4,340,953.83
5% Images macosx1014-64-shippable opt 4,576,161.78 -> 4,345,541.65
5% Images windows10-64-shippable opt 5,534,805.71 -> 5,284,882.31
4% Images linux64-qr opt 5,934,921.89 -> 5,706,045.65
4% Images windows10-64-qr opt 8,300,584.07 -> 7,998,869.81
3% Images windows10-64-shippable-qr opt 8,283,544.02 -> 8,059,352.83
3% Images windows10-64-shippable-qr opt 8,276,536.62 -> 8,060,724.17
2% Images linux64-shippable opt stylo tp6 7,478,752.01 -> 7,312,223.70
2% Images linux64 opt stylo tp6 7,480,393.72 -> 7,321,844.08

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=23088