The default bug view has changed. See this FAQ.

nsBrowserGlue does unnecessary work when there are no new add-ons installed

RESOLVED FIXED in Firefox 14

Status

()

Firefox
General
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Unfocused, Assigned: Unfocused)

Tracking

unspecified
Firefox 14
Points:
---
Bug Flags:
in-testsuite -
in-litmus -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

In nsBrowserGlue, _onWindowsRestored checks for newly installed addons - if there are any then it opens tabs for about:newaddon. However, if there are *not* any new addons, then it still unnecessary uses the Add-ons Manager API to fetch data for an empty list of addons.
Created attachment 597593 [details] [diff] [review]
Patch v1

Think we should optimize those APIs directly too, but this code should at least not needlessly call it.

(Found after adding additional logging and timing measurements for optimizing on mobile. Yes really.)
Attachment #597593 - Flags: review?(dtownsend+bugmail)
Really? Calling AddonManager.getAddonsByIDs with an empty array is largely a no-op. If that is costing us then we have real issues :(
Attachment #597593 - Flags: review?(dtownsend+bugmail) → review+
Oh, I doubt it's costing us a lot - I just saw the function call in my logs. It's quick of course, but it's still doing a bunch of stuff for no reason.
https://hg.mozilla.org/integration/fx-team/rev/c4e3ec142aa8
Flags: in-testsuite-
Flags: in-litmus-
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 13
https://hg.mozilla.org/mozilla-central/rev/c4e3ec142aa8
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Depends on: 736542
Bug 736542 backed this out, as it broke:
https://hg.mozilla.org/integration/fx-team/rev/d23d714bc491
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Whiteboard: [fixed-in-fx-team]
since it's related, next time you land this could you also please change the module import to a lazy import? That's really useless work we do at app-startup :)
Created attachment 607890 [details] [diff] [review]
Patch v1.2

I so totally did what mak suggested 2 min before he suggested it. Really truely!

Also converted the nearby usages of defineLazyGetters into defineLazyModuleGetters while I was there, cos that's the new hotness.
Attachment #597593 - Attachment is obsolete: true
Attachment #607890 - Flags: review?(dtownsend+bugmail)
Created attachment 607896 [details] [diff] [review]
Patch v1.2.000001

This one is ever betterer! (Just a little code polish after comments from mak)
Attachment #607890 - Attachment is obsolete: true
Attachment #607896 - Flags: review?(dtownsend+bugmail)
Attachment #607890 - Flags: review?(dtownsend+bugmail)
Comment on attachment 607890 [details] [diff] [review]
Patch v1.2

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

oh well, since this doesn't touch add-ons manager code, and it's bitrotting with bug 482911, I'll r it in the hope it lands soon and we can unbitrot against it.

::: browser/components/nsBrowserGlue.js
@@ +430,1 @@
>      var changedIDs = AddonManager.getStartupChanges(AddonManager.STARTUP_CHANGE_INSTALLED);

nit: old code uses var, but newer code (see keywordURLUserSet) uses let, should probably use let.

@@ +430,4 @@
>      var changedIDs = AddonManager.getStartupChanges(AddonManager.STARTUP_CHANGE_INSTALLED);
> +    if (changedIDs.length > 0) {
> +      let win = this.getMostRecentBrowserWindow();
> +      let browser = win.gBrowser;

nit: win is used only here and once, may coalesce to oneline
Attachment #607890 - Attachment is obsolete: false
Comment on attachment 607896 [details] [diff] [review]
Patch v1.2.000001

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

yes what I meant
Attachment #607896 - Flags: review?(dtownsend+bugmail) → review+
https://hg.mozilla.org/integration/fx-team/rev/e4901b5d891a
Whiteboard: [fixed-in-fx-team]

Updated

5 years ago
Attachment #607890 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/e4901b5d891a
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: Firefox 13 → Firefox 14
You need to log in before you can comment on or make changes to this bug.