Turns out docshell already has a boolean flag indicating whether it is inside a pinned tab: isAppTab: https://searchfox.org/mozilla-central/rev/238406d4c1b3f147522ce0a45a4c6f84a8115781/docshell/base/nsIDocShell.idl#682

The feature currently doesn't have any tests so I only tested this manually. I decided to query the attribute manually because the queries should be all relatively cheap and infrequent, and that way we can handle tabs going in and out of pinned mode for free.

Why do we want to do this? Do we expect pinned tabs to use significant amount processing time? I'm worried that we end up throttling less than what we do currently.

With this patch we'd only skip budget throttling, not all throttling.

Comment on attachment 8884476 [details] [diff] [review] Exempt pinned tabs from budget based background timeout throttling Review of attachment 8884476 [details] [diff] [review]: ----------------------------------------------------------------- Seems reasonable assuming docshell "AppTab" is indeed pinned tab. I did not double-check that. To answer Olli's question perhaps we should make this a pref we can then run an experiment on. We also really need tests for this and the other budget throttling work. ::: dom/base/TimeoutManager.cpp @@ +1235,5 @@ > > +bool > +TimeoutManager::BudgetThrottlingEnabled() const > +{ > + if (mBudgetThrottleTimeouts) { Can you avoid the extra nesting by using short-circuit logic like: if (!mBudgetThrottleTimeouts) { return false; } nsIDocShell* docShell = mWindow.GetDocShell(); if (!docShell) { return true; } // etc

(In reply to Ben Kelly [reviewing, but slowly][:bkelly] from comment #6) > Comment on attachment 8884476 [details] [diff] [review] > Exempt pinned tabs from budget based background timeout throttling > > Review of attachment 8884476 [details] [diff] [review]: > ----------------------------------------------------------------- > > To answer Olli's question perhaps we should make this a pref we can then run > an experiment on. We could include this in the manual testing as well. > > We also really need tests for this and the other budget throttling work. Yep, we have Bug 1378402 to track adding tests.

So, I tested Chrome 59's treatment of background pinned tabs and they do not exempt those tabs from budget based throttling. I was under the impression that this bug is filed for compatibility with what Chrome does, but having realized that fixing this will actually make us incompatible with them, I no longer think this is a good idea. Sorry for not testing before. I'm gonna WONTFIX because of that reason. If someone wants to reopen they should probably start to first convince the Chrome team that they should do the same. I certainly don't think we should be implementing different heuristics here, especially if what they have has turned out Web compatible for them.