Closed Bug 958087 Opened 8 years ago Closed 8 years ago

nsAndroidHistory::SetURITitle is called for iframes and embeds

Categories

(Firefox for Android Graveyard :: General, defect)

x86_64
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 29

People

(Reporter: mfinkle, Assigned: mfinkle)

References

Details

Attachments

(1 file)

Since DocShell calls SetURITitle for iframes and other embedded content, we try to save titles to the DB, even though it will fail. For cnn.com, for example, this leads to 15 SetURITitle calls. We try to update the DB 15 times, and it can take ~100ms on some mid range phones.

Desktop keeps a list of recent "emdbed" URIs in it's History implementation. If a URI is in the "embed" list they do not try to save the title change. We could do something similar.
This patch mimics what desktop does, as best we can. We allow AndroidHistory to cache the last 128 non-toplevel URIs. We use that list to ignore SetURITitle changes.

I was profiling an HTC Desire HD, loading cnn.com, GlobalHistory.update() times:
before: 98ms
after: 10ms

Pages that have a lot of iframes, mainly for social buttons and ads, are affected more than nice mobile pages.
Assignee: nobody → mark.finkle
Attachment #8357812 - Flags: review?(blassey.bugs)
Comment on attachment 8357812 [details] [diff] [review]
Ignore embed URIs v0.1

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

::: mobile/android/components/build/nsAndroidHistory.cpp
@@ +132,5 @@
> +  bool equals = false;
> +  EmbedArray::index_type i;
> +  EmbedArray::size_type length = mEmbedURIs.Length();
> +  for (i = 0; i < length && !equals; ++i) {
> +    aURI->Equals(mEmbedURIs.ElementAt(i), &equals);

As I mentioned on irc, we might want to make this a sorted list and do a binary search.
Attachment #8357812 - Flags: review?(blassey.bugs) → review+
Blocks: 947390
https://hg.mozilla.org/integration/fx-team/rev/75f5cbf9c085

If we decide to add the sorting, we should try it for the RecentlyVisitedURIs array too.
https://hg.mozilla.org/mozilla-central/rev/75f5cbf9c085
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 29
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.