Closed
Bug 1387783
Opened 7 years ago
Closed 7 years ago
onZoomChange delazifies all browsers at once
Categories
(Firefox :: Tabbed Browser, defect)
Firefox
Tabbed Browser
Tracking
()
RESOLVED
FIXED
Firefox 57
People
(Reporter: dao, Assigned: dao)
References
Details
(Keywords: perf)
Attachments
(2 files)
4.49 KB,
patch
|
u462496
:
review+
|
Details | Diff | Splinter Review |
881 bytes,
patch
|
lizzard
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
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
Assignee | ||
Comment 1•7 years ago
|
||
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.
Component: WebExtensions: General → Tabbed Browser
Product: Toolkit → Firefox
Assignee | ||
Comment 2•7 years ago
|
||
Assignee: nobody → dao+bmo
Status: NEW → ASSIGNED
Attachment #8895037 -
Flags: review?(kevinhowjones)
Comment on attachment 8895037 [details] [diff] [review] patch Review of attachment 8895037 [details] [diff] [review]: ----------------------------------------------------------------- LGTM. Interesting syntax, thanks for cleaning everything up.
Attachment #8895037 -
Flags: review?(kevinhowjones) → review+
Pushed by dgottwald@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/9bbc4ded4bae Add lazy browser shims for fullZoom and textZoom. r=kevinjones
Comment 5•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/9bbc4ded4bae
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Assignee | ||
Comment 6•7 years ago
|
||
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]: /
Attachment #8895711 -
Flags: approval-mozilla-beta?
Updated•7 years ago
|
status-firefox55:
--- → affected
status-firefox56:
--- → affected
Comment 7•7 years ago
|
||
Comment on attachment 8895711 [details] [diff] [review] patch for uplift Fix for a perf issue with web extensions, let's uplift for beta 3.
Attachment #8895711 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 8•7 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/05d3c1eb0b02
Comment 9•7 years ago
|
||
Someone please add "perf" key word. Thanks.
You need to log in
before you can comment on or make changes to this bug.
Description
•