Closed Bug 699786 Opened 14 years ago Closed 14 years ago

Cancel pending favicon requests for tabs that changed location

Categories

(Firefox for Android Graveyard :: General, defect, P3)

Product:
Firefox for Android Graveyard
Component:
General
Platform:
All
Android
Type:
defect

Tracking

(firefox11 fixed, fennec11+)

Status:
RESOLVED FIXED
Tracking Flags:
Tracking Status
firefox11 --- fixed
fennec 11+ ---

People

(Reporter: lucasr, Assigned: lucasr)

References

Details

Attachments

(3 files)

(1/3) Use Android's AsyncTask in Favicons
4.16 KB, patch
blassey
: review+
Details | Diff | Splinter Review
(2/3) Add API and infra to support favicon load cancellation
6.76 KB, patch
blassey
: review+
Details | Diff | Splinter Review
(3/3) Cancel pending favicon loads when location changes
4.87 KB, patch
blassey
: review+
Details | Diff | Splinter Review
If favicon is requested but the corresponding tab changes location, we should simply cancel the pending request as we're not interested in its results anymore.
OS: Linux → Android
Hardware: x86 → All
Assignee: nobody → lucasr.at.mozilla
Priority: -- → P3
Attachment #575150 - Flags: review?(blassey.bugs)
Attachment #575150 - Flags: review?(blassey.bugs) → review+
Comment on attachment 575151 [details] [diff] [review] (2/3) Add API and infra to support favicon load cancellation Review of attachment 575151 [details] [diff] [review]: ----------------------------------------------------------------- Nit, make mNextFaviconLoadId a static member of the LoadFaviconTask class
Attachment #575151 - Flags: review?(blassey.bugs) → review+
(In reply to Brad Lassey [:blassey] from comment #4) > Comment on attachment 575151 [details] [diff] [review] [diff] [details] [review] > (2/3) Add API and infra to support favicon load cancellation > > Review of attachment 575151 [details] [diff] [review] [diff] [details] [review]: > ----------------------------------------------------------------- > > Nit, make mNextFaviconLoadId a static member of the LoadFaviconTask class Tried that but inner classes can't have static members...
Comment on attachment 575152 [details] [diff] [review] (3/3) Cancel pending favicon loads when location changes Review of attachment 575152 [details] [diff] [review]: ----------------------------------------------------------------- ::: embedding/android/GeckoApp.java @@ +589,5 @@ > tab.removeTransientDoorHangers(); > > + // Cancel any pending favicon load task > + long faviconLoadId = tab.getFaviconLoadId(); > + if (faviconLoadId > 0) should this be:? if (faviconLoadId != Favicons.NOT_LOADING)
Attachment #575152 - Flags: review?(blassey.bugs) → review+
(In reply to Brad Lassey [:blassey] from comment #6) > Comment on attachment 575152 [details] [diff] [review] [diff] [details] [review] > (3/3) Cancel pending favicon loads when location changes > > Review of attachment 575152 [details] [diff] [review] [diff] [details] [review]: > ----------------------------------------------------------------- > > ::: embedding/android/GeckoApp.java > @@ +589,5 @@ > > tab.removeTransientDoorHangers(); > > > > + // Cancel any pending favicon load task > > + long faviconLoadId = tab.getFaviconLoadId(); > > + if (faviconLoadId > 0) > > should this be:? > if (faviconLoadId != Favicons.NOT_LOADING) Yep, fixed.
tracking-fennec: --- → 11+
Depends on: 997478
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: