Last Comment Bug 734893 - LinkTextView in the tabs tray can be slow to draw
: LinkTextView in the tabs tray can be slow to draw
Status: VERIFIED FIXED
:
Product: Firefox for Android
Classification: Client Software
Component: General (show other bugs)
: unspecified
: ARM Android
-- normal (vote)
: Firefox 14
Assigned To: Sriram Ramasubramanian [:sriram]
:
: Sebastian Kaspari (:sebastian)
Mentors:
: 735027 (view as bug list)
Depends on:
Blocks: 708266
  Show dependency treegraph
 
Reported: 2012-03-12 08:53 PDT by Aaron Train [:aaronmt]
Modified: 2012-06-11 05:49 PDT (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+


Attachments
Patch (5.66 KB, patch)
2012-03-16 15:47 PDT, Sriram Ramasubramanian [:sriram]
mark.finkle: review+
blassey.bugs: review+
Details | Diff | Splinter Review

Description User image Aaron Train [:aaronmt] 2012-03-12 08:53:11 PDT
bug 708266 added a LinkTextView to the tabs-tray. On occasion, when opening the tabs tray, I notice that the link is slow to draw (~1/1.5s). In talks on IRC, it's possible that this due to blocking DB access. This bogs down responsiveness of the complete UI showing.

--
Tested via:

Samsung Galaxy Nexus (Android 4.0.2)
Nightly (03/12)
Mozilla/5.0 (Android; Mobile; rv:13.0) Gecko/13.0 Firefox/13.0a1
Comment 1 User image Aaron Train [:aaronmt] 2012-03-12 13:56:32 PDT
*** Bug 735027 has been marked as a duplicate of this bug. ***
Comment 2 User image Paul [pwd] 2012-03-12 15:27:12 PDT
Why does this require a DB call at all?
Comment 3 User image Mark Finkle (:mfinkle) (use needinfo?) 2012-03-12 15:54:29 PDT
(In reply to Paul [sabret00the] from comment #2)
> Why does this require a DB call at all?

So we can hide the link if you don't have any data to see
Comment 4 User image Paul [pwd] 2012-03-12 16:15:09 PDT
(In reply to Mark Finkle (:mfinkle) from comment #3)
> (In reply to Paul [sabret00the] from comment #2)
> > Why does this require a DB call at all?
> 
> So we can hide the link if you don't have any data to see

If a user has sync set up, they likely have data to see. If not there's nothing wrong with a screen saying "No tabs from other devices" or less desirable would be to assume there's tabs available until the query says otherwise. Both would be preferable over the current implementation.
Comment 5 User image Aaron Train [:aaronmt] 2012-03-12 16:33:28 PDT
"No tabs from other devices", would require the same db check.
Comment 6 User image Paul [pwd] 2012-03-12 17:00:11 PDT
After the fact, once you've already pulled up the screen.
Comment 7 User image Tony Chung [:tchung] 2012-03-15 13:31:53 PDT
Is this still happening on today's build for people?  I also noticed this 3 days ago, but today's nightly build seems much more quicker to draw.  (03-15-2012, galaxy nexus)
Comment 8 User image Aaron Train [:aaronmt] 2012-03-15 13:33:00 PDT
Nominating for poor performance perception in the product.
Comment 9 User image Sriram Ramasubramanian [:sriram] 2012-03-16 15:47:30 PDT
Created attachment 606770 [details] [diff] [review]
Patch

This patch sends a simplified query to the database.
Also, the GeckoAsyncTask uses priority to do certain tasks with higher priority.
Comment 10 User image Mark Finkle (:mfinkle) (use needinfo?) 2012-03-16 18:58:23 PDT
Comment on attachment 606770 [details] [diff] [review]
Patch

Looks ok to me. Asking Brad for a quick review on the GeckoAsyncTask change.

It would be nice to get performance feedback on this patch before it lands. Hopefully Tony or Aaron can test a build on their phones.
Comment 11 User image Brad Lassey [:blassey] (use needinfo?) 2012-03-18 18:44:03 PDT
Comment on attachment 606770 [details] [diff] [review]
Patch

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

::: mobile/android/base/GeckoAsyncTask.java
@@ +53,2 @@
>      public void execute(final Params... params) {
> +        Runnable r = new Runnable() {

move this Runnable out of the function:

UITaskRunnable implements Runnable {
  public void run() {
    ...
  }
}

public void execute(final Params... params) {
Comment 12 User image Sriram Ramasubramanian [:sriram] 2012-03-19 10:55:08 PDT
http://hg.mozilla.org/integration/mozilla-inbound/rev/837da0646b7d
Comment 13 User image Marco Bonardo [::mak] 2012-03-20 03:58:29 PDT
https://hg.mozilla.org/mozilla-central/rev/837da0646b7d
Comment 14 User image Mark Finkle (:mfinkle) (use needinfo?) 2012-03-24 08:58:29 PDT
https://hg.mozilla.org/releases/mozilla-aurora/rev/86d9c18c1460
Comment 15 User image Adrian Tamas (:AdrianT) 2012-06-11 05:47:14 PDT
Links show instantly on Aurora 15.0a2 2012-06-09 and Nightly 16.0a1 2012-06-10 on the HTC Desire running Android 2.2. Marking the issue as verified.

Note You need to log in before you can comment on or make changes to this bug.