1389926 - Site tries to open external URL in new window resulting in blank page

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect, P2)

Description

[Ilgaz Öcal]

User Agent: Mozilla/5.0 (Android 5.1.1; Mobile; rv:57.0) Gecko/57.0 Firefox/57.0
Build ID: 20170813100228

Steps to reproduce:

1) navigate to https://eksisozluk.com
2) check a database entry like https://eksisozluk.com/entry/3871726
3) tap on a external URL in the entry (this one includes http://www.mozilla.org/products/firefox link)



Actual results:

A blank new tab opens


Expected results:

New tab with external URL should open

Comment 1

[Ilgaz Öcal]

I have also tested it on nightly Android version, it exists.

Comment 2

[Safwan Rahman (:safwan)]

I can reproduce this. But I can not create a minimal example of it for testing. Its working in all other places. I think its a issue with the site event listener.

Comment 3

[Oana Horvath [:ohorvath]]

The issue is reproducible on the latest Nightly 58 build 2017-10-30.
Device: Huawei P9 Lite (Android 6.0)

Comment 4

[Eugen Sawin [:esawin]]

That's a regression on Fennec 53 from

https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=a77180ae4e06fe05353d97beaa750ec65c3ce2d6&tochange=9211bfa02ed5d2807fba8253cff6eb9383ce3f5d

Ben, the interval code has gone through a lot of iterations since, do you have an idea how that change could affect this case?

Comment 5

[Ben Kelly [:bkelly, not reviewing]]

There are some sites that try to do this:

1. open a blank window
2. perform some async operation using setTimeout() (usually for tracking)
3. navigator the window opened in (1)

But, opening a new tab puts the current tab in the background.  Which means each setTimeout() in (2) gets throttled and delayed up to a second.

I just tested this on my phone.  If I navigate back to the original page then the new tab is loaded.

Also, I opened the site in desktop edge.  It noticeably pauses for 1 second before navigating the window.  It is almost certainly hitting the same background throttling there.

I believe fennec is havng a much slower experience because mobile.js specifically sets the background delay to 15 minutes:

https://searchfox.org/mozilla-central/source/mobile/android/app/mobile.js#714

This was done in bug 736602.

In general, though, its bad site design to require a background tab to performing timing operations that block navigation of a foreground tab.  There are plenty of ways to avoid this.

Comment 6

[Ben Kelly [:bkelly, not reviewing]]

Note, if I manually set dom.min_background_timeout_value back to the default of 1000 the page does load, but with some delay.

Comment 7

[Eugen Sawin [:esawin]]

Thanks, Ben.

I think the only way to help with such a case on the client side is to delay setTimeout-throttling when backgrounding the tab since reducing the timeout value is out of question.
However, I agree that this is a web-compat issue, we shouldn't have to work around this.

Comment 8

[Ben Kelly [:bkelly, not reviewing]]

(In reply to Eugen Sawin [:esawin] from comment #7)
> I think the only way to help with such a case on the client side is to delay
> setTimeout-throttling when backgrounding the tab since reducing the timeout
> value is out of question.

Yea, I explicitly don't want to do that.  And at least one other browser (edge) does not do that either.

> However, I agree that this is a web-compat issue, we shouldn't have to work
> around this.

Note, we could probably reduce our 15 minute background throttle on mobile now that we have our budget-based throttling in place.  That would at least turn this into "opens slowly" instead of "appears to never open".

Comment 9

[Eugen Sawin [:esawin]]

(In reply to Ben Kelly [:bkelly] from comment #8)
> Note, we could probably reduce our 15 minute background throttle on mobile
> now that we have our budget-based throttling in place.  That would at least
> turn this into "opens slowly" instead of "appears to never open".

How does the budget-based throttling work in detail? Wouldn't reducing the timeout to anything sensible for this case (~1s) make us vulnerable to e.g., battery-draining scripts running in background?

Comment 10

[Ben Kelly [:bkelly, not reviewing]]

Andreas can explain it better than me, but basically we enforce a budget on CPU usage in background tabs.  If a setTimeout() callback consumes a lot of time then we dynamically throttle the next callback to fire much later.  The goal is to limit background tabs to at most 1% of CPU or something.

Its currently on by default starting in FF58:

https://searchfox.org/mozilla-central/source/modules/libpref/init/all.js#1319

Comment 11

[Eugen Sawin [:esawin]]

Keeping the radio active by periodic data requests doesn't need to consume much CPU to quickly drain the battery, do (/can?) we enforce a budget on that, too?

Comment 12

[Ben Kelly [:bkelly, not reviewing]]

(In reply to Eugen Sawin [:esawin] from comment #11)
> Keeping the radio active by periodic data requests doesn't need to consume
> much CPU to quickly drain the battery, do (/can?) we enforce a budget on
> that, too?

Not that I am aware of, no.

Comment 13

[Eugen Sawin [:esawin]]

In this case, a periodic 10s network ping will probably not hit its CPU budget but could potentially keep the radio up constantly (radio control state transitions are network-configured and a 10s power-down delay would not be unrealistic for a HSPA+ network).

However, the same issue holds for the active tab, where we don't enforce any limits, so I'm not sure what a good balance to counteract setTimeout abusers would be.

Comment 14

[Ben Kelly [:bkelly, not reviewing]]

I think what you are talking about is a different feature from the background setTimeout() throttling.  For example, ss designed, we don't throttle at all if the background tab is using WebSockets.  A WebSocket will definitely keep the radio.

If you want something to block network activity on background tabs I think you want to build a different thing.

Comment 15

[Eugen Sawin [:esawin]]

Attachment: 0001-Bug-1389926-1.0-Reduce-timeout-clamp-for-background-.patch

In this case and assuming that budget-based throttling is in effect on mobile, it might be good to keep parity on the timeouts with desktop to reduce web-compat issues.

Comment 16

[Andreas Farre [:farre]]

(In reply to Eugen Sawin [:esawin] from comment #15)
> Created attachment 8924746 [details] [diff] [review]
> 0001-Bug-1389926-1.0-Reduce-timeout-clamp-for-background-.patch
> 
> In this case and assuming that budget-based throttling is in effect on
> mobile, it might be good to keep parity on the timeouts with desktop to
> reduce web-compat issues.

If this change in itself is too radical, you now have the opportunity to tweak how the background throttling of timeouts happen with regard to budget! The prefs are:

* dom.timeout.background_throttling_max_budget

This is how large the max budget is, default is 50ms. This budget is kept by every window (we usually say tab, because of this happening for background, but it really is on the window level, not the tab level). Whenever a timeout callback executes, the execution time is deducted from the budget and when the budget is negative, the next timeout cannot trigger until the budget has regenerated.

* dom.timeout.background_budget_regeneration_rate

This determines how many milliseconds it takes to regenerate one millisecond of budget. Default is 100 ms (so, for example, if a budget is overdrawn and becomes -13 ms, it takes 1.3 seconds with the default settings to have a positive budget again).

* dom.timeout.budget_throttling_max_delay

This is the maximum amount of time we'll delay a timeout due to the budget being negative, which really implies that there is a clamp on how small the budget can be as well. This is set to default to 15000 seconds, and this with a regeneration factor of 1/100 ms means that the lowest value that the internal budget of a window can have is -150ms.

The intention with the budget throttling system is mainly to punish timeouts that run "too long and too often". For example, since we, by default, regenerate 10 ms per 1000ms, it is possible to run two timeouts that take 5ms every 500ms, and not overdraw the budget. At this level we'll hit the ordinary, once-per-second, background throttling before the budget throttling. If we want to throttle harder, we can lower the regeneration to, again as an example only, to 5 ms per second (thedom.timeout.background_budget_regeneration_rate set to 200). Then if we try to run 5ms every 500ms, we'll get to the point where the most recent timeout will have resulted in the budget becoming -5ms, and since this takes 1 second to regenerate, we'll not run again until after 1 second, at which point we'll again overdraw by 5ms, and again have to wait 1 second. So as you see, if a page behaves nicely, it still gets to run quite a few timeouts in the background.

The default values are the way they are due to the fact that we wish to achieve a throttling where a window in the background only uses 1% CPU, so doing the maths with the prefs we see that:

1) We assume that the value 50ms is reasonable estimate for how long a timeout callback should be allowed to be without being considered as a "bad" callback, for example the requestIdleCallback spec uses this value for the maximum idle period length.
2) If we assume all callbacks are "good" callbacks then we know that we'll not at most have a negative budget of -50ms until we start throttling.
3) If we want to make background windows only use 1%CPU, then the regeneration rate should regenerate 50ms in 50ms/0.01 = 5000ms, which happens to be 1ms every 100ms

The same kind of calculations can of course be done to achieve different CPU utilization levels.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/24d704a58bca
[1.0] Reduce timeout clamp for background tabs to 1s. r=snorp

Comment 18

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

https://hg.mozilla.org/mozilla-central/rev/24d704a58bca