751690 - Parts of long pages stay on low-res tiles forever

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Sam Tobin-Hochstadt [:samth]]

Sometimes, if I scroll down on a large page while it's loading, I eventually reach low-resolution tiles, and they never get converted to high-res tiles, unless I reload the page.  

This happened before the switch to low-resolution tiles -- previously it was just displayed as all-black.

Unfortunately, I can't make this happen reliably, but it most often happens when all three of the following are the case:

1. I'm on a slow network (ie, 3G).
2. I scroll down quickly on a very long page.
3. I double-tap to zoom before the page finishes loading.

Comment 1

[Chris Lord [:cwiiis]]

I saw this happening the other day, but also have no solid way of reproducing... It seems to happen more often after restoring fennec from the background, but that may just be coincidence.

Comment 3

[Richard Newman [:rnewman]]

I've seen this too. Happens a lot on forum pages. Didn't get around to filing yet.

Happened to me on a Transformer tablet on wifi, so it doesn't seem to be much to do with speed.

Comment 4

[Kevin Brosnan [Ex-Mozilla]]

Browsing around http://ftp.mozilla.org seems to cause this to happen.

Comment 5

[Brad Lassey [:blassey] (use needinfo?)]

Chris can you look into this?

Comment 6

[Richard Newman [:rnewman]]

Attempted to repro, but couldn't. Dammit, Fennec, you're too fast!

Comment 7

[Chris Lord [:cwiiis]]

I'm having a hard(/impossible) time reproducing this. If anyone has reliable, or even semi-reliable STR, please list them.

Comment 8

[Paul Feher]

I've described a similar behavior in the Bug 751838 which was duplicated after this one. Maybe the STR posted there will help.

Comment 9

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

I was able to reproduce this on gmail (basic HTML site, not the mobile one) after pinch zooming. I just retested today using a nightly and I can't reproduce it anymore.

Comment 10

[Aaron Train [:aaronmt]]

You wont see this on Nightly because low res is busted: bug 752910.

Comment 11

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

(In reply to Aaron Train [:aaronmt] from comment #10)
> You wont see this on Nightly because low res is busted: bug 752910.

OK. I tested in Aurora, using 2012-05-07 and I see this issue on GMail. Use the desktop site. I can't pan around. Pinch zoom in a decent amount. Now I can pan around and when I do the page can go low-res and not go high-res.

Comment 12

[Chris Lord [:cwiiis]]

Ok, after poking at the lifecycle code for bug 752910, I've found solid STR for any page page.

1- Load a page.
2- Press the awesome-bar to bring up the awesome-screen
3- Press the home button
4- Load the browser again

You will now be in a state where the compositor will not redraw anything - you'll still see the old rendered content, retained tiles and the low-res cache, but no tiles will get updated.

This seems to be due to the stop/start events in GeckoApp, and what happens when isApplicationInBackground is true. Due to what I assume is a work-around, isApplicationInBackground only ever returns false when an activity has been launched (such as the awesome-screen).

I'll take a look at this tomorrow.

Comment 13

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Take a glance at bug 752368 and maybe try the patch there -- it could be related, though the symptoms are different (I get just a black content area when the content/compositor gets wedged, not old content).  But it could be related.

Comment 14

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Yeah, different than bug 752368; ignore that.  I can reproduce it in my build with those patches applied.  I also noticed that once it's in the state after the STR from comment #12, I can navigate to a new page and get its content rendered... but zooming in/panning/etc. doesn't update any tiles beyond that.

Comment 16

[Sriram Ramasubramanian [:sriram]]

Attachment: Patch

This puts a static variable in GeckoApplication, which knows whether the application was sent to background or not. Every activity relies on that static variable to ascertain if the application is in background or not.

Note: isApplicationSentToBackground() is not intended to be used by any activity. It will be used by GeckoApplication to see whether it is in foreground or not.

Comment 17

[Richard Newman [:rnewman]]

Comment on attachment 622171 [details] [diff] [review]
Patch

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

::: mobile/android/base/GeckoActivity.java.in
@@ +69,2 @@
>      public boolean isApplicationInBackground() {
> +        return GeckoApplication.isApplicationInBackground();

Why is this static, when the rest of this file uses ((GeckoApplication) getApplication()).foo() ?

::: mobile/android/base/GeckoApplication.java
@@ +10,5 @@
>  import android.app.Application;
>  
>  public class GeckoApplication extends Application {
>  
> +    private static boolean mInBackground = false;

Kill the `static`. Otherwise you're just going to have to fix this later (e.g., for Bug 753102).

(And isn't m* the warped-Hungarian for "member"? If this is static, it should be sInBackground.)

Comment 18

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

Comment on attachment 622171 [details] [diff] [review]
Patch

r+ but I agree about dropping the static stuff

Comment 19

[Sriram Ramasubramanian [:sriram]]

Attachment: Patch

This patch removes the "static" member.

I felt "isApplicationSentToBackground()" will still be a misnomer for anyone trying to use on the activity. Hence I used "isGeckoActivityOpened()" which reflects the actual state of the variable.

Comment 20

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

Is this patch causing the crashes you told me about on IRC?

Comment 21

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

(In reply to Mark Finkle (:mfinkle) from comment #20)
> Is this patch causing the crashes you told me about on IRC?

<sriram> i tried removing the "static" 
<sriram> but it crashes the app
<sriram> like.. WINDEATH
<sriram> with no exception

So maybe we should try to use the original patch, which uses the "static" bits for now.

Chris - Can you try that patch out when you get a chance?

Comment 22

[Chris Lord [:cwiiis]]

(In reply to Mark Finkle (:mfinkle) from comment #21)
> (In reply to Mark Finkle (:mfinkle) from comment #20)
> > Is this patch causing the crashes you told me about on IRC?
> 
> <sriram> i tried removing the "static" 
> <sriram> but it crashes the app
> <sriram> like.. WINDEATH
> <sriram> with no exception
> 
> So maybe we should try to use the original patch, which uses the "static"
> bits for now.
> 
> Chris - Can you try that patch out when you get a chance?

Patch seems to work for me, can't easily duplicate the issue in comment #12 anymore.

Comment 23

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

https://hg.mozilla.org/mozilla-central/rev/be5203774846

Comment 24

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

Comment on attachment 622205 [details] [diff] [review]
Patch

[Triage Comment]

Comment 25

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

https://hg.mozilla.org/releases/mozilla-aurora/rev/9bd737acd13f

Comment 26

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Verified on Galaxy S II; 5/11 Nightly.