_handleTabSelect accesses selectedItem too often

RESOLVED FIXED in Firefox 64

Status

()

P3
normal
RESOLVED FIXED
3 months ago
3 months ago

People

(Reporter: dao, Assigned: maggus.staab)

Tracking

({perf})

Trunk
Firefox 64
Points:
---

Firefox Tracking Flags

(firefox64 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

3 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 _handleTabSelect:

https://searchfox.org/mozilla-central/rev/99cbc0aec3e1c0b65ff9052523fb5c181b248f57/browser/base/content/tabbrowser.xml#400,402

See the patch in bug 1491286 for how to fix this.
(Assignee)

Comment 1

3 months ago
I will send a patch for this.
(Assignee)

Comment 2

3 months ago
I guess I did not get everything right, but the diff is on phabricator now:

https://phabricator.services.mozilla.com/D6287
Flags: needinfo?(dao+bmo)
(Assignee)

Comment 3

3 months ago
Created attachment 9010365 [details]
Bug 1491784 - Access selectedItem only once in _handleTabSelect. r=dao
(Assignee)

Comment 4

3 months ago
Created attachment 9010368 [details]
Bug 1491784 - Access selectedItem only once in _handleTabSelect. r=dao
(Reporter)

Comment 5

3 months ago
Comment on attachment 9010365 [details]
Bug 1491784 - Access selectedItem only once in _handleTabSelect. r=dao

Dão Gottwald [::dao] has approved the revision.
Attachment #9010365 - Flags: review+
(Reporter)

Comment 6

3 months ago
Attachment 9010365 [details] looks good. You can abandon attachment 9010368 [details].

Gonna land this in a bit.

Thanks!
Assignee: nobody → maggus.staab
Flags: needinfo?(dao+bmo)
(Assignee)

Comment 7

3 months ago
sorry for the double post. I am new to hg and mixed some commands up, because I am used to git and it seems the commands are not equivalent for some parts.

Comment 8

3 months ago
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2f51b39c98c0
Access selectedItem only once in _handleTabSelect. r=dao
(Assignee)

Updated

3 months ago
Attachment #9010368 - Attachment is obsolete: true
(Assignee)

Comment 9

3 months ago
(In reply to Dão Gottwald [::dao] from comment #6)
> You can abandon attachment 9010368 [details]

just marked the attachment as obsolete. didn't found a button or similar to close the corresponding diff in phabricator
(Assignee)

Updated

3 months ago
Flags: needinfo?(dao+bmo)
(Reporter)

Comment 10

3 months ago
Yeah, no clue how to do that in phabricator. I guess you can just leave the diff open.
Flags: needinfo?(dao+bmo)

Comment 11

3 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/2f51b39c98c0
Status: NEW → RESOLVED
Last Resolved: 3 months ago
status-firefox64: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 64
You need to log in before you can comment on or make changes to this bug.