Closed Bug 787318 Opened 8 years ago Closed 7 years ago

Tabs thumbnails are not updated

Categories

(Firefox for Android :: General, defect)

17 Branch
ARM
Android
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 19
Tracking Status
firefox17 --- affected
firefox18 --- verified
firefox19 --- verified

People

(Reporter: paul.feher, Assigned: gcp)

Details

Attachments

(2 files, 2 obsolete files)

Aurora 17.0a2 2012-08-31
Samsung Galaxy Tab (Android 3.1)

Steps to reproduce:
1. Load any page (Ex http://ftp.mozilla.org/pub/mozilla.org/mobile/)
2. In the same tab load another page. 
3. Open Tab menu.

Expected results:
The tab thumbnail is updated the correct background is displayed.

Actual results:
The tab thumbnail is not updated the same background is displayed.

Note:
The issue is not reproducible on other devices: Samsung Galaxy Tab 2 7.0 (4.0.4), HTC Desire Z (Android 2.3.3), Asus Transformer TF101 (Android 4.0.3)
Works for me; same device and build, same tabs.
Attached file Log file
The issue is still reproducible using
Aurora 17.0a2 2012-09-02
Samsung Galaxy Tab (Android 3.1)

Please see the video and the attached log file.
http://youtu.be/7tbM0u4HBIo
I'm seeing this bug and similar ones a lot on tablet, but not on the phone. Taking.
Assignee: nobody → gpascutto
Typical scenario, opening reddit and letting it load, then going to the awesomebar and loading Anandtech shows this at the end of the log:

I/GeckoApp( 4607): Got message: Content:StateChange
I/GeckoApp( 4607): State - 786448
I/GeckoApp( 4607): Got a document stop
I/GeckoApp( 4607): Return 
I/GeckoApp( 4607): getAndProcessThumbnailForTab AnandTech
I/GeckoApp( 4607): handleThumbnailData AnandTech
I/GeckoApp( 4607): tab AnandTech shouldUpdateThumbnail: true
I/GeckoApp( 4607): processThumbnail AnandTech

So we seem to understand the need for updating the thumbnail, but TabsTray is still showing the old one. It did update the Tab title, though.
The problem seems to be that Android is silently failing to replace the first image set by another Bitmap with RGB_565 config. If I add an explicit conversion to ARGB_8888 it starts working. Given that this has only been observed on Honeycomb, I'll make the workaround conditional.
Either ImageView is silently failing to update more than once from a RGB_565 bitmap, or there is some bug in bitmap caching. Forcing the format to RGB_888 works around the problem. Only use it where we have seen the problem so far.
Attachment #676145 - Flags: review?(blassey.bugs)
Comment on attachment 676145 [details] [diff] [review]
Patch 1. Workaround for failing image update

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

::: mobile/android/base/Tab.java
@@ +184,5 @@
> +                        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.HONEYCOMB
> +                            && Build.VERSION.SDK_INT <= Build.VERSION_CODES.HONEYCOMB_MR2) {
> +                            // Bug 787318: required to work around a bug in ImageView
> +                            // or Bitmap caching on Honeycomb Tablets
> +                            Bitmap temp = b.copy(Bitmap.Config.ARGB_8888, false);

I think you need b.recycle() here to free the data.
Attachment #676145 - Flags: review?(blassey.bugs) → review+
>I think you need b.recycle() here to free the data

We can't do the recycle() there because the Bitmap being passed in is the per-tab-Thumbnail-Bitmap we keep alive the entire time the tab lives. We also can't easily free the copy we make because the code that actually deals with it treats it as a Drawable instead of a Bitmap.

A fix would likely be to change Tab.getThumbnail() to create a new Bitmap and recycle the old one (which I strongly suspect would make this bug disappear in the first place) - but that's a performance penalty on newer Androids that otherwise work fine.

recycle() is optional, so at worst the thumbnail pixel data will live a bit longer than it does now.
I think this is the best we can do wrt recycling, though personally I'd be happy to take the first patch and let the GC deal with it, too.
Attachment #676145 - Attachment is obsolete: true
Attachment #676184 - Flags: review?(blassey.bugs)
Should probably add a comment like this:

// If the Bitmap this drawable points to is the per-tab one we use for Gecko screenshotting, leave it alone. If it is an ARGB_8888 copy we made here, we can recycle it before replacing it.
Comment on attachment 676184 [details] [diff] [review]
Patch 1. v2 Alternate with recycling

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

Might it be better to work around this bug by always drawing the images as 8888 for HC?

Also, is it the format of the image that matters or the fact that we're reusing the same bitmap every time?
Attachment #676184 - Attachment is obsolete: true
Attachment #676184 - Flags: review?(blassey.bugs)
Attachment #676540 - Flags: review?(blassey.bugs)
>Might it be better to work around this bug by always drawing the images as 8888 
>for HC?

Not sure what you mean exactly.

>Also, is it the format of the image that matters or the fact that we're reusing 
>the same bitmap every time?

I checked closer and it's the Bitmap that we reuse and copy pixel data in (as comment 9 suspected). Anyway, this gave me an idea for an alternate patch, that I think is a lot better and cleaner.
Attachment #676540 - Flags: review?(blassey.bugs) → review+
https://hg.mozilla.org/mozilla-central/rev/b93a413268f7
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 19
Comment on attachment 676540 [details] [diff] [review]
Patch 1. v3 More alternate, just reallocate the bitmap

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Unknown. Has been there for a long time.
User impact if declined: Tab thumbnails won't update when surfing on a HC tablet.
Testing completed (on m-c, etc.): On m-c for a day.
Risk to taking this patch (and alternatives if risky): No impact outside HoneyComb. Worst thing would be a race condition, but that can't happen in theory. So looks low risk to me.
Attachment #676540 - Flags: approval-mozilla-aurora?
Comment on attachment 676540 [details] [diff] [review]
Patch 1. v3 More alternate, just reallocate the bitmap

Approving on aurora as it is a low risk fix impacting HC only
Attachment #676540 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
I cannot reproduce this issue anymore on the latest Nightly and Aurora builds. The thumbnails for each opened page are changing as expected. Closing bug as verified fixed on:

Firefox 19.0a1 (2012-11-05)
Device: Galaxy S2
OS: Android 4.0.3
Status: RESOLVED → VERIFIED
Note that the original issue was reported on a Galaxy Tab running Honeycomb and the fix is Honeycomb-specific.
(In reply to Gian-Carlo Pascutto (:gcp) from comment #21)
> Note that the original issue was reported on a Galaxy Tab running Honeycomb
> and the fix is Honeycomb-specific.

It was verified on the Galaxy Tab 10.1 (Android 3.1) too. I just wanted to be sure that's everything fine both on phones and tablets. Thank you for pointing this out
You need to log in before you can comment on or make changes to this bug.