1686767 - Give weather extension more time to produce a result

Status: RESOLVED FIXED

(Firefox :: Address Bar, enhancement, P2)

Description

[Harry Twyford]

Now that I can measure the real-world performance of fetching location and weather data from Accuweather via Mozilla's proxy, it looks like the 400ms timeout added in bug 1679890 doesn't cut it. Since fetching weather/alert data blocks on first getting a location response, the average time to return a result to the controller is ~450ms. I've seen it go as high as 900ms for some queries. That average may get higher for slow connections or people with high pings. I plan on setting extension.timeout to 1000ms when the weather extension is installed. While this might technically cause flicker, I don't think it's a big deal seeing as the weather result always appears in the last position, so it's not pushing any other results down.

Comment 1

[Harry Twyford]

Attachment: Bug 1686767 - Unhide urlbar.extension.enabled pref and add experimental APIs to set it. r?adw!

Having browser.urlbar.extension.enabled be a hidden pref was causing issues with the new experimental APIs and their tests. Both the APIs and their tests first read the value of the pref before modifying it so it can later be restored to its default value. The default value of a hidden pref is undefined, which was causing errors. It's not a particularly sensitive pref, so I think unhiding it is preferable to finding a workaround to get the APIs/tests working with hidden prefs.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/663e0df951c7
Unhide urlbar.extension.enabled pref and add experimental APIs to set it. r=adw

Comment 3

[Narcis Beleuzu [:NarcisB]]

Backed out for bustages on browser.ini

Backout link: https://hg.mozilla.org/integration/autoland/rev/7c9496fb28fcca7af82f55492231c5997ab4789f
Log link: https://treeherder.mozilla.org/logviewer?job_id=326745522&repo=autoland&lineNumber=1281

Comment 4

[Harry Twyford]

New build: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ff8d8b04cd58f9ef17ad4b66f82cb9d9a06eaf6c

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e7217f34ea28
Unhide urlbar.extension.enabled pref and add experimental APIs to set it. r=adw

Comment 6

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/e7217f34ea28

Comment 7

[Harry Twyford]

Comment on attachment 9197152 [details]
Bug 1686767 - Unhide urlbar.extension.enabled pref and add experimental APIs to set it. r?adw!

Beta/Release Uplift Approval Request

• User impact if declined: We will be unable to run an experiment planned for 85.
• 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): Small change with tests; tightly scoped to private APIs developed to run experiments in the Urlbar.
• String changes made/needed:

Comment 8

[Julien Cristau [:jcristau] (back April 22)]

How does this block an experiment? Can't the experiment itself set the pref?

Comment 9

[Harry Twyford]

The pref is currently a hidden pref, which can't be managed by our usual extension APIs. This patch un-hides the pref. If uplift is a problem, I can write a workaround in the extension for 85 and then go back to using the standard API in 86.

Comment 10

[Julien Cristau [:jcristau] (back April 22)]

Comment on attachment 9197152 [details]
Bug 1686767 - Unhide urlbar.extension.enabled pref and add experimental APIs to set it. r?adw!

approved for 85 rc1

Comment 11

[Julien Cristau [:jcristau] (back April 22)]

https://hg.mozilla.org/releases/mozilla-release/rev/5a0663a9391a