_notifyBackgroundTab accesses selectedItem too often

RESOLVED FIXED in Firefox 64

Status

()

enhancement
P3
normal
RESOLVED FIXED
9 months ago
9 months ago

People

(Reporter: dao, Assigned: maggus.staab)

Tracking

({perf})

Trunk
Firefox 64
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox64 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Reporter

Description

9 months ago
+++ 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.
Reporter

Updated

9 months ago
Priority: -- → P3
Assignee

Comment 1

9 months ago
is this a good first bug I could work on?
Reporter

Comment 2

9 months ago
Sure!
Assignee: nobody → maggus.staab
Assignee

Comment 3

9 months ago
Posted patch patch.diff (obsolete) — Splinter Review
please find my patch enclosed
Flags: needinfo?(dao+bmo)
Reporter

Comment 4

9 months ago
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)
Assignee

Comment 5

9 months ago
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.
Assignee

Comment 7

9 months ago
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)
Assignee

Updated

9 months ago
Attachment #9010353 - Attachment is obsolete: true

Comment 8

9 months ago
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/90b5ea4678b3
Access selectedItem only once in _notifyBackgroundTab. r=dao
Reporter

Comment 9

9 months ago
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+
Reporter

Updated

9 months ago
Flags: needinfo?(dao+bmo)

Comment 10

9 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/90b5ea4678b3
Status: NEW → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 64
You need to log in before you can comment on or make changes to this bug.