737867 - Apply ViewHolder pattern on AwesomeBar listviews

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Lucas Rocha (:lucasr)]

This should give us a 4-5fps increase when scrolling awesomebar views due to the reduced number of findViewById() calls in each adapter.

Comment 1

[Lucas Rocha (:lucasr)]

Attachment: (1/3) Apply ViewHolder pattern in AwesomeBar's All/Bookmarks tabs

Comment 2

[Lucas Rocha (:lucasr)]

Attachment: (2/3) Apply ViewHolder pattern on AwesomeBar's History tab

Comment 3

[Lucas Rocha (:lucasr)]

Attachment: (3/3) Share same LayoutInflater instance in AwesomeBarTabs

Comment 4

[Lucas Rocha (:lucasr)]

Comment on attachment 607968 [details] [diff] [review]
(1/3) Apply ViewHolder pattern in AwesomeBar's All/Bookmarks tabs

I'd like to get feedback from Margaret as she did a lot of work on the bookmarks adapter.

Comment 5

[:Margaret Leibovic]

Comment on attachment 607968 [details] [diff] [review]
(1/3) Apply ViewHolder pattern in AwesomeBar's All/Bookmarks tabs

This looks good to me. I was reading up on the ViewHolder pattern, and you seem to be following what I've seen.

Although I wrote this code, I'm wondering why we don't have an @Override tag on getItemViewType, since the things I've read talked about overriding getItemViewType if you're using multiple list item types.

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 607968 [details] [diff] [review]
(1/3) Apply ViewHolder pattern in AwesomeBar's All/Bookmarks tabs

Looks OK to me too. I had to "read up" on the ViewHolder pattern too :)

Comment 7

[Lucas Rocha (:lucasr)]

Pushed:
http://hg.mozilla.org/integration/mozilla-inbound/rev/9b69a0003faf
http://hg.mozilla.org/integration/mozilla-inbound/rev/1b3fc8a81634
http://hg.mozilla.org/integration/mozilla-inbound/rev/4cf5f398d6bf

Comment 8

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/9b69a0003faf
https://hg.mozilla.org/mozilla-central/rev/1b3fc8a81634
https://hg.mozilla.org/mozilla-central/rev/4cf5f398d6bf

Comment 9

[Lucas Rocha (:lucasr)]

Comment on attachment 607970 [details] [diff] [review]
(3/3) Share same LayoutInflater instance in AwesomeBarTabs

Mobile only. Release blocker.

Comment 10

[Lucas Rocha (:lucasr)]

Comment on attachment 607968 [details] [diff] [review]
(1/3) Apply ViewHolder pattern in AwesomeBar's All/Bookmarks tabs

Mobile only. Release blocker.

Comment 11

[Lucas Rocha (:lucasr)]

Comment on attachment 607969 [details] [diff] [review]
(2/3) Apply ViewHolder pattern on AwesomeBar's History tab

Mobile only. Release blocker.

Comment 12

[Alex Keybl [:akeybl]]

Comment on attachment 607968 [details] [diff] [review]
(1/3) Apply ViewHolder pattern in AwesomeBar's All/Bookmarks tabs

[Triage Comment]
Mobile only & blocking Fennec 1.0. Approved for Aurora 13.

Comment 13

[Mark Finkle (:mfinkle) (use needinfo?)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/398778e61a9d
https://hg.mozilla.org/releases/mozilla-aurora/rev/4c2e0ab2fa4f
https://hg.mozilla.org/releases/mozilla-aurora/rev/07b68ea1083f