Closed Bug 1491786 Opened 2 years ago Closed 2 years ago

_notifyBackgroundTab accesses selectedItem too often

Categories

(Firefox :: Tabbed Browser, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
Firefox 64
Tracking Status
firefox64 --- fixed

People

(Reporter: dao, Assigned: maggus.staab)

References

Details

(Keywords: perf)

Attachments

(1 file, 1 obsolete file)

+++ This bug was initially created as a clone of Bug #1491286 +++

The selectedItem getter is kind of expensive, we should only access it once in _notifyBackgroundTab:

https://searchfox.org/mozilla-central/rev/99cbc0aec3e1c0b65ff9052523fb5c181b248f57/browser/base/content/tabbrowser.xml#766-767

See the patch in bug 1491286 for how to fix this.
Priority: -- → P3
is this a good first bug I could work on?
Sure!
Assignee: nobody → maggus.staab
Attached patch patch.diff (obsolete) — Splinter Review
please find my patch enclosed
Flags: needinfo?(dao+bmo)
Comment on attachment 9010353 [details] [diff] [review]
patch.diff

Hm, this looks good at a first glance, but when I try to apply the patch, I get:

patch unexpectedly ends in middle of line
patch: **** Only garbage was found in the patch input.

Did you manually edit the patch file?
Flags: needinfo?(dao+bmo)
I did not manually fiddle with it. But since I got arcanist running I could try to push the patch to phabricator instead.

I will try that.
got a phabricator diff up, after a lot or research.

thx to the cool people in the #phabricator irc channel (especially imadueme and mcote)
Flags: needinfo?(dao+bmo)
Attachment #9010353 - Attachment is obsolete: true
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/90b5ea4678b3
Access selectedItem only once in _notifyBackgroundTab. r=dao
Comment on attachment 9010427 [details]
Bug 1491786 - Access selectedItem only once in _notifyBackgroundTab. r=dao

Dão Gottwald [::dao] has approved the revision.
Attachment #9010427 - Flags: review+
Flags: needinfo?(dao+bmo)
https://hg.mozilla.org/mozilla-central/rev/90b5ea4678b3
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 64
You need to log in before you can comment on or make changes to this bug.