From bug 1379480 comment 7, we should not run the animation for stop->reload if the page has finished loading within 150ms. This way we won't show the reload->stop animation and then immediately the stop->reload animation.

Hey Patrick, for fixing this bug I'm looking at taking the aRequest object that we get in onStateChange, and running QueryInterface on it to convert it to a nsITimedChannel. With that, I get access to the following properties that describe how long the request took to load: http://searchfox.org/mozilla-central/rev/8a61c71153a79cda2e1ae7d477564347c607cc5f/netwerk/base/nsITimedChannel.idl#77-96 However, the values that I get from `responseEndTime - responseStartTime` doesn't seem to match up with what I expect. It's always a lot lower, around 100ms, for something that visually appears to take around 250ms. Doing `responseEndTime - channelCreationTime` doesn't lead to better results. Should I be doing this calculation differently? Perhaps I need to sum up the differences for each timing pair? I see that http://searchfox.org/mozilla-central/rev/8a61c71153a79cda2e1ae7d477564347c607cc5f/devtools/shared/webconsole/network-monitor.js#1484-1491 calculates the totalTime by getting the sum of the pairs, though that calculation uses values from nsITimedChannel plus values from some httpActivity.timings object. Is this achievable using only the nsITimedChannel?

I'm going to shift your ni to valentin, as he's the timedChannel expert. But I suspect a huge part of what you're seeing is that timedChannel mostly measures network time (as its supposed to), but there are a whole lot of other things that have to happen before your code gets a hold of the channel. If reproducible it might be useful quantum flow fodder.

oh, and a timedChannel corresponds to a single URL.. its not a page concept.

Ideally what I would like is the ability to get `window.performance.timing.responseEnd - window.performance.timing.navigationStart` Is this not achievable with either a web progress listener or a request channel?

We know exactly when we switch from reload to stop. Seem like we should keep that timestamp and compare it when switching from stop to reload? Why would you want to use nsITimedChannel for that?

(In reply to Dão Gottwald [::dao] from comment #5) > We know exactly when we switch from reload to stop. Seem like we should keep > that timestamp and compare it when switching from stop to reload? Why would > you want to use nsITimedChannel for that? Because the same stop and reload functions are used for all tabs, so if someone switched tabs then the numbers wouldn't coordinate.

I suppose I could store those values on the tab itself, though we would need to make sure it doesn't get broken by tearing off tabs.

That sounds overly complicated. We could just drop the timestamp when switching tabs.

Comment on attachment 8890928 [details] Bug 1384180 - Don't animate the stop->reload if the page loads within 150ms (too distracting). https://reviewboard.mozilla.org/r/162132/#review167408 ::: browser/base/content/browser.js:5053 (Diff revision 2) > return; > > + // Store the time that we switched to the stop button only if a request > + // is active. Requests are null if the switch is related to a tabswitch. > + // This is used to determine if we should show the stop->reload animation. > + this.timeWhenSwitchedToStop = aRequest instanceof Ci.nsIRequest ? Why aRequest instanceof Ci.nsIRequest rather than just a null check? ::: browser/base/content/browser.js:5053 (Diff revision 2) > return; > > + // Store the time that we switched to the stop button only if a request > + // is active. Requests are null if the switch is related to a tabswitch. > + // This is used to determine if we should show the stop->reload animation. > + this.timeWhenSwitchedToStop = aRequest instanceof Ci.nsIRequest ? I think it would be safer if you called something like CombinedStopReload.onTabSelect() from XULBrowserWindow.onUpdateCurrentBrowser, because switching tabs won't necessarily call switchToStop. ::: browser/base/content/browser.js:5085 (Diff revision 2) > + // If we don't know when we switched to stop (a tabswitch occured while > + // the page was loading), then we will not prevent the animation from > + // occuring. > + let loadTimeExceedsMinimumForAnimation = this.timeWhenSwitchedToStop ? > + window.performance.now() - this.timeWhenSwitchedToStop > 150 : > + true; !this.timeWhenSwitchedToStop || window.performance.now() - this.timeWhenSwitchedToStop > 150;

Comment on attachment 8890928 [details] Bug 1384180 - Don't animate the stop->reload if the page loads within 150ms (too distracting). https://reviewboard.mozilla.org/r/162132/#review167422 ::: browser/base/content/browser.js:4874 (Diff revision 3) > > // simulate all change notifications after switching tabs > onUpdateCurrentBrowser: function XWB_onUpdateCurrentBrowser(aStateFlags, aStatus, aMessage, aTotalProgress) { > if (FullZoom.updateBackgroundTabs) > FullZoom.onLocationChange(gBrowser.currentURI, true); > + CombinedStopReload.onTabSwitch(); nit: add a blank line before and after this ::: browser/base/content/browser.js:5092 (Diff revision 3) > + // If we don't know when we switched to stop (a tabswitch occured while > + // the page was loading), then we will not prevent the animation from > + // occuring. > + let loadTimeExceedsMinimumForAnimation = > + !this.timeWhenSwitchedToStop || > + window.performance.now() - this.timeWhenSwitchedToStop > 150; Running this (especially calling performance.now()) when we don't need it because of the other conditions is wasteful. Can you either inline this in the expression or make loadTimeExceedsMinimumForAnimation a function rather than a boolean value? ::: browser/base/content/browser.js:5095 (Diff revision 3) > + let loadTimeExceedsMinimumForAnimation = > + !this.timeWhenSwitchedToStop || > + window.performance.now() - this.timeWhenSwitchedToStop > 150; > + > let shouldAnimate = AppConstants.MOZ_PHOTON_ANIMATIONS && > aRequest instanceof Ci.nsIRequest && nit: replace this with a null-check as well for consistency?

Comment on attachment 8890928 [details] Bug 1384180 - Don't animate the stop->reload if the page loads within 150ms (too distracting). https://reviewboard.mozilla.org/r/162132/#review169282

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again. hg error in cmd: hg rebase -s e6a3ee892b6a -d 7c6f894e8a3e: rebasing 411030:e6a3ee892b6a "Bug 1384180 - Don't animate the stop->reload if the page loads within 150ms (too distracting). r=dao" (tip) merging browser/base/content/browser.js warning: conflicts while merging browser/base/content/browser.js! (edit, then use 'hg resolve --mark') unresolved conflicts (see hg resolve, then hg rebase --continue)

Pushed by jwein@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/714713b0c49e Don't animate the stop->reload if the page loads within 150ms (too distracting). r=dao

Backed out as requested by jaws because bug 1379620 got backed out: https://hg.mozilla.org/integration/autoland/rev/cdf6256ae8c70325bee62b19c88f49a225f87c02

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again. hg error in cmd: hg rebase -s 7f3d3f23d67e -d 742dfaf7631d: rebasing 411366:7f3d3f23d67e "Bug 1384180 - Don't animate the stop->reload if the page loads within 150ms (too distracting). r=dao" (tip) merging browser/base/content/browser.js warning: conflicts while merging browser/base/content/browser.js! (edit, then use 'hg resolve --mark') unresolved conflicts (see hg resolve, then hg rebase --continue)

Pushed by jwein@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/63f67675955d Don't animate the stop->reload if the page loads within 150ms (too distracting). r=dao

I see some improvements: == Change summary for alert #8817 (as of August 03 2017 14:26 UTC) == Improvements: 3% tp5o % Processor Time windows7-32 pgo e10s 56.70 -> 55.20 For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=8817

Build ID 20170908220146 User Agent Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:57.0) Gecko/20100101 Firefox/57.0 This bug is Verified with latest Nightly 57.0a1 ! There is no animation if the page loads within 150ms.