1387783 - onZoomChange delazifies all browsers at once

Status: RESOLVED FIXED

(Firefox :: Tabbed Browser, defect)

Description

[Dão Gottwald [:dao]]

from mozillazine:

> [bug 1345098] Lazy browser prematurely inserted via 'fullZoom' property access:
> getter@chrome://browser/content/tabbrowser.xml:2293:45
> ZoomManager_getZoomForBrowser@chrome://global/content/viewZoomOverlay.js:45:1
> getZoomLevel@chrome://browser/content/ext-tabs.js:687:20
> getAPI/self.tabs.onZoomChange<@chrome://browser/content/ext-tabs.js:698:39
> addListener@resource://gre/modules/ExtensionCommon.jsm:1441:42
> addListener@resource://gre/modules/ExtensionCommon.jsm:1479:50
> addListener@resource://gre/modules/ExtensionParent.jsm:760:35
> async*receiveMessage@resource://gre/modules/ExtensionParent.jsm:606:11
> tabbrowser.xml:2293:23
> 
> Several thousands of errors like that

http://searchfox.org/mozilla-central/rev/39ccebaf1890f8731d534b5eb3818b66d2b25221/browser/components/extensions/ext-tabs.js#694-700

Comment 1

[Dão Gottwald [:dao]]

When accessing fullZoom on a lazy browser, the browser gets inserted and then fullZoom always returns 1 regardless because the browser doesn't have anything loaded yet. So we might as well add a shim returning 1 without inserting the browser.

Comment 2

[Dão Gottwald [:dao]]

Attachment: patch

Comment 3

[u462496]

Comment on attachment 8895037 [details] [diff] [review]
patch

Review of attachment 8895037 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM.  Interesting syntax, thanks for cleaning everything up.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9bbc4ded4bae
Add lazy browser shims for fullZoom and textZoom. r=kevinjones

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/9bbc4ded4bae

Comment 6

[Dão Gottwald [:dao]]

Attachment: patch for uplift

Approval Request Comment
[Feature/Bug causing the regression]: bug 906076
[User impact if declined]: bad performance with the onZoomChange webextension API
[Is this code covered by automated tests?]: not sure if there are webextension tests for this
[Has the fix been verified in Nightly?]: no
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: /
[Is the change risky?]: hardly
[Why is the change risky/not risky?]: small, isolated and straightforward fix
[String changes made/needed]: /

Comment 7

[Liz Henry (:lizzard)]

Comment on attachment 8895711 [details] [diff] [review]
patch for uplift

Fix for a perf issue with web extensions, let's uplift for beta 3.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/05d3c1eb0b02

Comment 9

[Worcester12345]

Someone please add "perf" key word. Thanks.